Bug 93717 - Meta mipmap generation can corrupt texture state
Summary: Meta mipmap generation can corrupt texture state
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Ian Romanick
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-14 20:33 UTC by Ian Romanick
Modified: 2016-01-25 18:45 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Ian Romanick 2016-01-14 20:33:40 UTC
At the end of _mesa_meta_GenerateMipmap, is the following sequence:

   _mesa_meta_end(ctx);

   _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, maxLevelSave);
   if (genMipmapSave)
      _mesa_TexParameteri(target, GL_GENERATE_MIPMAP, genMipmapSave);
   if (swizzleSaved)
      _mesa_TexParameteriv(target, GL_TEXTURE_SWIZZLE_RGBA, swizzle);

maxLevelSave, genMipmapSave, and swizzle all came from the texture object passed into the function.  This may not be the texture bound to ctx->Texture.CurrentUnit when the function is called!

We need a simple test case to verify this bug.

1. Create two textures, A and B, and fill each with some data.

2. Bind A and set GL_TEXTURE_MAX_LEVEL, GL_GENERATE_MIPMAP, and GL_TEXTURE_SWIZZLE_RGBA (assuming either GL_EXT_texture_swizzle or GL_ARB_texture_swizzle is supported).

3. While A is bound (but B is not bound), call glGenerateMipmaps on B.

4. Verify the GL_TEXTURE_MAX_LEVEL, GL_GENERATE_MIPMAP, and GL_TEXTURE_SWIZZLE_RGBA state of A.

It may be worth making this test generic so that other meta operations (and other texture state) could be tested in the future.
Comment 1 Ian Romanick 2016-01-21 00:53:14 UTC
I have sent out a piglit test that reproduces the problem.  It turns out that the only way to expose the problem is using glGenerateTextureMipmap.  glGenerateMipmap operates on the texture bound to the currently active texture unit.  The call to _mesa_meta_end restores all of that state, so the _mesa_TexParameter calls after that hit the correct texture (somewhat by luck).

http://patchwork.freedesktop.org/patch/71188/

I also have a Mesa patch that fixes the problem.
Comment 2 Ian Romanick 2016-01-25 18:45:56 UTC
Should be fixed by the following commit.

commit 2542871387393e855f6afe6c94d44611eefaf6eb
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Tue Jan 12 16:37:27 2016 -0800

    meta: Use internal functions to set texture parameters
    
    _mesa_texture_parameteriv is used because (the more obvious)
    _mesa_texture_parameteri just stuffs the parameter in an array and calls
    _mesa_texture_parameteriv.  This just cuts out the middleman.
    
    As a side bonus we no longer need check that ARB_stencil_texturing is
    supported.  The test doesn't allow non-supporting implementations to
    avoid any work, and it's redundant with the value-changed test.
    
    Fix bug #93717 because the state restore commands at the bottom of
    _mesa_meta_GenerateMipmap no longer depend on the bound state.
    
    Fixes  piglit   arb_direct_state_access-generatetexturemipmap  with  the
    changes  recently sent  to the  piglit mailing  list.  See  the bugzilla
    entry for more info.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93717
    Cc: "11.0 11.1" <mesa-stable@lists.freedesktop.org>
    Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.