Bug 76749

Created attachment 96559 [details]
dota2-world-lighting-expected.png

These lights should be shaded by spheres rendered using the deferred_simple_light shader. The reason I mention this is that the name of the shader should end up being included in the final GLSL source passed to the driver, so if you're looking at the command stream with apitrace/VOGL it should be fairly straightforward to isolate one problem draw call using that information.

This is also reproducible on SNB.  Several other (probably unrelated) issues are noticed on SNB when replaying the trace file from which the snapshots were taken.

commit 9630ba6c6e754b438cf67c7d76ec1c99488df3ba
Author: Matt Turner <mattst88@gmail.com>
Date:   Tue Mar 11 13:14:08 2014 -0700

    i965/vec4: Let dead code eliminate trim dead channels.

causes an assertion failure:

glretrace: brw_vec4_generator.cpp:184: void brw::vec4_generator::generate_math1_gen6(brw::vec4_instruction*, brw_reg, brw_reg): Assertion `dst.dw1.bits.writemask == 0xf' failed.

commit dc0f5099fa3cb564c25eb892fde93cacd29df8f1
Author: Matt Turner <mattst88@gmail.com>
Date:   Tue Mar 11 13:06:20 2014 -0700

    i965/vec4: Track live ranges per-channel, not per vgrf.

causes another assertion failure:

glretrace: brw_vec4.cpp:346: bool brw::vec4_visitor::dead_code_eliminate(): Assertion `this->virtual_grf_end[inst->dst.reg] >= pc' failed.

Finally,

commit 947c828d5cbffe9640ac63103a6223112eeff27f
Author: Matt Turner <mattst88@gmail.com>
Date:   Thu Dec 12 00:30:16 2013 -0800

    i965/fs: Add a saturation propagation optimization pass.

causes more lights to have no effect.

Moved to HSW.  Noticed another rendering issue introduced by

commit 9cd51bb0c4608258199c69bc7738e72f055799d2
Author: Matt Turner <mattst88@gmail.com>
Date:   Tue Mar 11 13:16:37 2014 -0700

    i965/vec4: Eliminate writes that are never read.

It does not seem to be related to this bug either.

But I can confirm that the deferred_simple_light shader is the source of the bug.  I had a look at the FS part of the shader.  The lighting effect can be observed when I replaced "oT1", which is the varying for the light direction AFAICT, by a fixed value.  It is possible that the VS part is miscompiled, giving FS wrong light direction.  I will dig more tomorrow.

i have problem whith my texture after updating linux karnel 3.14 i m runing mint 16 

http://imgur.com/5pCfzOn
http://imgur.com/1si6T86
http://imgur.com/UrBpTfy
http://imgur.com/IQc2Txl

any help for me i think that is rendering mode problem ?

Graphics: Card: Intel 2nd Generation Core Processor Family Integrated Graphics Controller 
X.Org: 1.14.5 drivers: intel (unloaded: fbdev,vesa) Resolution: 1360x768@59.8hz 
GLX Renderer: Mesa DRI Intel Sandybridge Mobile GLX Version: 3.0 Mesa 10.2.0-devel


