[cairo] _cairo_surface_wrapper_get_target_extents() too small

[Guillaume Ayoub]

Le vendredi 29 mars 2013 à 22:55 +0100, Guillaume Ayoub a écrit :
> Le vendredi 29 mars 2013 à 15:43 +0100, Simon Sapin a écrit :
> > Le 27/03/2013 13:12, Simon Sapin a écrit :
> > > Hi,
> > >
> > > When rendering to PDF with WeasyPrint a document containing a SVG image,
> > > SVG elements in the right and bottom of the image are not shown in some
> > > cases.
> > >
> > > git bisect indicates that the bug appeared in this cairo commit:
> > >
> > > http://cgit.freedesktop.org/cairo/commit/?id=09b42c748e9dbcc923560c7d8bf5298fbffe95ef
> > >
> > > Reverting it on top of today’s master fixes the bug.
> > >
> > >
> > > WeasyPrint has CairoSVG render the image to an intermediate cairo SVG
> > > surface, which is then used in surface pattern as a source to paint in
> > > the final PDF surface. The bug appears when the target context has a
> > > scale to make the image smaller than its "intrinsic" size. Making the
> > > SVG surface smaller (with its content scaled accordingly) and
> > > compensating with a scale on the target surface works around the problem.
> > >
> > > Apparently, the rectangle returned by
> > > _cairo_surface_wrapper_get_target_extents() is smaller than it should
> > > be. Something is transformed wrong, but I’m not sure what coordinate
> > > system each rectangle is supposed to be in.
> > 
> > 
> > Based on further research, it appears that the wrapper->transform value 
> > in _cairo_surface_wrapper_get_target_extents() should account for the 
> > target context’s transformation matrix, but doesn’t. wrapper->transform 
> > is always the identity matrix in our tests, even though the context has 
> > a non-identity transform when painting the surface pattern.
> > 
> > Removing any calls to cairo_save() and cairo_restore() also works around 
> > the issue, but for reasons we do not understand.
> 
> I finally think that I have found what's happening.
> 
> In cairo-analysis-surface, the transformation matrix is stored in
> surface->ctm, and this matrix is handled by _add_operation for all the
> operations added in the analysis surface. In
> _analyze_recording_surface_pattern, the ctm surface is set, everything
> is OK for the future operations.
> 
> But _analyze_recording_surface_pattern calls
> _cairo_recording_surface_replay_and_create_regions, calling
> _cairo_recording_surface_replay_internal, calling
> _cairo_surface_wrapper_get_target_extents. As replay_and_create_regions
> calls replay_internal without the surface_transform parameter, the
> original transformation matrix is lost. get_target_extents relies on
> wrapper.transform that is not set, and get_target_extents then uses the
> original size of the target to reduce the extents rectangle, instead of
> using the size multiplied by the transformation matrix.
> 
> A simple solution to fix this is to add a surface_transform parameter to
> _cairo_recording_surface_replay_and_create_regions (as it is done in
> _cairo_recording_surface_replay_with_clip), and to use the invert matrix
> of &tmp->ctm as surface_transform parameter of
> _cairo_recording_surface_replay_and_create_regions.

Here is a simple patch solving this issue.

This patch adds a "surface_transform" parameter to
replay_and_create_regions. It was already done in
surface_replay_with_clip (it is needed for the clip), but it's also
needed by replay_and_create_regions, because get_target_extents clips
the target surface for performance issues (that's what the "culprit"
commit 09b42c7 does).

I can provide further information if needed.
-- 
Guillaume
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cairo-fix-extents.patch
Type: text/x-patch
Size: 3248 bytes
Desc: not available
URL: <http://lists.cairographics.org/archives/cairo/attachments/20130513/e6b19ef2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://lists.cairographics.org/archives/cairo/attachments/20130513/e6b19ef2/attachment.pgp>