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

[Ilia Mirkin]

On Thu, Sep 3, 2015 at 12:05 PM, Chris Wilson <chris at chris-wilson.co.uk> 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);
>        }

This seems right... any reason not to also throw in checks for
GL_UNSIGNED_SHORT_4_4_4_4_REV and GL_UNSIGNED_SHORT_1_5_5_5_REV?

Also why does RGB4 not check for that? Very odd. This whole function
is on the odd side wrt the list of formats it checks for. (A16 checks
for LA8 but not LA16? wtf?) Feels very much like "I know what the
drivers support, so I'll just check for those things" kind of
situation, which fails when you add a new driver.

Anyways, no reason to hold this patch up for all that. Just musing
about the imperfections we observe in life...

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

>        /* fallthrough */
>     case GL_RGB8:
> --
> 2.5.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev