[Mesa-dev] [PATCH] mesa: Match MESA_FORMAT_B5G6R5 for a shallow pixel format of GL_RGB

[Chris Wilson]

On Thu, Sep 03, 2015 at 11:08:11AM -0700, Ian Romanick wrote:
> On 09/03/2015 09:05 AM, Chris Wilson wrote:
> > If the user supplies a pixel format of GL_RGB + GL_UNSIGNED_SHORT_5_6_5
> > and specifies a generic unsized GL_RGB internal format, match that to a
> > texture format of MESA_FORMAT_B5G6R5 if supported by the hardware.
> > 
> > Noticed while playing with mesa-demos/teximage:
> > 
> >   TexImage(RGB/565 256 x 256): 79.8 images/sec, 10.0 MB/sec
> >   TexSubImage(RGB/565 256 x 256): 3804.9 images/sec, 475.6 MB/sec
> >   GetTexImage(RGB/565 256 x 256): 99.5 images/sec, 12.4 MB/sec
> > 
> > becomes
> > 
> >   TexImage(RGB/565 256 x 256): 3439.1 images/sec, 429.9 MB/sec
> >   TexSubImage(RGB/565 256 x 256): 3744.1 images/sec, 468.0 MB/sec
> >   GetTexImage(RGB/565 256 x 256): 4713.5 images/sec, 589.2 MB/sec
> > 
> > on a puny Baytrail which is still far from what it is capable of. The
> > reason for the disparity is that the teximage demo uses a busy texture
> > which is performs an accelerated pixel conversion from the user's B5G6R5
> > into the native X8B8G8R8. After the patch, no conversion is required
> > allowing use of the blitter and memcpy fast paths.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  src/mesa/main/texformat.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
> > index fd9f335..866c7b3 100644
> > --- a/src/mesa/main/texformat.c
> > +++ b/src/mesa/main/texformat.c
> > @@ -114,6 +114,8 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
> >     case GL_RGB:
> >        if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
> >           RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
> > +      } else if (type == GL_UNSIGNED_SHORT_5_6_5) {
> > +         RETURN_IF_SUPPORTED(MESA_FORMAT_B5G6R5_UNORM);
> 
> Oh man... I hate this function.  It's such a big pile of hacks.  I think
> this change is good for now, but I think you're other idea is in the
> right direction.  This function really should check, "If internalFormat
> is a generic base format that matches <format,type>, is there supported
> format that matches <format,type> exactly?"

An issue that cropped in testing was glGenerateMipmap(). As this
switches to using B5G6R5 internally we also use that format for storing
the intermediate layers when generating the mipmap (thus incurring
quantisation errors at each step). What I have in mind to remedy this is
to convert the texture to floating point, generate the mipmap, then
convert back to the internal format. (However, I expect this also to
change test results!) Does that seem reasonable?

"The internal formats of the derived mipmap images all match those of
the levelbase image. The contents of the derived images are computed by
repeated, filtered reduction of the levelbase+1 image. For one- and
two-dimensional array and cube map array textures, each layer is
filtered independently."

Does not seem to exclude the use of resampling.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre