Bug 105531 - [PATCH] Implement ArthurOutputDev::drawImageMask
Summary: [PATCH] Implement ArthurOutputDev::drawImageMask
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: arthur backend (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-15 20:07 UTC by oliver.sander
Modified: 2018-04-13 04:18 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
WIP patch that implements ArthurOutputDev::drawImageMask (3.63 KB, patch)
2018-03-15 20:07 UTC, oliver.sander
Details | Splinter Review
Patch that implements ArthurOutputDev::drawImageMask (3.68 KB, patch)
2018-03-27 19:35 UTC, oliver.sander
Details | Splinter Review
Document the method 'OutputDev::drawImageMask' (1.02 KB, patch)
2018-03-27 19:38 UTC, oliver.sander
Details | Splinter Review
said file (1.98 MB, application/pdf)
2018-03-27 22:12 UTC, Albert Astals Cid
Details
Patch that really switches of type3 font rendering (997 bytes, patch)
2018-04-05 20:10 UTC, oliver.sander
Details | Splinter Review

Description oliver.sander 2018-03-15 20:07:17 UTC
Created attachment 138147 [details] [review]
WIP patch that implements ArthurOutputDev::drawImageMask

I am trying to implement type3 font support for the Arthur backend.  When such fonts are present, the method ArthurOutputDev::drawImageMask is called repeatedly, which is currently empty.  So I figured I'd go and implement that method first.

Attached is a prototype implementation that I hacked together using the information from 

  http://www.verypdf.com/document/pdf-format-reference/pg_0350.htm

My problem now is that I do not have an example file to test this.  All the files I have call drawImageMask as part of type3 font rendering, which adds an additional layer of mystery to the problem.  Can anybody help me out with a pdf file that contains an image mask but no type3 fonts?
Comment 1 oliver.sander 2018-03-27 19:35:01 UTC
Created attachment 138381 [details] [review]
Patch that implements ArthurOutputDev::drawImageMask
Comment 2 oliver.sander 2018-03-27 19:37:11 UTC
I updated the patch: the old patch had the meaning of the 'invert' flag inverted (sic).

By now I have a working protoype for type3 font rendering that uses drawImageMask, so my drawImageMask implementation can't be all that bad.  Please test and review!  Thanks.
Comment 3 oliver.sander 2018-03-27 19:38:45 UTC
Created attachment 138382 [details] [review]
Document the method 'OutputDev::drawImageMask'

Here is a bit of documentation for the OutputDev::drawImageMask method.
Comment 4 Albert Astals Cid 2018-03-27 22:12:22 UTC
Hmmmm, can you have a look at the 090_Font-Support_x3.pdf file i'm attaching? With your patch there's a big n appearing on the bottom left that seems like shouldn't be there?
Comment 5 Albert Astals Cid 2018-03-27 22:12:54 UTC
Created attachment 138385 [details]
said file
Comment 6 oliver.sander 2018-04-05 20:09:27 UTC
That big 'n' is one of the type3 font letters contained in the file.  I think what is happening is the following:

- ArthurOutputDev claims to do type3 support by the beginType3Char/endType3Char mechanism

- Hence beginType3Char/endType3Char are being called, but they do nothing.

- In between, the actual glyph rendering operations are called.  For each glyph there is one call to drawImageMask, because the font is a bitmap font.

- But since beginType3Char/endType3Char do not do the necessary transformations, the glyph rendering identifies font coordinates and page coordinates, and all letters appear in the lower left corner.  The one you see is the last one.

My explanation is substantiated by the following simple patch, which switches off type3 rendering for good. Then the 'n' is gone (but also drawImageMask is not called anymore, and the document ceases to be a good test case for that method).

Thinking about this some more, one could probably implement type3 font support by "simply" making beginType3Char/endType3Char do the correct transformations.  This would be an alternative implementation to

  https://bugs.freedesktop.org/show_bug.cgi?id=105772

and possibly simpler.  Given that 105772 already kind-of works, do you think the alternative should be explored?
Comment 7 oliver.sander 2018-04-05 20:10:09 UTC
Created attachment 138640 [details] [review]
Patch that really switches of type3 font rendering
Comment 8 Albert Astals Cid 2018-04-05 21:54:27 UTC
I guess that if at this stage the type3 fonts don't work the best is simply switch them off.

Do you want me to start a regtest with the patch that disables them?
Comment 9 oliver.sander 2018-04-06 07:22:32 UTC
Yes, please!

BTW, do the other glyphs in the example document look good for you?  Because on my machine some of the fonts are way too heavy.  Unfortunately, my impression is that the problem appeared after some random Debian testing update, and it may be a Qt bug.
Comment 10 Albert Astals Cid 2018-04-07 21:47:40 UTC
I've pushed it, to be honest i only went through like 50 of the 300 files the regression test found differences on, but the 50 first files were a definitive improvement so let's say this is a step in the right direction :)
Comment 11 oliver.sander 2018-04-08 05:42:28 UTC
Thanks!  Would you also push the documentation patch?

BTW, should such patches use doxygen syntax or something similar?
Comment 12 Albert Astals Cid 2018-04-12 20:44:42 UTC
Are you sure the documentation is totally correct? I'm not saying it isn't. i'm not very well versed on what the method exactly does. I'm just saying that sometimes is preferable to have no docu than docu that is partially wrong, but if you're totally sure it's correct i'll push it :)
Comment 13 oliver.sander 2018-04-13 04:18:04 UTC
Yes, I am totally sure.


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.