Hi Daniel, So was feeling a bit bored and decided to remind myself about all the crazy formats out there :-)
I believe you've got a couple of mistakes, plus there's a couple of questions/ideas below. Note that handful of those are inspired by now format info is stored in DRM. On 3 March 2017 at 23:05, Daniel Stone <dani...@collabora.com> wrote: > --- /dev/null > +++ b/libweston/pixel-formats.c +#include <drm/drm_fourcc.h> Err... please don't. This will include the kernel copy of the header while we want to use the libdrm one -> <drm_fourcc.h> > +static const struct pixel_format_info pixel_format_table[] = { > + .format = DRM_FORMAT_YUYV, > + .sampler_type = EGL_TEXTURE_Y_XUXV_WL, > + .num_planes = 1, > + .hsub = 2, > + }, > + { > + .format = DRM_FORMAT_YVYU, > + .sampler_type = EGL_TEXTURE_Y_XUXV_WL, > + .num_planes = 1, > + .chroma_order = ORDER_VU, > + .hsub = 2, > + }, > + { > + .format = DRM_FORMAT_UYVY, > + .sampler_type = EGL_TEXTURE_Y_XUXV_WL, > + .num_planes = 1, > + .luma_chroma_order = ORDER_CHROMA_LUMA, > + }, > + { > + .format = DRM_FORMAT_VYUY, > + .sampler_type = EGL_TEXTURE_Y_XUXV_WL, > + .num_planes = 1, > + .luma_chroma_order = ORDER_CHROMA_LUMA, > + .chroma_order = ORDER_VU, All of these four should all have hsub = 2, afaict. The only difference between them is the order in which the components are stored. > + }, > + { > + /* our one format with an alpha channel and no opaque equiv; > + * also cannot be trivially converted to RGB without writing > + * a new shader in gl-renderer */ > + .format = DRM_FORMAT_AYUV, > + .sampler_type = EGL_TEXTURE_Y_XUXV_WL, > + .num_planes = 1, > + }, According to WL_bind_wayland_display and the i965/i915 drivers (afaict) above five should have two planes. Then again, the FOURCC and DRM formats are pretty clear that we're talking about a packed format in a single plain... Did you test these, do we have a spec bug, is the i915/i965 iimplementation working/been tested ? Seems like DRM_FORMAT_AYUV is going to act strange with the pixel_format_is_opaque() helper. As-in latter will return true, despite the format has alpha. > + { > + .format = DRM_FORMAT_NV16, > + .sampler_type = EGL_TEXTURE_Y_UV_WL, > + .num_planes = 2, > + .hsub = 2, > + .vsub = 1, > + }, > + { > + .format = DRM_FORMAT_NV61, > + .sampler_type = EGL_TEXTURE_Y_UV_WL, > + .num_planes = 2, > + .chroma_order = ORDER_VU, > + .hsub = 2, > + .vsub = 1, > + }, Might be the only person, who finds the mixed use of 0 and 1 as [hv]sub sampling confusing. Strictly peaking 1x1 is equivalent no subsampling, hence why we have 0's below (NV24/42). At the same time, for the single plain packed formats we only specify the hsub, even though we should also mention vsub [=1]. For simplicity (?) the kernel provides 1, when there's no {v,h}sub-sampling. Worth using same the approach here ? > +struct pixel_format_info { > + /** How the format should be sampled, expressed in terms of tokens > + * from the EGL_WL_bind_wayland_display extension. If not set, > + * assumed to be either RGB or RGBA, depending on whether or not > + * the format contains an alpha channel. */ > + uint32_t sampler_type; > + If we have special field say "is_rgb_format" one can correctly deduct the sampler_type. I will also help with the "DRM_FORMAT_AYUV does not have alpha?" question above. - RGB formats - has alpha - RGBA, RGB otherwise - YUV formats - 1/2/3 num_planes -> EGL_TEXTURE_Y_XUXV_WL/EGL_TEXTURE_Y_XUXV_WL/EGL_TEXTURE_Y_XUXV_WL respectively Considering DRM_IOCTL_MODE_ADDFB2 was introduced with kernel 3.3 (2012-03-18) do we really want to have the depth/bpp scruft ? DRM_IOCTL_MODE_ADDFB supported only 5 formats from this table. Thanks Emil _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel