[PATCH xserver 1/2] glamor: Fix copy-like Render operations between 15 and 16 depth.

Eric Anholt eric at anholt.net
Fri Jan 22 10:31:05 PST 2016


Keith Packard <keithp at keithp.com> writes:

> Eric Anholt <eric at anholt.net> writes:
>
>>      if (op == PictOpSrc) {
>> +        /* We can't do direct copies between different depths at 16bpp
>> +         * because r,g,b are allocated to different bits.
>> +         */
>> +        if (dst->pDrawable->bitsPerPixel == 16 &&
>> +            dst->pDrawable->depth != src->pDrawable->depth) {
>> +            return 0;
>> +        }
>> +
>
> How can this pass the following check then? The render format is
> supposed to completely define the component layout within a pixel?

Yes, it's supposed to, but we don't actually store 16bpp anything in
glamor, because we suck.  We do getimage/putimage in and out of our
32bpp storage using two different 16bpp channel layouts depending on the
depth.  So, it's all a terrible mess, and trying to represent Render
operations on top of it is basically hopeless (this is why the general
composite path doesn't support anything for 16bpp).  We need texstorage.

To more clearly outline how things were broken, let's say I upload some
contents to a 15 depth pixmap:
xrrrrrgggggbbbbb

which gets splatted out to a 32bpp rgba actual storage
xxxxxxxxrrrrrrrrggggggggbbbbbbbb

then do a Render Src from this as x1r5g5b5 to a dest with x1r5g5b5, but
my dest is 16 depth, hitting this path.  It's still stored as 32bpp
rgba, so that's:
xxxxxxxxrrrrrrrrggggggggbbbbbbbb

then I download with getimage, and since it's 16bpp glamor_transfer.c
gives us back:
rrrrrggggggbbbbb

Wait!  I uploaded xrrrrrgggggbbbbb and did what should have been a
bit-for-bit copy (except the x is undefined), and my rs and gs got
shifted!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20160122/f2a966ae/attachment.sig>


More information about the xorg-devel mailing list