[PATCH 2/3] glamor: Use glamor_get_tex_format_type_from_pixmap for glamor_transfer

[Keith Packard]

Eric Anholt <eric at anholt.net> writes:

> Keith Packard <keithp at keithp.com> writes:
>
>> glamor_transfer had some hard-coded depth to format/type conversions
>> which may not match the format used with render. This switches to
>> using the same formats as the render portion of glamor.
>
> glamor_get_tex_format_type_from_pixmap() looks hopelessly broken.  It's
> trying to choose the texture's actual channel layout using the
> format/type arguments to TexImage.  But GL doesn't say anything about
> format/type implying anything about the resulting texture channel sizes
> or layout -- until ARB_texture_view, it just strongly implies that
> format/type doesn't matter, and with ARB_texture_view and
> ARB_shader_image_load_store it's guaranteed that it doesn't matter.

If you ignore how the data are actually stored and just say that glamor
should be using a consistent format for pixels going in and coming out,
then it makes sense to track what the last format we used was. If the
driver can support 2 10 10 10, then presumably you're far more likely to
get 2 10 10 10 data if you ask for it.

> So what you end up doing here is saying "oh, somebody's attached a
> 2101010 picture to my pixmap!  Now in PutImage I'll upload my 32 bits of
> data as 2 bits expanded into the 8-bit alpha and 10 bits truncated into
> the 8 bits or r, g, and b."

Yeah, if it isn't actually attempting to fix the format of the pixmap by
downloading in 8 8 8 8 format and re-uploading in 2 10 10 10 format,
then it's hopelessly broken. However, if we create a pixmap, create a
picture and then upload data, things might work?

> For bonus brokenness, delete the picture (unsetting
> pixmap_priv->base.picture), and do a GetImage (now using
> format_for_depth, 8888 like before), and your data is trashed because
> you read your actually-8888 data as 8888 without doing any bitshifting
> to partially recover from the 2101010->8888 screwup.

Right, clearly the pixmap/fbo needs to track the format rather than
using the attached picture for this data.

> I think instead we want to just rip out the wacky-format support from
> glamor_render.c and the pixmap_priv->picture field, and fall back until
> we get texture_view code.

We have 2 10 10 10 scanout devices, and I don't really want to just not
support those at all for now; is there anything we can do?

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140425/64bb88c5/attachment-0001.sig>