[Mesa-dev] [PATCH] mesa: Fix regression introduced by commit "mesa: fix packing of float texels to GL_SHORT/GL_BYTE".

Popov, Pavel E pavel.e.popov at intel.com
Mon Jul 7 21:02:08 PDT 2014


Hi Chris,

I found your new piglit tests for arb_texture_view (format-consistency-get and format-consistency-render) and checked that all of them pass with my patch.
Also I slightly modified test format-consistency-get and now it fails without patch and demonstrates the regression. The test and patch are attached.

I changed the first two elements in 'ref' array. You can just swap 2 lines which define this array.
Also I moved downloading texture image part inside the loop. For some reason other cases somehow affect 'GL_RGB8_SNORM' and 'GL_RGB16_SNORM' and they pass without these modification.

Could you commit this patch and update your test? I think we can do this without any problems because macros *_TO_FLOAT_TEX preserve zero like *_TO_FLOATZ. 

Output. These 2 cases fail without patch:
PIGLIT:subtest {'GL_RGB8_SNORM' : 'fail'}
expected:
ffffff8f ffffff9e 2d
actual:
ffffff90 ffffff9f 2d

PIGLIT:subtest {'GL_RGB16_SNORM' : 'fail'}
expected:
ffffff8f ffffff9e 2d 3c 4b 5a
actual:
ffffff90 ffffff9e 2d 3c 4b 5a

Regards,
Pavel

-----Original Message-----
From: Chris Forbes [mailto:chrisf at ijw.co.nz] 
Sent: Thursday, July 03, 2014 3:19 AM
To: Popov, Pavel E
Cc: mesa-dev at lists.freedesktop.org
Subject: Re: [PATCH] mesa: Fix regression introduced by commit "mesa: fix packing of float texels to GL_SHORT/GL_BYTE".

Assuming this causes no piglit regressions,

Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>

Can we get some piglits which demonstrate these problems? oglconform is too secret.

