[cairo] 16bpp patch

[David Reveman]

On Tue, 2006-02-14 at 11:08 -0800, Carl Worth wrote:
> On Tue, 14 Feb 2006 17:51:58 +0100, David Reveman wrote:
> >
> > It seams like libpixman is sometimes getting the the depth wrong in
> > 16bpp images.
> 
> I don't think this is necessarily a problem in pixman. I think
> cairo-xlib-surface has a bunch of improper depth handling.

Might be true, but I don't think this is the case here. I think missing
functionality in pixman is the big problem.

> 
> > I don't know exactly where it goes wrong but the attached
> > patch avoids the problem by changing so that we don't rely on
> > libpixman's depth value when calling XPutImage.
> 
> Frankly I'm a bit surprised the patch works. It's definitely not the
> right fix. Obviously to use XPutImage we need to have ximage.depth ==
> surface->depth, but just setting it to that isn't what we
> want. Instead, we need to make sure the image is getting created with
> the correct depth in the first place.

Well the image is created correctly. However, you can't pass pixman a
depth value, it's calculated in pixman by counting the number of bits in
the bit masks. And it seams like the server I debugged this on reported
depth 16 but only used 15 bits in the rgb masks so pixman end up with
depth 15. As far as I can tell pixman isn't really using this depth
value for anything, it's just using the masks and bpp and those are
correct.

I know this isn't the perfect fix but it should never end up more wrong
than what we have now, it just fixes those cases where pixman calculates
the wrong depth.

> 
> There are currently two separate paths to _draw_image_surface.
> 
> The first is through acquire/release_dest_image. This path starts with
> an Xlib surface and creates an image surface to match. The actual
> creation happens in _get_image_surface. Note the "XXX: This can't
> work" comment in that function as well as this existing bug report
> specific to a depth-8 server:
> 
> 	Crash inside _get_image_surface() on 8-bit pseudocolor display Xlib surface
> 	https://bugs.freedesktop.org/show_bug.cgi?id=5846

Yes, I know about this as well. However, this one is harder to fix
without some serious work on libpixman.

> 
> The second path is through _cairo_xlib_surface_clone_similar where we
> are starting with an image surface and creating an Xlib surface to
> match.
> 
> So the right fix is to fix these two surface creation things to ensure
> that by the time _draw_image_surface is called, image->depth ==
> surface->depth. For the clone_similar case, we've got an easy way out
> since if there is any "hard" case, we can just identify that and
> return UNSUPPORTED. It's the _get_image_surface case that may force us
> to do "hard" work.
> 
> -Carl
> _______________________________________________
> cairo mailing list
> cairo at cairographics.org
> http://cairographics.org/cgi-bin/mailman/listinfo/cairo

-David