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:chr...@ijw.co.nz]
Sent: Thursday, July 03, 2014 3:19 AM
To: Popov, Pavel E
Cc: mesa-dev@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 <chr...@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.po...@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@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.po...@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.
/*
* Copyright © 2014 Intel Corporation
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice (including the next
* paragraph) shall be included in all copies or substantial portions of the
* Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*
* Author: Chris Forbes <chr...@ijw.co.nz>
*/
/**
* Tests format consistency for texture views, across all formats in each view
class.
* Based on the OpenGL 4.4 spec, section 8.26 "Texture Image Loads and Stores".
*
* Hardware will typically implement views by arranging for the memory layouts
to
* be trivially aliasable, but the spec is written in terms of conversions via
* scratch memory.
*
* This test ensures that whatever the hardware is doing is consistent with the
* specified conversions.
*/
#include "piglit-util-gl-common.h"
#include "common.h"
#include "view-classes.h"
PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 15;
config.supports_gl_core_version = 31;
config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
PIGLIT_GL_TEST_CONFIG_END
enum piglit_result
piglit_display(void)
{
return PIGLIT_FAIL;
}
bool
do_test(int bits, struct format_info *fmt)
{
GLuint tex;
bool pass = true;
/* reference pixel data -- up to 16 bytes */
char ref[] = {0x8f, 0x9e, 0x2d, 0x3c, 0x4b, 0x5a, 0x69, 0x78,
0x87, 0x96, 0xa5, 0xb4, 0xc3, 0xd2, 0xe1, 0xf0};
printf("Testing %d bits class:\n", bits);
for (; fmt->internalformat; fmt++) {
GLuint view;
char data[16];
int i;
glGenTextures(1, &tex);
glBindTexture(GL_TEXTURE_2D, tex);
glTexStorage2D(GL_TEXTURE_2D, 1, fmt->internalformat,
1, 1);
glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 1, 1,
fmt->pixelformat, fmt->pixeltype,
ref);
glGenTextures(1, &view);
glTextureView(view, GL_TEXTURE_2D, tex,
fmt->internalformat,
0, 1, 0, 1);
glBindTexture(GL_TEXTURE_2D, view);
glGetTexImage(GL_TEXTURE_2D, 0, fmt->pixelformat,
fmt->pixeltype, data);
if (memcmp(data, ref, bits >> 3)) {
piglit_report_subtest_result(PIGLIT_FAIL,
piglit_get_gl_enum_name(fmt->internalformat));
pass = false;
printf("expected: \n");
for (i = 0; i < bits >> 3; i++) {
printf("%02x ", ref[i]);
}
printf("\n");
printf("actual: \n");
for (i = 0; i < bits >> 3; i++) {
printf("%02x ", data[i]);
}
printf("\n");
}
else {
piglit_report_subtest_result(PIGLIT_PASS,
piglit_get_gl_enum_name(fmt->internalformat));
}
glDeleteTextures(1, &view);
glDeleteTextures(1, &tex);
}
return pass;
}
void
piglit_init(int argc, char **argv)
{
bool pass = true;
pass = do_test(8, view_class_8bits) && pass;
pass = do_test(16, view_class_16bits) && pass;
pass = do_test(24, view_class_24bits) && pass;
pass = do_test(32, view_class_32bits) && pass;
pass = do_test(48, view_class_48bits) && pass;
pass = do_test(64, view_class_64bits) && pass;
pass = do_test(96, view_class_96bits) && pass;
pass = do_test(128, view_class_128bits) && pass;
piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
}
format-consistency-get.patch
Description: format-consistency-get.patch
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
