Bug 92216

Summary:	[Regression, bisected] piglit gl-1.0-beginend-coverage, gl-1.0-no-op-paths, and gl-2.1-pbo broken
Product:	Mesa	Reporter:	Mark Janes <mark.a.janes>
Component:	Drivers/DRI/i965	Assignee:	Brian Paul <brianp>
Status:	CLOSED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	blocker
Priority:	medium	CC:	idr, lemody, mark.a.janes, vlee
Version:	git
Hardware:	x86-64 (AMD64)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	test output test output test output hack that fixes issue

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.