Review Request 127366: Resize annotations

Tue Mar 14 00:43:04 UTC 2017

> On März 11, 2017, 4:57 nachm., Albert Astals Cid wrote:
> > Feature Freeze date for 17.04 is March 23, i suggest you try to workaround/disable the known bugs/todo since this sems a nice feature to get in. It can always be improved after it lands, but what lands needs to be "good" and "consistent"

Ok, all obvious bugs and misbehaviors should be fixed now. If more things need to be done, I will try to get them finished until March 23.

Testing was done with documents gathered from other projects, and some handcrafted Latex stuff. Coverage is not perfect due to lack of professional PDF tools or reference documnets on my side.

I've got a minor concern about the UI concept: If some day we should be able to move/resize file-, movie-, sound annotations, then there will be a conflict on what happens when left-clicking on it. It would mean "save"/"play" and "select" at the same time. Currently there's no problem because move/resize is disabled for the mentioned annotation types.

And finally, if there's a brave guy who grasps pageview.cpp in its entire beautifulness it would be good if she/he could have a careful look at it ;-)

- Tobias

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127366/#review102789
-----------------------------------------------------------

On März 13, 2017, 11:42 nachm., Tobias Deiminger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127366/
> -----------------------------------------------------------
> 
> (Updated März 13, 2017, 11:42 nachm.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 177778, 314843, 358060, and 375648
>     http://bugs.kde.org/show_bug.cgi?id=177778
>     http://bugs.kde.org/show_bug.cgi?id=314843
>     http://bugs.kde.org/show_bug.cgi?id=358060
>     http://bugs.kde.org/show_bug.cgi?id=375648
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> Add annotation resize functionality (Bug 177778, Bug 314843, Bug 358060, Bug 375648).
> 
> Usage:
> If you left-click an annotation, it gets selected. Resize handles appear on the selection rectangle. When cursor is moved over one of the 8 resize handles on the corners/edges, the cursor shape changes to indicate resize mode. Everywhere else on the annotation means "move", just as it was before resize feature was added. Pressing ESC or clicking an area outside the annotation cancels a selection. Pressing Del deletes a selected annotation.
> 
> Feature is only applicable for annotation types AText, AStamp and AGeom.
> 
> Implementation:
> It works by eventually changing AnnotationPrivate::m_boundary and notifying generator (i.e. poppler) about that change. Annotation state handling is shifted out of PageView into a new class MouseAnnotation (ui/pageviewmouseannotation.cpp). Some functionality not related to resizing but to annotation interaction in general is also shifted to class MouseAnnotation, to build a single place of responsiblity.
> 
> Other changes:
> Add method Document::adjustPageAnnotation, backed by a QUndoCommand.
> class Okular::AdjustAnnotationCommand.
> Add Annotation::adjust and Annotation::canBeResized methods.
> Draw resize handles in PagePainter::paintCroppedPageOnPainter.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2f75a9f182cb8ded90bdf536893cbb55bee4d7aa 
>   core/annotations.h d657602fed83b4850a29d0ccec5c6bca5d7a4365 
>   core/annotations.cpp c95fe877316398a5341e29146379dd231dfa40c7 
>   core/annotations_p.h 07b124a4fae40b7a983aa382ae824125e6d25746 
>   core/document.h 1fd86262f89651b7c09d87c16400aebb79fcfe55 
>   core/document.cpp f8f825c6a0037656c9ffc046f20e897ce32dbbf5 
>   core/document_p.h 851194d301dd9c085f7d9b7d2be3cf9a0941d723 
>   core/documentcommands.cpp aafc45a1a989a7240520f2168e253d51d6744f7c 
>   core/documentcommands_p.h b8a6f239ce11374f973baca0c0c31bfd5886bc5e 
>   generators/poppler/annots.cpp df67986adc076f722e601c3bea187200ecf9df31 
>   ui/pagepainter.cpp 9d8b1da9436b75e7019994a707fa77560d53dbee 
>   ui/pageview.cpp e30eb336bd40f5d70075d8333421c58d55e0b8b3 
>   ui/pageviewmouseannotation.h PRE-CREATION 
>   ui/pageviewmouseannotation.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127366/diff/
> 
> 
> Testing
> -------
> 
> Resize and move work
> -for types AText, AStamp and AGeom
> -on all pages of document
> -when viewport position changes
> -when zoom level changes
> -for all page rotations (0°, 90°, 180°, 270°)
> 
> Selection is canceled
> -when currently selected annotation is deleted
> -on mouse click outside of currently selected annotation
> -ESC is pressed
> 
> Viewport is shifted when mouse cursor during move/resize comes close to viewport border.
> Resize to negative is prevented.
> Tiny annotations are still selectable.
> If mouse is moved over an annotation type that we can focus, and the annotation is not yet focused, mouse cursor shape changes to arrow.
> If mouse cursor rests over an annotation A, while annotation B is focused, a tooltip for annotation A is shown.
> Selected Annotation is deleted when Del is pressed.
> 
> Test for regressions:
> -Annotation interaction (focus, move, resize, start playback, ...) are only done in mode EnumMouseMode::Browse.
> -If mouse is moved over an annotation type where we can start an action, mouse cursor shape changes to pointing hand.
> -If mouse is moved over an annotation type that we can't interact with, mouse cursor shape stays a open hand.
> -If mouse cursor rests over an annotation of any type, a tooltip for that annotation is shown.
> -Grab/move scroll area (on left click + mouse move) is prevented, if mouse is over focused annotation, or over AMovie/AScreen/AFileAttachment annotation.
> -A double click on a annotation starts the "annotator".
> 
> 
> Thanks,
> 
> Tobias Deiminger
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20170314/0d8a8adb/attachment-0001.html>