Keith Packard <[email protected]> writes:

> Eric Anholt <[email protected]> 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!

Attachment: signature.asc
Description: PGP signature

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to