OpenGL vendor string: Intel Open Source Technology Center
OpenGL renderer string: Mesa DRI Intel(R) Sandybridge Mobile 
OpenGL core profile version string: 3.1 (Core Profile) Mesa 10.2.0-devel
OpenGL core profile shading language version string: 1.40
OpenGL core profile context flags: (none)
OpenGL core profile extensions:
OpenGL version string: 3.0 Mesa 10.2.0-devel
OpenGL shading language version string: 1.30
OpenGL context flags: (none)
OpenGL extensions:
[reply] [-] Comment 10 domagoj 2014-04-01 14:43:38 UTC
(In reply to comment #7)
> Jordan, I don't think we have independent dithering controls for RGB vs. A.
> Just disabling it altogether for 1010102 seems reasonable to me.




i have problem whith my texture after updating linux karnel 3.14 i m runing mint 16 

http://imgur.com/5pCfzOn
http://imgur.com/1si6T86
http://imgur.com/UrBpTfy
http://imgur.com/IQc2Txl

any help for me i think that is rendering mode problem ?

Graphics: Card: Intel 2nd Generation Core Processor Family Integrated Graphics Controller 
X.Org: 1.14.5 drivers: intel (unloaded: fbdev,vesa) Resolution: 1360x768@59.8hz 
GLX Renderer: Mesa DRI Intel Sandybridge Mobile GLX Version: 3.0 Mesa 10.2.0-devel


OpenGL vendor string: Intel Open Source Technology Center
OpenGL renderer string: Mesa DRI Intel(R) Sandybridge Mobile 
OpenGL core profile version string: 3.1 (Core Profile) Mesa 10.2.0-devel
OpenGL core profile shading language version string: 1.40
OpenGL core profile context flags: (none)
OpenGL core profile extensions:
OpenGL version string: 3.0 Mesa 10.2.0-devel
OpenGL shading language version string: 1.30
OpenGL context flags: (none)
OpenGL extensions:
Add Comment

(In reply to comment #3)
> This is also reproducible on SNB.  Several other (probably unrelated) issues
> are noticed on SNB when replaying the trace file from which the snapshots
> were taken.
> 
> commit 9630ba6c6e754b438cf67c7d76ec1c99488df3ba
> Author: Matt Turner <mattst88@gmail.com>
> Date:   Tue Mar 11 13:14:08 2014 -0700
> 
>     i965/vec4: Let dead code eliminate trim dead channels.
> 
> causes an assertion failure:
> 
> glretrace: brw_vec4_generator.cpp:184: void
> brw::vec4_generator::generate_math1_gen6(brw::vec4_instruction*, brw_reg,
> brw_reg): Assertion `dst.dw1.bits.writemask == 0xf' failed.
> 
> commit dc0f5099fa3cb564c25eb892fde93cacd29df8f1
> Author: Matt Turner <mattst88@gmail.com>
> Date:   Tue Mar 11 13:06:20 2014 -0700
> 
>     i965/vec4: Track live ranges per-channel, not per vgrf.
> 
> causes another assertion failure:
> 
> glretrace: brw_vec4.cpp:346: bool brw::vec4_visitor::dead_code_eliminate():
> Assertion `this->virtual_grf_end[inst->dst.reg] >= pc' failed.

There are bugs for these that I'm working on now.

> Finally,
> 
> commit 947c828d5cbffe9640ac63103a6223112eeff27f
> Author: Matt Turner <mattst88@gmail.com>
> Date:   Thu Dec 12 00:30:16 2013 -0800
> 
>     i965/fs: Add a saturation propagation optimization pass.
> 
> causes more lights to have no effect.

Ken made a fix to this pass, commit 4d2e79269a. Does that fix this bit?

(In reply to comment #6)
> (In reply to comment #3)
> > This is also reproducible on SNB.  Several other (probably unrelated) issues
> > are noticed on SNB when replaying the trace file from which the snapshots
> > were taken.
> > 
> > commit 9630ba6c6e754b438cf67c7d76ec1c99488df3ba
> > Author: Matt Turner <mattst88@gmail.com>
> > Date:   Tue Mar 11 13:14:08 2014 -0700
> > 
> >     i965/vec4: Let dead code eliminate trim dead channels.
> > 
> > causes an assertion failure:
> > 
> > glretrace: brw_vec4_generator.cpp:184: void
> > brw::vec4_generator::generate_math1_gen6(brw::vec4_instruction*, brw_reg,
> > brw_reg): Assertion `dst.dw1.bits.writemask == 0xf' failed.
> > 
> > commit dc0f5099fa3cb564c25eb892fde93cacd29df8f1
> > Author: Matt Turner <mattst88@gmail.com>
> > Date:   Tue Mar 11 13:06:20 2014 -0700
> > 
> >     i965/vec4: Track live ranges per-channel, not per vgrf.
> > 
> > causes another assertion failure:
> > 
> > glretrace: brw_vec4.cpp:346: bool brw::vec4_visitor::dead_code_eliminate():
> > Assertion `this->virtual_grf_end[inst->dst.reg] >= pc' failed.
> 
> There are bugs for these that I'm working on now.
> 
> > Finally,
> > 
> > commit 947c828d5cbffe9640ac63103a6223112eeff27f
> > Author: Matt Turner <mattst88@gmail.com>
> > Date:   Thu Dec 12 00:30:16 2013 -0800
> > 
> >     i965/fs: Add a saturation propagation optimization pass.
> > 
> > causes more lights to have no effect.
> 
> Ken made a fix to this pass, commit 4d2e79269a. Does that fix this bit?
Nope.  With mesa master, the spotlight on the heroes is still missing unless I comment out fs_visitor::opt_saturate_propagation.

The other commit, 9cd51bb0c4608258199c69bc7738e72f055799d2, causes the ground to be rendered with a random pattern.  It looks like the commit incorrectly eliminates some writes whose results are actually read.

Those comments are notes to myself while looking into this specific bug.  I planned to revisit them after this bug is fixed.  It would be great if you are interested too.  The trace file in question can be found at people.freedesktop.org:~cworth/dota2-jordan-trim.trace.  I can file bug reports if that is preferred.

(In reply to comment #5)
> i have problem whith my texture after updating linux karnel 3.14 i m runing
> mint 16 
> 
> http://imgur.com/5pCfzOn
> http://imgur.com/1si6T86
> http://imgur.com/UrBpTfy
> http://imgur.com/IQc2Txl
> 
> any help for me i think that is rendering mode problem ?
I can confirm the problem, and I think it is introduced by commit 9cd51bb0c4608258199c69bc7738e72f055799d2.

It is unrelated to this bug though.  Do you mind filing another bug, with me and Matt CCed?

(In reply to comment #4)
> Moved to HSW.  Noticed another rendering issue introduced by
> 
> commit 9cd51bb0c4608258199c69bc7738e72f055799d2
> Author: Matt Turner <mattst88@gmail.com>
> Date:   Tue Mar 11 13:16:37 2014 -0700
> 
>     i965/vec4: Eliminate writes that are never read.
> 
> It does not seem to be related to this bug either.
> 
> But I can confirm that the deferred_simple_light shader is the source of the
> bug.  I had a look at the FS part of the shader.  The lighting effect can be
> observed when I replaced "oT1", which is the varying for the light direction
> AFAICT, by a fixed value.  It is possible that the VS part is miscompiled,
> giving FS wrong light direction.  I will dig more tomorrow.
It looks like the vertex shader is miscompiled.  I was deciphering the vertex shader by giving variables meaningful names instead of (re)using r0, r1, r2 for all kinds of different purposes, and suddenly the lighting is correctly rendered.  Could be a bug in the VS instruction scheduler or somewhere else.

Created attachment 96764 [details] [review]
fix vs copy propagation

This patch should fix this bug.  However, there appear to be two other rendering issues introduced by commit 947c828d5cbffe9640ac63103a6223112eeff27f and commit 9cd51bb0c4608258199c69bc7738e72f055799d2 respectively.  The latter commit in particular prevents this patch from working correctly.  I will need to look into why before posting the patch to the list for review.

(In reply to comment #10)
> Created attachment 96764 [details] [review] [review]
> fix vs copy propagation
> 
> This patch should fix this bug.  However, there appear to be two other
> rendering issues introduced by commit
> 947c828d5cbffe9640ac63103a6223112eeff27f and commit
> 9cd51bb0c4608258199c69bc7738e72f055799d2 respectively.  The latter commit in
> particular prevents this patch from working correctly.  I will need to look
> into why before posting the patch to the list for review.

See "i965/vec4: Consider sources of non-GRF-dst instructions for dead channels." awaiting review, and https://bugs.freedesktop.org/show_bug.cgi?id=76616

I'll see if I can spot the problem with the saturation propagation pass. I can reproduce the issue with Jordan's trace.

(In reply to comment #11)
> I'll see if I can spot the problem with the saturation propagation pass. I
> can reproduce the issue with Jordan's trace.

I think I've found the shader that it's incorrectly optimizing.

(In reply to comment #12)
> (In reply to comment #11)
> > I'll see if I can spot the problem with the saturation propagation pass. I
> > can reproduce the issue with Jordan's trace.
> 
> I think I've found the shader that it's incorrectly optimizing.

I've sent a three patch series to fix some bugs in the saturate propagation pass. It fixes some lighting issues (the whole scene looked too dark before), but doesn't affect the issue Jordan's pngs show.

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > I'll see if I can spot the problem with the saturation propagation pass. I
> > > can reproduce the issue with Jordan's trace.
> > 
> > I think I've found the shader that it's incorrectly optimizing.
> 
> I've sent a three patch series to fix some bugs in the saturate propagation
> pass. It fixes some lighting issues (the whole scene looked too dark
> before), but doesn't affect the issue Jordan's pngs show.
The patch in the attachments should fix this bug.

I am on holidays, and haven't had a chance to look at the patches for VS DCE and saturation propagation.  I will take a look once I get back, of course unless they have been committed by then.

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > I'll see if I can spot the problem with the saturation propagation pass. I
> > > can reproduce the issue with Jordan's trace.
> > 
> > I think I've found the shader that it's incorrectly optimizing.
> 
> I've sent a three patch series to fix some bugs in the saturate propagation
> pass. It fixes some lighting issues (the whole scene looked too dark
> before), but doesn't affect the issue Jordan's pngs show.

These patches are now upstream.

Fix committed and I can no longer reproduce it with the trace file.