Bug 79948 - [i965] Incorrect pixels when using discard and uniform loads
Summary: [i965] Incorrect pixels when using discard and uniform loads
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Cody Northrop
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 77449
  Show dependency treegraph
 
Reported: 2014-06-12 18:47 UTC by Cody Northrop
Modified: 2014-07-09 07:31 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to piglit test showing the discard problem. (5.20 KB, text/plain)
2014-06-12 18:47 UTC, Cody Northrop
Details
Proposed patch to fix the issue, widening the discard mask. (1.79 KB, patch)
2014-06-12 18:49 UTC, Cody Northrop
Details | Splinter Review
Proposed patch to fix the issue, widening the discard mask, v2 (1.89 KB, patch)
2014-06-12 19:34 UTC, Cody Northrop
Details | Splinter Review

Description Cody Northrop 2014-06-12 18:47:36 UTC
Created attachment 100932 [details]
Patch to piglit test showing the discard problem.

Commit 17c7ead7 exposed a bug in how uniform loading happens in the presence of discard.  It manifested itself in an application as randomly incorrect pixels on the borders of conditional areas.

We've boiled it down to a piglit test case, attached as a patch to existing test.  When you run it on HSW with mesa 10.2 or later, notice the white line along the diagonal axis.  That shouldn't be there.  The white is coming from a texture sample to make it easy to see the problem.  It should have been multiplied by the uniform values as the rest of the fragments were.  The key bits are in the second fragment shader.

I believe we've root caused it and I'll next attach a fix that I'll send to the mesa-dev list.
Comment 1 Cody Northrop 2014-06-12 18:49:38 UTC
Created attachment 100933 [details] [review]
Proposed patch to fix the issue, widening the discard mask.

Added a proposed fix.
Comment 2 Cody Northrop 2014-06-12 19:34:00 UTC
Created attachment 100938 [details] [review]
Proposed patch to fix the issue, widening the discard mask, v2

Updated version of patch.
Comment 3 Ian Romanick 2014-06-24 14:52:57 UTC
Patches are supposed to be sent to the mailing list for review.  Otherwise the people who should see and review them are unlikely to ever know the patches exist.
Comment 4 Cody Northrop 2014-06-24 15:16:57 UTC
Here is the email I sent out at the same time.

http://lists.freedesktop.org/archives/mesa-dev/2014-June/061338.html

I thought doing the bug first, followed by mail to the dev list referencing it would be most effective.  I guess I should have also added the archive link here.

-C
Comment 5 Ian Romanick 2014-06-24 15:21:32 UTC
(In reply to comment #4)
> Here is the email I sent out at the same time.
> 
> http://lists.freedesktop.org/archives/mesa-dev/2014-June/061338.html
> 
> I thought doing the bug first, followed by mail to the dev list referencing
> it would be most effective.  I guess I should have also added the archive
> link here.

Yes. :) I was just going to point out that the best-practice is to do something like bug #80115 comment #4.
Comment 6 Ian Romanick 2014-06-24 15:22:26 UTC
Also... if you're actively working on a bug, please assign it to yourself, and put it in the assigned state.
Comment 7 Ian Romanick 2014-06-24 17:14:44 UTC
Regarding the test case attachment #100932 [details], I can't see how that would even run, at all, on Mesa.  It tries to use a UBO in a compatibility profile without enabling GL_ARB_uniform_buffer_objects.

I think the right approach is to basically clone tests/spec/arb_uniform_buffer_object/rendering.c (call the new test rendering-discard.c) and modify it to hit the paths you want to hit.
Comment 8 Cody Northrop 2014-06-24 19:23:41 UTC
Yeah, the test case is just cobbled together to show the problem, using steps similar to the application that exposed it.

I'll follow up with a full test using your recommendation.

Thanks,

-C
Comment 9 Cody Northrop 2014-07-01 19:37:34 UTC
Okay, I resent the patch to mesa-dev with all requested changes incorporated:

https://www.mail-archive.com/mesa-dev@lists.freedesktop.org/msg61727.html

And I sent a full test (arb_uniform_buffer_object-rendering-discard) to the piglit dev list:

http://lists.freedesktop.org/archives/piglit/2014-July/011557.html


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.