On Thu, Sep 03, 2015 at 12:15:50PM -0400, Ilia Mirkin wrote:
> On Thu, Sep 3, 2015 at 12:05 PM, Chris Wilson <ch...@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 <ch...@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...
What I tried first was to use _mesa_format_from_format_and_type() and
try and check that the it matched the internal format.
Something like:
if (_mesa_format_is_unsized(internalFormat)) {
mesa_format tex_format = _mesa_format_from_format_and_type(format, type);
if (_mesa_base_tex_format(ctx, internalFormat) ==
_mesa_get_format_base_format(tex_format)) {
if (_mesa_format_is_mesa_array_format(tex_format))
tex_format = _mesa_format_from_array_format(tex_format);
if (tex_format < MESA_FORMAT_COUNT)
RETURN_IF_SUPPORTED(tex_format);
}
}
but I got lost trying to work out if that prevented all possible
unwanted/illegal conversions (e.g. sRGB, even whether the source pixel is
linear or sRGB was unclear). Hence just adding the one type and
ignoring the rest was much safer :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
