# Describe the Feature 1. Allow the user to draw a selection over the rendered page in order to define a visible bounding-box to be applied to all pages in the document. Primarily intended for PDF files but applies equally to .ps and .djvu. 2. Optionally, allow the user to draw across two pages in a "Facing pages" view mode to handle two-sided documents with different inner/outer margins. 3. Very Optionally, persist the selected bbox info across open/close of application. # Are you offering to implement it Yes. # Why is this a good idea? 4. In many cases, the PDF files users view are layed out with print in mind and their dimensions are inconvenient for on-screen viewing even on modern large screens. "Trim margins" can help with this but, being automated, it doesn't always produce satisfatory results and a manual specification can improve readability. 5. For scientific/journal papers in particular, but generally, there are usually header/footer lines and additional spacing between them and the typearea which is redundant for on-screen reading. Removing these areas from view can win precious screen real-estate for viewing the actual text. Reproducible: Always I have a crude implementation of this already working and personally find it so useful I'd like to see a refined version of it merged into mainline. To improve the chances of getting the feature accepted for inclusion, I'd like to solicit some guidance from the developers, primarily on the UI aspects of integrating a new feature. Once I know what you'd like to see, I'll put what I have on github for testing and further feedback. Questions: 1. I'm currently piggybacking the "Selection Tool", by adding a "trim to selection" item to the menu that appears once a selection has been made. Is this acceptable? it isn't as "discoverable" for the user as the "Trim Margins" option. 2. "Trim Margins" and the suggested "Trim to selection" interfere with each other, there needs to be a way to switch from one to the other or turn both off. Would an additional "Trim to selection" menu item, mutually-exclusive with "Trim Margins", b appropriate? 3. I've been unable to disable this "Trim to selection" mode from being reapplied when okular is restarted. "Trim Margins" behaves this way but it is inappropriate for "Trim to selection", where in the source should I be looking? 4. Other concerns?
1. just make it a new tool? 2. What would that menu item do? 3. How is it actually applied? I mean it is applied if you save the data for it to be applied. 4. I never liked Trim Margins and it's a can of bugs, expanding in that direction is probably going to introduce even more bugs. Is this really something someone else than you is going to use?
1. Unlike other tools, it is also modal -- applying it changes the view, and deselecting it shouldn't disable it. You may want to trim an article to read it, then annotate as you read. 2. Show that the mode is activated. Allow Deactivation. Possibly trigger the selection tool. 3. I've followed the "Trim Margins" code closely and introduced an entry into the settings. I think the save/restore logic is somehow part of that but I don't see where I need to tweak it. 4. How can I answer that? I do like it. I do use it. All software has bugs. Any new feature introduces bugs. Bugs can usually be fixed (unless they are due to bad design). Yes, I find it extremely useful and yes I think others would too. Other tickets on this bugtracker show that users are making use of "Trim margins", mention its usefulness, and there are multiple requests for increased functionality in that direction, including what I'm suggesting.
Here's an example file for which "Trim Margins" is insufficient: http://ajcn.nutrition.org/content/82/1/69.full.pdf
1. right 2. so maybe make the trim margins a submenu with two items that are mutually exclusive and that you can uncheck? 3. If you don't need to save it, why store it in the settings? (and no i'm not going to be able to give you any more concrete help until you post a review request in reviewboard and there's code to see)
3. Fair enough, that can wait anyway. Assuming the code is reasonably unobtrusive, is there a fair chance this'll be merged if I get it ready? if yes, I'll make the necessary changes and put up the code for your comments.
Let's see the code, and then we can talk about it. If it has tests, the chances for merging increase :)
Tests! The bane of driveby code contributions.... grrr
If you're going to dump some code that i'm have to going to maintain for the next 10 years, you may as well help me a bit with it, no?
True.
Just about there, I'll let it sleep tonight and have a fresh look again tomorrow to spot any brain-slippage, then i'll up it to RB. I'll add the tests once we're agreed the interaction is ok. Before upping for review: - What branch should I rebase against? - Is there a project-mandated `astyle` incantation (or somth) for code formatting?
- What branch should I rebase against? master - Is there a project-mandated `astyle` incantation (or somth) for code formatting? No, just follow the style of the file
Review Request is up: https://git.reviewboard.kde.org/r/124716/
*** Bug 350073 has been marked as a duplicate of this bug. ***
ping
I've seen the review, i'm very busy and since this is a new feature that can't be released until december i'm giving it a bit less priority, don't worry i won't forget
Git commit 172d78c6b3f9d7894a1233fb12e5e292f13710ee by Albert Astals Cid, on behalf of Jake Linder. Committed on 27/08/2015 at 20:09. Pushed by aacid into branch 'master'. Add "Trim To Selection" feature Changes C1. Added submenu, moved "Trim margins" (TM mode) to it and added "Trim To Selection" (TS mode). C2. Activating "Trim To selection" enters a new mousemode, similar to RectSelect for defining a viewport. C3. Once a viewport has been defined, it serves as a viewport for all pages in the document. C4. Left/Right pages are not treated differently. Manual Testing T1. Switching between modes enforces at most one active. T2. Can deactivate a mode by selecting it again from the menu. T3. When draggin bbox selection, clicking outside page does not crash. T4. When in "Facing Pages" mode, mouse release must be over any page (or is ignored). T5. Normalized bbox coords are computed relative to page indicated by point of mouse release. T6. Behave as expected when switching between any pair of No Trim/Trim Margins/Trim To Selection. T7. TM mode persisted across app restarts (existing behavior). T8. TS mode forgotten across app restarts (as desired). T9. Exiting and reselectin "Trim To Selection" prompts for new bbox. T10. Choosing a small Trim bbox enforces minimium dimensions size (As percentag of total), as it does in TM mode, because of the "scale big and crop down" implementation, to avoid huge pixmaps. TS mode minimum set at 20% (vs. TM mode's 50%). REVIEW: 124716 M +9 -0 conf/okular.kcfg M +2 -2 part-viewermode.rc M +2 -2 part.rc M +162 -16 ui/pageview.cpp M +4 -0 ui/pageview.h http://commits.kde.org/okular/172d78c6b3f9d7894a1233fb12e5e292f13710ee