[cairo-bugs] [Bug 88203] Replaying recording surfaces with OVER has far worse performance than when using the SOURCE operator

[bugzilla-daemon at freedesktop.org]

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

--- Comment #5 from Chris Wilson <chris at chris-wilson.co.uk> ---
(In reply to Emanuele Aina from comment #4)
> > Right, an OVER operator with no mask need no incur the replay onto an
> > intermediate surface.
> 
> \o/
>  
> >      /* Are we just copying a recording surface? */
> > -    if (inplace &&
> > +    if ((inplace || (op == CAIRO_OPERATOR_OVER && no_mask)) &&
> 
> Is it safe to ignore need_clip_mask when the operator is OVER?

Indeed, the clip_mask needs to be considered as well. My main point is that we
don't try to overload inplace with the secondary meaning of being able to
replay over top.

> >         /* first clear the area about to be overwritten */
> > -       if (! dst->is_clear) {
> > +       if (! dst->is_clear && op_is_source) {
> >             status = compositor->fill_boxes (dst,
> >                                              CAIRO_OPERATOR_CLEAR,
> >                                              CAIRO_COLOR_TRANSPARENT,
> 
> 
> If I got the code right, using op_is_source instead of checking for
> CAIRO_OPERATOR_SOURCE here would produce some redundant clears in case we're
> using CAIRO_OPERATOR_OVER or CAIRO_OPERATOR_ADD with an opaque pattern.
> 
> > or even replace that op_is_source with inplace for consistency.
> 
> I'm not sure what you meant here, sorry. :(

Hmm, this could with a big comment. It indeed only does need to be op ==
SOURCE, because in the cases where op_is_source/inplace cause us to take this
path with OVER/ADD dst->is_clear by definition. Tricky, but you were right.


> > Can you please check that the recording_surface_over test hits the new path
> > and check that it missed the optimisation before.
> 
> I'm having some issue with the testsuite.
> 
> Even when using `Xvfb -screen 0 1680x1024x24 -ac -nolisten tcp :2` and
> `DISPLAY=:2 CAIRO_TEST_TARGET=xlib ./cairo-test-suite
> recording-surface-over` I only get xlib.argb32 and xlib-render-0_0.rgb24 to
> PASS, while xlib.rgb24, xlib-window.rgb24 and xlib-fallback.rgb24 FAIL (with
> or without the patch applied).

It's just aliasing noise in the edge rendering. The references need to be
regenerated, however Xvfb doesn't render correctly (that's another can of
worms), and actually shouldn't be hitting this code path anyway.

> The two tests using CAIRO_TEST_TARGET=image PASS cleanly, without limiting
> CAIRO_TEST_TARGET I get other failures.
> 
> But I also get lots of failures when I run the full testsuite, so I'm quite
> at a loss here. :(

For testing this change, verifying image is sufficient. The testcase will need
to modified slightly (well a new one to be written) as the current OVER replay
is designed to hit the old optimised path. We will want another OVER replay
that uses a mask so that it is never optimised away, and another OVER replay
that will be optimised by your new code but would not be by the old (i.e. it
should replay over existing content).

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cairographics.org/archives/cairo-bugs/attachments/20150108/94160a63/attachment.html>