Bug 91425 - [regression, bisected] Piglit spec/ext_packed_float/ getteximage-invalid-format-for-packed-type fails
Summary: [regression, bisected] Piglit spec/ext_packed_float/ getteximage-invalid-form...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-22 08:30 UTC by Samuel Iglesias
Modified: 2015-07-23 05:26 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Iglesias 2015-07-22 08:30:17 UTC
I found a piglit regression when testing today's master:

$ bin/getteximage-invalid-format-for-packed-type -auto -fbo
Mesa: User error: GL_INVALID_VALUE in glGetTextImage(depth = 0)
Unexpected GL error: GL_INVALID_VALUE 0x501
(Error at /home/siglesias/devel/piglit/tests/spec/ext_packed_float/getteximage-invalid-format-for-packed-type.c:124)
Expected GL error: GL_INVALID_OPERATION 0x502
PIGLIT: {"subtest": {"GL_UNSIGNED_BYTE_3_3_2, GL_RGBA" : "fail"}}
Mesa: User error: GL_INVALID_VALUE in glGetTextImage(depth = 0)
Unexpected GL error: GL_INVALID_VALUE 0x501
(Error at /home/siglesias/devel/piglit/tests/spec/ext_packed_float/getteximage-invalid-format-for-packed-type.c:118)
PIGLIT: {"subtest": {"GL_UNSIGNED_BYTE_3_3_2, GL_RGB" : "fail"}}
Mesa: User error: GL_INVALID_VALUE in glGetTextImage(depth = 0)
Unexpected GL error: GL_INVALID_VALUE 0x501
(Error at /home/siglesias/devel/piglit/tests/spec/ext_packed_float/getteximage-invalid-format-for-packed-type.c:124)
Expected GL error: GL_INVALID_OPERATION 0x502
PIGLIT: {"subtest": {"GL_UNSIGNED_BYTE_3_3_2, GL_RED" : "fail"}}
Mesa: User error: GL_INVALID_VALUE in glGetTextImage(depth = 0)
Unexpected GL error: GL_INVALID_VALUE 0x501
(Error at /home/siglesias/devel/piglit/tests/spec/ext_packed_float/getteximage-invalid-format-for-packed-type.c:124)
[...]

I bisected it and found the first bad commit:

---

f20cfc5a409b69d5ae3d10a870d90e0b4e493ddf is the first bad commit
commit f20cfc5a409b69d5ae3d10a870d90e0b4e493ddf
Author: Brian Paul <brianp@vmware.com>
Date:   Tue Jul 21 18:35:38 2015 -0600

    mesa: overhaul the glGetTexImage code
    
    1. Reorganize the error checking code.
    2. Lay groundwork for getting sub images by passing image offset and
       dimensions to the error checking code.
    3. Implement _mesa_GetnTexImageARB(), _mesa_GetTexImage() and
       _mesa_GetTextureImage() all in terms of get_texture_image().
    
    v2: pass offset/width/height/depth arguments to the error checking
    function, avoid using magic width/height/depth values.
    v3: remove unused bufSize param to get_texture_image()
    
    Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>

---

Mesa master HEAD: 800efb0690e962750b9a072bcbab279fdaae24a1

$ glxinfo | grep git
OpenGL core profile version string: 3.3 (Core Profile) Mesa 10.7.0-devel (git-800efb0)
OpenGL version string: 3.0 Mesa 10.7.0-devel (git-800efb0)
OpenGL ES profile version string: OpenGL ES 3.0 Mesa 10.7.0-devel (git-800efb0)
Comment 1 Brian Paul 2015-07-22 14:16:29 UTC
Hmm, I can't reproduce that.  The test passes completely for me.

What driver are you using?  I'm testing llvmpipe/softpipe.
Comment 2 Michel Dänzer 2015-07-22 14:47:19 UTC
FWIW, I got the same regression with radeonsi.
Comment 3 Ilia Mirkin 2015-07-22 16:30:26 UTC
(In reply to Brian Paul from comment #1)
> Hmm, I can't reproduce that.  The test passes completely for me.
> 
> What driver are you using?  I'm testing llvmpipe/softpipe.

Odd. I repro with llvmpipe. I guess the issue is the

      if (depth != 1) {
         _mesa_error(ctx, GL_INVALID_VALUE,
                     "%s(depth = %d)", caller, depth);
         return true;
      }

check for 1d/2d targets. The piglit test does:

glGetTexImage(GL_TEXTURE_2D, 0, format, type, pxBuffer);

Since it's checking for, roughly speaking, invalid things, I assume that the texImage isn't there, and so get_texture_image_dims will end up setting a depth = 0. Should that be depth = 1 instead?
Comment 4 Brian Paul 2015-07-22 17:13:38 UTC
OK, the problem is my local copy of getteximage-invalid-format-for-packed-type.c has the patch "ext_packed_float: fix getteximage-invalid-format-for-packed-type test" which I posted for review (and was R-b'd by Jose) but not yet pushed to master.

I believe the piglit test was defective as-is, per my check-in comment:

    The GL spec doesn't explicitly say that glGetTexImage should generate
    GL_INVALID_OPERATION when attempting to retrieve a non-existant texture
    image, but that's what NVIDIA's driver does.
    
    The purpose of this test is to check the format/type parameters, so let's
    define a packed float texture to avoid the undefined texture situation.
    
    Test now passes with NVIDIA.

I'll push that patch.
Comment 5 Samuel Iglesias 2015-07-23 05:26:44 UTC
(In reply to Brian Paul from comment #4)
> OK, the problem is my local copy of
> getteximage-invalid-format-for-packed-type.c has the patch
> "ext_packed_float: fix getteximage-invalid-format-for-packed-type test"
> which I posted for review (and was R-b'd by Jose) but not yet pushed to
> master.
> 
> I believe the piglit test was defective as-is, per my check-in comment:
> 
>     The GL spec doesn't explicitly say that glGetTexImage should generate
>     GL_INVALID_OPERATION when attempting to retrieve a non-existant texture
>     image, but that's what NVIDIA's driver does.
>     
>     The purpose of this test is to check the format/type parameters, so let's
>     define a packed float texture to avoid the undefined texture situation.
>     
>     Test now passes with NVIDIA.
> 
> I'll push that patch.

It passes now. Thanks Brian!


bug/show.html.tmpl processed on Jan 24, 2017 at 07:03:08.
(provided by the Example extension).