----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109627/#review29666 -----------------------------------------------------------
Quick and mostly "stylistic" review, had a look at how it works for a line and to be honest i found it a bit hard (i.e. i had to be quite on top of the line) to select it, but i guess it is just a matter of getting used to. I am trying to slowly get new features to come with some kind of tests to make sure stuff does not break. Do you have any knowledge of QTest? Do you think you'd be able to create a test for this? In my mind it would be like * Open pdf * Add annotations of all types * Programatically move the mouse to the locations around the annotations and right click and then check if the menu correctly detects we are on the object or not What do you think? core/annotations.cpp <http://git.reviewboard.kde.org/r/109627/#comment22104> Make the function static core/annotations.cpp <http://git.reviewboard.kde.org/r/109627/#comment22105> function static and & to last param core/annotations.cpp <http://git.reviewboard.kde.org/r/109627/#comment22106> please make as many of this vars as you can const core/annotations.cpp <http://git.reviewboard.kde.org/r/109627/#comment22107> const & for quad core/area.h <http://git.reviewboard.kde.org/r/109627/#comment22110> Don't think this belongs to area.h to be honest core/area.h <http://git.reviewboard.kde.org/r/109627/#comment22108> Please add @p var in your docu like we do in the other places core/area.h <http://git.reviewboard.kde.org/r/109627/#comment22109> don't particulary like the distanceSqr name, but it seems we where already using it, so let it be i guess :D - Albert Astals Cid On March 21, 2013, 12:50 a.m., Peter Grasch wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109627/ > ----------------------------------------------------------- > > (Updated March 21, 2013, 12:50 a.m.) > > > Review request for Okular. > > > Description > ------- > > Multiple features in Okular require to determine what object is at a given > position. Traditionally, this relied on the bounding boxes of the given > object. > These do not necessarily correlate with the user would expect (for example, a > diagonal line of 1px has a very large bounding box). > > This patch implementes shape based selection for the following annotation > types: > Ink, Geometric, Line, Highlight. > Other objects default to the old behaviour. > > > Diffs > ----- > > core/annotations.h 72abdff > core/annotations.cpp 49ab5bd > core/annotations_p.h 221572d > core/area.h 4f63759 > core/area.cpp d772fc0 > core/page.cpp 1db2763 > > Diff: http://git.reviewboard.kde.org/r/109627/diff/ > > > Testing > ------- > > I tested the annotation objects above and a couple of special cases mentioned > in the IRC. > > > Thanks, > > Peter Grasch > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
