This PDF uses type 3 chars, and somehow the caching of type 3 chars in Splash fails with several:

Syntax Warning: Bad bounding box in Type 3 glyph

I don't completely understand this type of caching in splash, but I see that even if it would work with this PDF, the cached glyphs are never used. And if I remove just the caching of glyphs, it works fine!

Cairo uses the same implementation but it never caches the glyphs, so they are always redrawn.

PSOutput neither uses beginType3Char nor drawChar.

So I think about to remove the type3 glyph caching and do it similar to cairo, but I'm not sure if this cache in splash ever speeds up some rendering and therefore is necessary.

(In reply to Thomas Freitag from comment #1)
> Cairo uses the same implementation but it never caches the glyphs, so they
> are always redrawn.

Cairo caches the glyphs within cairo. Always drawing the glyphs would be very slow.

(In reply to Adrian Johnson from comment #2)
> (In reply to Thomas Freitag from comment #1)
> > Cairo uses the same implementation but it never caches the glyphs, so they
> > are always redrawn.
> 
> Cairo caches the glyphs within cairo. Always drawing the glyphs would be
> very slow.

??? 
Looking in the implementation of CairoOutputDev::beginType3Char the code of the glyph is not saved or used, it always return gFalse, therefore Gfx calls display for the charProc and the endType3Char always, and CairoOutputDev::endType3Char just restores what is saved in beginType3Char. Is there some magic I don't understand?

On the other hand, I missed that splash indeed uses sometimes the cached glyphs for this PDF, I missed that somehow in my first investigation. And the rendering time increases a little bit 

w using the cache

real	0m0.182s
user	0m0.163s
sys	0m0.021s

w/o using the cache

real	0m0.214s
user	0m0.200s
sys	0m0.018s

So I fear I need some more time for investigation.

(In reply to Adrian Johnson from comment #2)
> (In reply to Thomas Freitag from comment #1)
> > Cairo uses the same implementation but it never caches the glyphs, so they
> > are always redrawn.
> 
> Cairo caches the glyphs within cairo. Always drawing the glyphs would be
> very slow.

Forget my comment 3 in case of cairo. Cairo implements CairoOutputDev::beginType3Char and CairoOutputDev::endType3Char, but it is not used because of interpretType3Chars() returns gFalse J

Created attachment 123621 [details] [review]
restore the current position also in output device

As in the comment in Gfx.cc in case of using beginType3Char, GfxState::restore() does *not* restore the current position. But of course this is also the case for any output device uses this branch, so we have to update the CTM also in the output device.

This patch does it.

Pushed!

Hi Thomas,

I came across your patch when implementing type3 support for the Arthur backend.  The one line added by your patch makes my seemingly correct type3 implementation fail.  More details can be found in 

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

Looking at your patch got me confused.  It calls updateCTM with an all-zero matrix.  In my understanding of what updateCTM does, this will multiply the current CTM with zero, and all later rendering will be scaled to size zero.  This is indeed what is happening in ArthurOutputDev.

On the other hand, when I revert your patch, I cannot reproduce the original bug that made you write the patch anymore.  I tried with the test file from

  https://bugs.kde.org/show_bug.cgi?id=362765

I get lots of errors saying

  "Error: Save (q) operator before d0/d1 in Type 3 glyph"

but the rendering seems to be okay.  So would it be possible to get some explanation of why you wrote the patch the way you did?  Is my understanding of updateCTM wrong?  Or does your fix need improvements?

Many thanks,
Oliver

PS: I wrote:

"In my understanding of what updateCTM does, this will multiply the current CTM with zero, and all later rendering will be scaled to size zero."

That's not quite correct.  In my understanding, calling updateCTM with all zero will set the scaling and shearing to zero, and leave the displacement untouched.