[cairo] Corrupt image rendering in 1.5.18

[Carl Worth]

Brown paper bag time...

Just before I made the 1.5.18 snapshot, Owen pointed out an
interesting new bug[*] and it looked like it had an easy fix, so I
threw it in.

To understand the issue here. First recall that cairo_paint is
conceptually an operation that reads from an infinitely large pattern
and writes to everywhere on the target surface. But for efficiency, we
don't want to do any more work than necessary, of course. So for a
surface pattern with EXTEND_NONE, in _cairo_pattern_get_extents we had
been assuming that the extents of the surface itself are exactly the
extents of the surface.

However, that's an invalid assumption with a filter like BILINEAR
which with cairo_paint() and EXTEND_NONE, (while scaling up by a
non-integer amount, say), causes a larger region of pixels in the
result to be affected than the extents of the original surface, (since
the image data is blurred together with the transparent areas outside
the surface provided by EXTEND_NONE). I've got more to say about this
blurring, but I'll put that in a separate mail.

So I tweaked _cairo_pattern_get_extents to expand the pre-transformed
extents by 0.5 in the case of a BILINEAR filter.

Of course, we know now that this has led to some nasty bugs. For
example, mozilla is getting corrupted results like this:

	http://people.mozilla.com/~vladimir/misc/broken-xlib-rgb24-out.png

So one problem here is that I threw in a fix extremely late in the
release for a poorly-justified problem. By "poorly justified" I don't
mean that Owen did anything wrong. He encountered a real bug and he
reported it. What I mean is that anybody that gets that blurriness
changes their code to avoid it. Nobody in practice complains that they
didn't get quite enough blurriness, (well, OK, Owen did, but again,
hopefully you understand the point).

So one problem here is that I'm here in two roles---both fixing bugs
and being the gatekeeper of commits leading to the release. A separate
gatekeeper, (or more discipline on my part), may have led to a
decision to delay the bug fix. Or maybe not, it's always hard to
predict unanticipated, new bugs introduced by fixes for other bugs.

Anyway, prior to 1.5.18 Owen did see further things[2] that could be
improved, and he actually was careful to point out that these further
things should wait until after 1.6. And maybe he would have even said
the same thing about the fixes for the first bug, (I'm the one that
shoved them right in).

So, some ideas about avoiding new bugs late in the release process
might be helpful. Meanwhile, we've got this current bad behavior to
fix.

One option would be to back out the change I committed. Another option
is to actually fix the bug. Owen has started chasing the bug down, and
he has found problems in _cairo_pattern_acquire_surface. He's put some
good writeups about the problem into bugzilla, and I'm inviting him to
follow up here on the list, (Owen, you can just quote what you've
written already).

What's interesting is that the buggy _cairo_pattern_acquire_surface
was tweaked just before the cairo 1.4.4 release. Again, there we had
made a change to be more efficient, (allocating a mask only as large
as the extents of trapezoids being rendered), and the filter expansion
caused us some problems. But note the disclaimers in Behdad's fix for
that bug:

	commit 84c10a79ffd233a953434bd787dcfe57787552f8
	Author: Behdad Esfahbod <behdad at behdad.org>
	Date:   Fri Apr 13 16:33:07 2007 -0400

	    [cairo-pattern] Slightly hackish fix for bug #10508

	    The so-attributed-to-X-server bug was that cairo maps the drawing
	    region to the pattern space, rounds the box, and uploads only that
	    part of the source surface to the X server.  Well, this only works for
	    NEAREST filter as any more sophisticated filter needs to sneak a peek
	    at the neighboring pixels around the edges too.

	    The right fix involves taking into account the filter used, and the
	    pattern matrix, but for most cases, a single pixel should be enough.
	    Not sure about scaling down...

	    Anyway, this is just a workaround to get 1.4.4 out of the door. I'll
	    commit a proper fix soon.

There we put in a "slightly hackish", "just a workaround" patch to get
a release out the door. And it would appear from the current situation
that the "proper fix" never happened. So I do want to be cautious that
we don't do that again. Of course, that can be a conflicting caution
with the caution of "don't make changes that aren't justified by
significant problems" as I was describing earlier in this thread.

Software and releases management is hard.

-Carl

[1] The original bug is described here:

	bad clipping with EXTEND_NONE
	http://bugs.freedesktop.org/show_bug.cgi?id=15349

[2] Owen's further improvements are here:

	Improve filtering handling in cairo-pattern.c
	https://bugs.freedesktop.org/show_bug.cgi?id=15367
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080407/07540d75/attachment.pgp