Bug 102036 - [PATCH] Arthur backend renders highlight annotations wrongly
Summary: [PATCH] Arthur backend renders highlight annotations wrongly
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: arthur backend (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-04 13:00 UTC by oliver.sander
Modified: 2017-12-28 23:19 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch that implements ArthurOutputDev::updateBlendMode (3.82 KB, patch)
2017-08-04 13:00 UTC, oliver.sander
Details | Splinter Review
Test document: a short text that is partially highlighted (11.59 KB, application/pdf)
2017-08-04 13:01 UTC, oliver.sander
Details
New test file (24.01 KB, application/pdf)
2017-10-20 10:17 UTC, oliver.sander
Details
Patch that implements ArthurOutputDev::updateBlendMode (2.96 KB, patch)
2017-10-28 19:24 UTC, oliver.sander
Details | Splinter Review
Patch that replaces the QPainter by a stack of QPainters (12.77 KB, patch)
2017-10-28 19:32 UTC, oliver.sander
Details | Splinter Review
Patch implementing rudimentary transparency group support (4.21 KB, patch)
2017-10-28 19:33 UTC, oliver.sander
Details | Splinter Review

Description oliver.sander 2017-08-04 13:00:29 UTC
Created attachment 133245 [details] [review]
Patch that implements ArthurOutputDev::updateBlendMode

The Arthur backend renders highlight annotations wrongly.  The yellow rectangles are simply rendered on top of the highlighted text, hiding that text completely.

The obvious explanation was that the updateBlendMode method is not implemented in ArthurOutputDev.  I therefore prepared a patch that implements that method.  It is simple and self-contained.

Full disclosure: I actually took the implementation from the much larger patch that Mihai Niculescu posted to the mailing list in

  https://lists.freedesktop.org/archives/poppler/2013-June/010370.html

That's why the patch contains Mihai's name in the copyright list.

Only, it doesn't solve the problem: With it, the yellow rectangles still hide the text.  And I don't understand why this is.  When the ArthurOutputDev::fill method that draws the yellow rectangle is called, the blend mode is gfxBlendNormal, which I suppose means "draw right on top of it".  I need a hint here.  What is the mechanism that allows me to see the highlighted text through the rectangle, if it isn't the blend mode?  It doesn't seem to be opacity, either.
Comment 1 oliver.sander 2017-08-04 13:01:25 UTC
Created attachment 133246 [details]
Test document: a short text that is partially highlighted
Comment 2 Albert Astals Cid 2017-08-07 21:39:41 UTC
If you want i can commit this, but honestly i don't have time to sink on ArthurOutputDev trying to help you
Comment 3 oliver.sander 2017-10-20 10:17:48 UTC
Created attachment 134934 [details]
New test file

Updated the test file.  For some reason I don't see a highlight at all any more in the old one.
Comment 4 oliver.sander 2017-10-28 19:24:05 UTC
Created attachment 135143 [details] [review]
Patch that implements ArthurOutputDev::updateBlendMode

Rebased the patch, and removed a stray debugging output.
Comment 5 oliver.sander 2017-10-28 19:31:59 UTC
I finally figured out how to make Arthur render highlight annotations properly.
(Thanks to Adrian for his advice---I should have listened earlier.)  These annotations need support for transparency groups.  My implementation comes as three separate patches:

1) The implementation of updateBlendMode.  Please commit this, it is definitely needed.

2) A patch that replaces the single QPainter object by a stack of QPainters. That one is a bit invasive, that's why I'm submitting it separately.  All references to the current painter are now m_painter.top() rather than simple m_painter. Having a getter method for that may be nice---I am undecided.

3) The actual implementations of {begin|end|paint}TransparenceGroup.  These are short and rudimentary.  Transparency groups can do much more than just highlights, but that has to wait until later.

Thanks for your review.
Comment 6 oliver.sander 2017-10-28 19:32:29 UTC
Created attachment 135144 [details] [review]
Patch that replaces the QPainter by a stack of QPainters
Comment 7 oliver.sander 2017-10-28 19:33:13 UTC
Created attachment 135145 [details] [review]
Patch implementing rudimentary transparency group support
Comment 8 Albert Astals Cid 2017-12-28 23:19:32 UTC
pushed


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.