Bug 105772

Created attachment 138384 [details] [review]
Patch that implements type3 font support in Arthur backend

I forgot to mention: this needs the implementation of drawImageMask from

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

to work.

- Why white?  Why not any other color?

Because the code is buggy 

- Apparently the method is rarely called: The renderToArthur method
  in file poppler-page.cc, for example, does not call it.

Are you sure about that? renderToArthur calls displayPageSlice that calls displaySlice that calls createGfx that calls Gfx constructor with pagenum that calls startPage

- Most importantly: We want to use ArthurOutputDev to render type3
  glyphs.  But they should have a transparent background, or rather,
  no background at all.

I don't understand this reasoning.

>> - Why white?  Why not any other color?
> Because the code is buggy 

I'd happily try to fix the code, if somebody tells me what the startPage method is supposed to do.

>> - Apparently the method is rarely called: The renderToArthur method
>>   in file poppler-page.cc, for example, does not call it.

> Are you sure about that? renderToArthur calls displayPageSlice that calls displaySlice that calls createGfx that calls Gfx constructor with pagenum that calls startPage

Well, I said 'rarely', not 'never'. :-)  In particular, I had to make the renderToImage method fill the page with the page color, to get a background at all (in 7e844eae94bc4eda1c6dcc3460840b25f4ca7898) The code in startPage didn't seem to have any effect.  But I readily admit that I was never quite sure what the proper location is for the code that initializes the page background.

Tangentially related: What is the precise meaning of the IgnorePaperColor flag?  Does it mean 'make the background transparent'?

>> - Most importantly: We want to use ArthurOutputDev to render type3
>>  glyphs.  But they should have a transparent background, or rather,
>>  no background at all.

> I don't understand this reasoning.

The type3 code in the attached patch is partially cargo-culted from the cairo backend.  It creates new ArthurOutputDev and Gfx objects for each glyph, and uses them to render a single glyph into a QPicture.  The cairo backend calls startPage on the output device, which in the case of the ArthurOutputDev leads to the effect that each glyph has a white rectangular background.  You will notice that when rendering type3 text on a non-white background.

(In reply to oliver.sander from comment #4)
> >> - Why white?  Why not any other color?
> > Because the code is buggy 
> 
> I'd happily try to fix the code, if somebody tells me what the startPage
> method is supposed to do.

To be honest i don't think there's a definition of what is supposed to do other than "do the things that you need to do when starting a new page"

> 
> >> - Apparently the method is rarely called: The renderToArthur method
> >>   in file poppler-page.cc, for example, does not call it.
> 
> > Are you sure about that? renderToArthur calls displayPageSlice that calls displaySlice that calls createGfx that calls Gfx constructor with pagenum that calls startPage
> 
> Well, I said 'rarely', not 'never'. :-)  

I don't understand this, you said "renderToArthur method in file poppler-page.cc, for example, does not call it." and i showed you that it does.

> In particular, I had to make the
> renderToImage method fill the page with the page color, to get a background
> at all (in 7e844eae94bc4eda1c6dcc3460840b25f4ca7898) The code in startPage
> didn't seem to have any effect.  But I readily admit that I was never quite
> sure what the proper location is for the code that initializes the page
> background.
> 
> Tangentially related: What is the precise meaning of the IgnorePaperColor
> flag?  Does it mean 'make the background transparent'?

Yes, i'd say you awnt to hide that tmpimg.fill(bgColor); inside a check for not IgnorePaperColor

> >> - Most importantly: We want to use ArthurOutputDev to render type3
> >>  glyphs.  But they should have a transparent background, or rather,
> >>  no background at all.
> 
> > I don't understand this reasoning.
> 
> The type3 code in the attached patch is partially cargo-culted from the
> cairo backend.  It creates new ArthurOutputDev and Gfx objects for each
> glyph, and uses them to render a single glyph into a QPicture.  The cairo
> backend calls startPage on the output device, which in the case of the
> ArthurOutputDev leads to the effect that each glyph has a white rectangular
> background.  You will notice that when rendering type3 text on a non-white
> background.

I see.

Created attachment 138679 [details] [review]
Patch that implements type3 font support in Arthur backend

Updated the patch: The old patch didn't to the text matrix transformation, and therefore misrendered the example file from 105531 ( https://bugs.freedesktop.org/attachment.cgi?id=138385 )

a quick regtest look says It is a definite improvement, so i can commit it if you want. 

*BUT* it still has bugs, page 281 of http://caml.inria.fr/pub/docs/oreilly-book/ocaml-ora-book.pdf shows the hatched parts totally back when it should be only some diagonal bars.

Your call, do you want to try to fix that first or should i commit this?

Let me have a look at that bug first.

There is also the open question whether an alternative implementation using beginType3Char/endType3Char could be simpler and faster.  Should I give that a try, too?  I am undecided.

Shit, i pushed the patch by mistake :( Sorry

Want me to revert it?

To be honest i don't know enough about type3 to be able to suggest what's a better implementation

Keep the patch---you said it is a definite improvement.

- I may look at alternative implementations, but my motivation is independent of whether this patch is submitted or not.

- I had a quick look at the problem with the ocaml book.  I could probably fix it, but I am low on time for a few months now, so let's track that in a separate bug report.

Ok, closing this then and let's track the other in a different place :)