On Wed, Jul 2, 2014 at 10:54 PM, Popov, Pavel E <pavel.e.popov at intel.com> wrote:
> Hi Chris,
>
> Could you review this patch?
>
> Probably a better solution here is to use only one type of conversion formulas to avoid issues like this. We can remove old formulas ("BYTE_TO_FLOAT", "FLOAT_TO_BYTE", etc.) and use only formulas which were recently added in spec ("BYTE_TO_FLOAT_TEX", "FLOAT_TO_BYTE_TEXT", etc.). But now I just replaced *_TO_FLOATZ in function extract_float_rgba to *_TO_FLOAT_TEX to fix oglconform regressions.
>
> Regards,
> Pavel
>
> -----Original Message-----
> From: Popov, Pavel E
> Sent: Monday, June 30, 2014 10:22 PM
> To: mesa-dev at lists.freedesktop.org
> Cc: Popov, Pavel E
> Subject: [PATCH] mesa: Fix regression introduced by commit "mesa: fix packing of float texels to GL_SHORT/GL_BYTE".
>
> This commit "mesa: fix packing of float texels to GL_SHORT/GL_BYTE" replaced *_TO_BYTE to *_TO_BYTE_TEX because *_TO_FLOAT_TEX are used to unpack the texels to floats.
> In this case *_TO_FLOATZ in function extract_float_rgba also should be replaced to *_TO_FLOAT_TEX. Underline that these macros automatically preserve zero when converting.
>
> The regression was observed on 3 oglconform tests:
>     snorm-textures basic.getTexImage
>     snorm-textures advanced.mipmap.manual.getTex
>     snorm-textures advanced.mipmap.upload.getTex
>
> Signed-off-by: Pavel Popov <pavel.e.popov at intel.com>
> ---
>  src/mesa/main/pack.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c index 
> 1df6568..70c8b93 100644
> --- a/src/mesa/main/pack.c
> +++ b/src/mesa/main/pack.c
> @@ -3253,10 +3253,10 @@ extract_float_rgba(GLuint n, GLfloat rgba[][4],
>           PROCESS(aSrc, ACOMP, 1.0F, 255, GLubyte, UBYTE_TO_FLOAT);
>           break;
>        case GL_BYTE:
> -         PROCESS(rSrc, RCOMP, 0.0F,   0, GLbyte, BYTE_TO_FLOATZ);
> -         PROCESS(gSrc, GCOMP, 0.0F,   0, GLbyte, BYTE_TO_FLOATZ);
> -         PROCESS(bSrc, BCOMP, 0.0F,   0, GLbyte, BYTE_TO_FLOATZ);
> -         PROCESS(aSrc, ACOMP, 1.0F, 127, GLbyte, BYTE_TO_FLOATZ);
> +         PROCESS(rSrc, RCOMP, 0.0F,   0, GLbyte, BYTE_TO_FLOAT_TEX);
> +         PROCESS(gSrc, GCOMP, 0.0F,   0, GLbyte, BYTE_TO_FLOAT_TEX);
> +         PROCESS(bSrc, BCOMP, 0.0F,   0, GLbyte, BYTE_TO_FLOAT_TEX);
> +         PROCESS(aSrc, ACOMP, 1.0F, 127, GLbyte, BYTE_TO_FLOAT_TEX);
>           break;
>        case GL_UNSIGNED_SHORT:
>           PROCESS(rSrc, RCOMP, 0.0F,      0, GLushort, USHORT_TO_FLOAT);
> @@ -3265,10 +3265,10 @@ extract_float_rgba(GLuint n, GLfloat rgba[][4],
>           PROCESS(aSrc, ACOMP, 1.0F, 0xffff, GLushort, USHORT_TO_FLOAT);
>           break;
>        case GL_SHORT:
> -         PROCESS(rSrc, RCOMP, 0.0F,     0, GLshort, SHORT_TO_FLOATZ);
> -         PROCESS(gSrc, GCOMP, 0.0F,     0, GLshort, SHORT_TO_FLOATZ);
> -         PROCESS(bSrc, BCOMP, 0.0F,     0, GLshort, SHORT_TO_FLOATZ);
> -         PROCESS(aSrc, ACOMP, 1.0F, 32767, GLshort, SHORT_TO_FLOATZ);
> +         PROCESS(rSrc, RCOMP, 0.0F,     0, GLshort, SHORT_TO_FLOAT_TEX);
> +         PROCESS(gSrc, GCOMP, 0.0F,     0, GLshort, SHORT_TO_FLOAT_TEX);
> +         PROCESS(bSrc, BCOMP, 0.0F,     0, GLshort, SHORT_TO_FLOAT_TEX);
> +         PROCESS(aSrc, ACOMP, 1.0F, 32767, GLshort, 
> + SHORT_TO_FLOAT_TEX);
>           break;
>        case GL_UNSIGNED_INT:
>           PROCESS(rSrc, RCOMP, 0.0F,          0, GLuint, UINT_TO_FLOAT);
> --
> 1.9.1
>
>
> --------------------------------------------------------------------
> Closed Joint Stock Company Intel A/O
> Registered legal address: Krylatsky Hills Business Park,
> 17 Krylatskaya Str., Bldg 4, Moscow 121614, Russian Federation
>
> This e-mail and any attachments may contain confidential material for 
> the sole use of the intended recipient(s). Any review or distribution 
> by others is strictly prohibited. If you are not the intended 
> recipient, please contact the sender and delete all copies.
>

--------------------------------------------------------------------
Closed Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park, 
17 Krylatskaya Str., Bldg 4, Moscow 121614, 
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: format-consistency-get.c
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140708/44cee8e6/attachment-0001.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: format-consistency-get.patch
Type: application/octet-stream
Size: 1231 bytes
Desc: format-consistency-get.patch
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140708/44cee8e6/attachment-0001.obj>


More information about the mesa-dev mailing list