Bug 92216 - [Regression, bisected] piglit gl-1.0-beginend-coverage, gl-1.0-no-op-paths, and gl-2.1-pbo broken
Summary: [Regression, bisected] piglit gl-1.0-beginend-coverage, gl-1.0-no-op-paths, a...
Status: CLOSED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium blocker
Assignee: Brian Paul
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
: 92231 92263 92325 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-10-01 17:31 UTC by Mark Janes
Modified: 2015-10-08 21:55 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
test output (107.24 KB, text/plain)
2015-10-01 17:31 UTC, Mark Janes
Details
test output (3.79 KB, text/plain)
2015-10-01 17:32 UTC, Mark Janes
Details
test output (2.19 KB, text/plain)
2015-10-01 17:32 UTC, Mark Janes
Details
hack that fixes issue (474 bytes, patch)
2015-10-02 05:36 UTC, Tapani Pälli
Details | Splinter Review

Description Mark Janes 2015-10-01 17:31:39 UTC
Created attachment 118568 [details]
test output

On all tested platforms, the following failures have been bisected to a single commit:

piglit.spec.!opengl 2_1.pbo
piglit.spec.!opengl 1_0.gl-1.0-beginend-coverage

Additionally, the same commit regressed the following test ONLY on 32 bit platforms:
piglit.spec.!opengl 1_0.gl-1.0-no-op-paths

The problematic commit:

Author: Brian Paul <brianp@vmware.com>
Date:   Wed Sep 30 11:29:13 2015 -0600

    mesa: consolidate texture binding code
    
    Before, we were doing the actual _mesa_reference_texobj() call and
    ctx->Driver.BindTexture() and misc housekeeping in three different
    places.  This consolidates the common code in a new bind_texture()
    function.
    
    Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

Output for each test saved in attachments.
Comment 1 Mark Janes 2015-10-01 17:32:31 UTC
Created attachment 118569 [details]
test output
Comment 2 Mark Janes 2015-10-01 17:32:49 UTC
Created attachment 118570 [details]
test output
Comment 3 Tapani Pälli 2015-10-02 05:00:00 UTC
It is _mesa_BindTexture that is broken, I can't quite see why though. Will try to debug.
Comment 4 Tapani Pälli 2015-10-02 05:36:15 UTC
Created attachment 118583 [details] [review]
hack that fixes issue

The bug always occurs whenever default texture object get used, it's textureIndex is always 0 so textureIndex used in new bind_texture function is incorrect. Attached patch fixes the issue, maybe real fix would be to add index as one of the parameters, what do you think Brian?
Comment 5 Tapani Pälli 2015-10-02 05:41:00 UTC
*** Bug 92231 has been marked as a duplicate of this bug. ***
Comment 6 Mark Janes 2015-10-02 14:34:09 UTC
The hack does not fix the issue on g33, fwiw
Comment 7 Tapani Pälli 2015-10-03 02:18:16 UTC
*** Bug 92263 has been marked as a duplicate of this bug. ***
Comment 8 Brian Paul 2015-10-03 13:08:44 UTC
(In reply to Tapani Pälli from comment #4)
> Created attachment 118583 [details] [review] [review]
> hack that fixes issue
> 
> The bug always occurs whenever default texture object get used, it's
> textureIndex is always 0 so textureIndex used in new bind_texture function
> is incorrect. Attached patch fixes the issue, maybe real fix would be to add
> index as one of the parameters, what do you think Brian?

I can't reproduce the regressions on a 64-bit nor 32-bit host with llvmpipe.  I did full piglit runs before/after my patch series and there were no regressions (modulo the BindTextureUnit expected error code).

Has anyone else been able to repro??

In any case, we should initialize the default textures' TargetIndex fields but I think a different patch/approach would be better.  I'll post a patch for that in a bit.
Comment 9 Brian Paul 2015-10-05 18:20:42 UTC
I just posted a patch series to mesa-dev.  The 3rd patch changes the TextureTarget initialization code and covers the case which the "hack that fixes the issue" did.

I have no idea, though, if this will help with the regression that Mark hit.
Comment 10 Mark Janes 2015-10-05 23:18:05 UTC
I tested this patch and found:

regressions:
  spec.arb_shader_storage_buffer_object.layout-std140-write-shader  (BDW only)
    expected[1] = 1.00. Read value: 0.00

  spec.arb_copy_image.arb_copy_image-srgb-copy  (assertion)
    arb_copy_image-srgb-copy: src/mesa/main/texobj.c:1739:
      _mesa_BindTexture: Assertion `newTexObj->TargetIndex == targetIndex'
      failed.

fixes:
  spec.!opengl 1_0.gl-1.0-beginend-coverage
  spec.!opengl 1_0.gl-1.0-no-op-paths
  spec.arb_copy_image.arb_copy_image-formats
Comment 11 Mark Janes 2015-10-07 21:32:07 UTC
*** Bug 92325 has been marked as a duplicate of this bug. ***
Comment 12 Mark Janes 2015-10-08 21:55:44 UTC
fixed by mesa 7d7dd18


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.