ui/guiutils.cpp
77	I'd be grateful for any ideas on how to make this more accurate. Sometimes it captures text of the previous or next line, sometimes it just misses some text.

You are not touching “Highlight with Comment”-style texts, which is probably good. (And would fit in another commit anyway.)
There was a discussion about this in D10797, and in T8533 in more general, but this patch is probably fine.
(T8533 is why I thought the annotations would have been y-ordered.)

Your code:

You use auto some times. I think that’s only good if the type doesn’t matter or is too long to write it twice on the same line. NormalizedPoint and HighlightAnnotation::Quad will make it more readable.
You use braces for switch cases. Didn’t see that before. For consistency with the rest of Okular: remove them?
Bonus: If you create new functions or types, or modify functions you understand, could you write documentation for them?

I’m also wondering whether the Annotation specific members of GuiUtils shouldn’t be members of Annotation.

Didn’t know HighlightAnnotation::Quad, will add that to my documentation todo list. Probably important for Bug 334297.

ui/guiutils.cpp
77	If you use TextPage::textArea( textSelction ), you will get something like (using above source lines as example...): xtSelection(leftTop, rightDown);// TODO this does not seem if only seem was highlighted, right? That’s because textArea() is this monster function which implements text selection for Text Selection mode. Did you try to make a RegularAreaRect from all NormalizedRect from leftTop and rightDown of each Quad, and pass that to text()?
80	QString::simplified() is more predictable.
83	Why `{}`?
198	Move 5 lines up, so code stays uniform.

In D21376#469822, @davidhurka wrote:

Your code:

Wouldn’t static_cast<>() be better than a C-style cast?

davidhurka added inline comments.May 26 2019, 10:54 AM

ui/guiutils.cpp
130	i18nc could be appropiate here, underline could be a verb as well.

davidhurka added inline comments.Jun 2 2019, 1:21 PM

ui/guiutils.cpp
77	As I learned recently, normalized coordinates are not necessarily normalized in rotation. We will need to test this with rotated view as well. (Or ignore it and then fix it together with Bug 334297.)
80	Well, it’s probably the same, because of the text reordering in TextPage. :)

This would help to fix/implement bug 377886. The missing part is tweaking the search field above the annotation tree view.

https://bugs.kde.org/show_bug.cgi?id=377886 Store text along with annotations like highlight, underline and squiggle

@jangmarker Are you still there, or did I scare you away?

Please move as a Merge Request in https://invent.kde.org/kde/okular

We have pre-commit CI and lots of checks including clazy and clang-tidy there so it's a much better place for doing the review/approval/merge of the code.

This revision now requires changes to proceed.Feb 21 2020, 6:54 PM

Revision Contents
Changeset List

			Path	Packages
M			ui/annotationmodel.cpp (7 lines)
M			ui/annotationpopup.cpp (2 lines)
M			ui/guiutils.h (6 lines)
M			ui/guiutils.cpp (49 lines)
M			ui/pageview.h (1 line)
M			ui/pageviewmouseannotation.cpp (2 lines)