Bug 387639

Summary: Annotations are only partially selectable on a hidpi screen
Product: [Applications] okular Reporter: Oliver Sander <oliver.sander>
Component: generalAssignee: Okular developers <okular-devel>
Status: ASSIGNED ---    
Severity: normal CC: alexkde, haxtibal, nate, postix, simonandric5
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: Test file with a single 'note' annotation
Screenshot showing the selected annotation. Note how the grey selection rectangle only covers the top-left quadrant.
LaTeX source of the test file
Screenshot with Tobias' poppler patch
Screenshot with poppler patch, take 2

Description Oliver Sander 2017-12-06 09:20:48 UTC
Created attachment 109223 [details]
Test file with a single 'note' annotation

'note' annotations (the ones from the top entry of the annotation toolbar) should be selectable by clicking on them.  However, with the current git master I can only select them by clicking on the (roughly) top-left quadrant.  I will attach an example document and corresponding screenshot.

This used to work, and I suspect that it broke when the hidpi branch was merged.

I am using fractional scaling with
$echo $QT_SCREEN_SCALE_FACTORS 
eDP-1=1.5;DP-1=1.5;HDMI-1=1.5;HDMI-2=1.5;
Comment 1 Oliver Sander 2017-12-06 09:21:56 UTC
Created attachment 109224 [details]
Screenshot showing the selected annotation.  Note how the grey selection rectangle only covers the top-left quadrant.
Comment 2 Tobias Deiminger 2018-01-02 22:33:35 UTC
Hi Oliver! Could you give the patch from https://phabricator.kde.org/D9615 a try and tell if it fixes the bug you've found?
Comment 3 Oliver Sander 2018-01-03 08:51:48 UTC
Tobias, thanks for looking into this.  Strangely, your patch doesn't change anything here.  Did you see any change in behavior using the pdf file that is attached here?  Please let me know how I can help you to debug this further.
Comment 4 Tobias Deiminger 2018-01-03 11:25:50 UTC
I guess this is for the same reason as why QT_SCREEN_SCALE_FACTORS works generally unexpected in my setup. See https://bugs.kde.org/show_bug.cgi?id=388458#c2, there are obviously two different DPI settings in effect at once. I have to research a bit on this and hopefully can figure out what's the correct resolution to consider for the icon size calculation.
Comment 5 Tobias Deiminger 2018-01-03 12:10:37 UTC
Ah, wrong guess. Indeed, I can reproduce the error when using your attached PDF file. This happens because the annotation was created and saved to the document before the patch was applied. The saved metadata still holds the wrong annotation size. If you create a new annotation instead, the rectangle size should be fixed. Can you confirm this?
Comment 6 Oliver Sander 2018-01-03 12:47:27 UTC
Recreating the file does not solve the problem, because it was actually created using LaTeX, i.e., without okular.  So maybe this is a bug in the LaTeX pdfcomment package?  I'll attach the source file just in case.
Comment 7 Oliver Sander 2018-01-03 12:48:00 UTC
Created attachment 109649 [details]
LaTeX source of the test file
Comment 8 Tobias Deiminger 2018-01-03 13:38:16 UTC
Sorry, I wasn't aware you used latex to create the annotation. The patch currently only works when creating the notes with Okulars own annotating tools (the ones you get with F6). I have not yet considered externally created annotations. Good hint, will do that.
Comment 9 Tobias Deiminger 2018-01-03 16:23:46 UTC
Here's an analysis of what happens for your document:

Your PDF has page size 56.693 x 56.693 pts (from pdfinfo).
LaTex created your annotation object with a /Rect [ 23.448 39.927 35.403 51.882 ] (inspected with qpdf --qdf --object-streams=disable).
I.e., LaTex set the icon size to 11,955 x 11,955 pts (width=35.403-23.448, heigth = 51.882-39.927).
There's no embedded appearance. That's ok, default icon shall be used.
When loading the file to Okular, Poppler reports annotation boundary metadata: l = 0,413596, t = 0,084861, r = 0,624469, b = 0,295733.
Now we can denormalize those values for verification, using page width * (r-l) and page height * (b-t): That's 11,955 x 11,955 pts again.
=> The same as in /Rect, all fine until now!
Now comes Poppler and forces the icon to size 24 x 24 pts during rendering, see [1]. It doesn't update metadata accordingly.
Okular can't know about that 24 x 24 pts. It looks at values derived from l, t, r, b and draws the rectangle for a icon of size 11,96 x 11,96 pts.
=> mismatch!

This is not even related to HiDPI. I get the same misfit in normal DPI mode. I think Poppler should really not ignore the values in /Rect. We need to decide if we want to go for an workaround in Okular (overwrite metadata with hardcoded known values), or if we try to solve it in poppler. I'd suggest to discuss this in https://phabricator.kde.org/D9615.

[1] https://cgit.freedesktop.org/poppler/poppler/tree/poppler/Annot.cc?h=poppler-0.62.0#n2469
Comment 10 Tobias Deiminger 2018-01-05 08:27:53 UTC
Oliver, would you give the Poppler patch from https://phabricator.kde.org/F5620896 a try? Is anybody interested in discussing this issue with the Poppler guys? I'd do it, but only with a mandate from the Okular team:)
Comment 11 Oliver Sander 2018-01-05 08:47:42 UTC
Hi Tobias, I'll try the patch later today.  As for the poppler guys: Albert, the poppler maintainer, must be reading this anyway.  Albert, your opinion & expertise would be appreciated here.
Comment 12 Oliver Sander 2018-01-05 12:50:31 UTC
I tried the poppler patch and things look much better.  The size of the frame is now basically correct, but the position is a few pixels off.  I'll attach a new screenshot.
Comment 13 Oliver Sander 2018-01-05 12:51:03 UTC
Created attachment 109693 [details]
Screenshot with Tobias' poppler patch
Comment 14 Tobias Deiminger 2018-01-05 18:57:30 UTC
Created attachment 109698 [details]
Screenshot with poppler patch, take 2

Thanks for testing Oliver. Hm, the Poppler patch produces an exact match under all conditions for me, no idea what causes the few pixel offset at your side. Doesn't matter much yet, just wanted to show that it could be done within Poppler. If we decided to fix it in Poppler, we'll have to improve the patch until it's good enough anyway.
Comment 15 Tobias Deiminger 2018-03-24 16:12:49 UTC
For the poppler part I've just started a more elaborated patch series at https://bugs.freedesktop.org/show_bug.cgi?id=105692#c4. Would be nice to receive comments/reviews there.
Comment 16 Oliver Sander 2018-03-25 08:45:13 UTC
Hi Tobias, I see myself unable to judge these patches, but do you want me to test anything?
Thanks,
Oliver
Comment 17 Tobias Deiminger 2018-03-25 16:38:05 UTC
Hi Oliver, thanks for the offer! Not yet. I hope for interest, discussion, and eventually agreement on how it should be. Then we can go into testing.
Comment 18 Bug Janitor Service 2021-04-23 18:53:14 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/412