Bug 34218

Summary:	[r300g] Unigine: some surfaces are reflecting too much light
Product:	Mesa	Reporter:	Pavel Ondračka <pavel.ondracka>
Component:	Drivers/Gallium/r300	Assignee:	Default DRI bug account <dri-devel>
Status:	RESOLVED FIXED	QA Contact:
Severity:	minor
Priority:	medium	CC:	hverbeet
Version:	git
Hardware:	x86 (IA32)
OS:	Linux (All)
URL:	http://unigine.com/download/#sanctuary
Whiteboard:
i915 platform:		i915 features:
Attachments:	screenshot screenshot before 68b701f5ded5f6b73a6b07cf37d862fab4836607

Description Pavel Ondračka 2011-02-12 14:22:06 UTC

Created attachment 43305 [details]
screenshot

Another bug I've found during my recent Sanctuary testing. Some surfaces are way too much bright. This is a regression:

commit 68b701f5ded5f6b73a6b07cf37d862fab4836607
Author: Tom Stellard <tstellar@gmail.com>
Date:   Sat Feb 5 22:39:58 2011 -0800

    r300/compiler: Disable register rename pass on r500
    
    The scheduler and the register allocator are not good enough yet to deal
    with the effects of the register rename pass.  This was causing a 50%
    performance drop in Lightsmark.  The pass can be re-enabled once the
    scheduler and the register allocator are more mature.  r300 and r400
    still need this pass, because it prevents a lot of shaders from using
    too many texture indirections.
    
    NOTE: This is a candidate for the 7.10 branch.

BTW commit fixed shadows in Sanctuary but now it seems it simultaneously broke another thing. I haven't seen this when testing the shadows because this is only visible with shaders quality high. I've tried going back before register rename pass was introduce to see if this is similar to bug 34030, however this bug was also present there, it just wasn't much visible since shadows were broken at that time.

RADEON_DEBUG=noopt fixes this particular problem, but there are some black glitches when noopt is used and the wireframe problem is also much more visible.

Sanctuary version: 2.3
shaders quality: high (low and medium works fine)
all other setting set to lowest possible or disabled
mesa: a6b7393eb8b4ef14c0d9ba8d64e57ed8ca82a9f7
GPU: RV530

So far I've done all my testing with almost lowest settings, but now when menus are fixed it is finally possible to easily change settings and enable different options and of course most of them doesn't work (like paralax mapping, translucence, HDR, etc.) should I open a new separate bug for each one broken?

Comment 1 Pavel Ondračka 2011-02-12 14:23:18 UTC

Created attachment 43306 [details]
screenshot before 68b701f5ded5f6b73a6b07cf37d862fab4836607

Comment 2 Marek Olšák 2011-02-12 14:26:16 UTC

Could you please also test with softpipe/llvmpipe/swrast/an intel driver or anything else non-r300 to rule out a GLSL compiler issue?

Comment 3 Pavel Ondračka 2011-02-12 14:55:40 UTC

(In reply to comment #2)
> Could you please also test with softpipe/llvmpipe/swrast/an intel driver or
> anything else non-r300 to rule out a GLSL compiler issue?

Both softpipe and llvmpipe doesn't work with Sanctuary here, but I have also one Nvidia card so I'll try nouveau tomorrow.

Comment 4 Tobias Jakobi 2011-02-12 15:47:38 UTC

Is Sanctuary supposed to work on i965 hardware?

Comment 5 Pavel Ondračka 2011-02-13 04:01:29 UTC

llvmpipe works fine, but you need to set MESA_EXTENSION_OVERRIDE="-GL_EXT_texture_array -GL_ARB_draw_instanced" for Sanctuary to run properly and use the same extensions as with r300g.

Comment 6 Tom Stellard 2011-04-18 22:04:20 UTC

Can you try this again with the latest git version of mesa (commit ffc1d166d24532aeaa4dcf06a431e43ab7e7e315 or newer)?

Comment 7 Pavel Ondračka 2011-04-19 01:36:51 UTC

(In reply to comment #6)
> Can you try this again with the latest git version of mesa (commit
> ffc1d166d24532aeaa4dcf06a431e43ab7e7e315 or newer)?

Still broken with current git. Works fine with RADEON_DEBUG=noopt.

Comment 8 Pavel Ondračka 2011-08-29 02:38:20 UTC

e6c64800cc8833fb4083a556c839b51e8ac84a8b is the first bad commit
commit e6c64800cc8833fb4083a556c839b51e8ac84a8b
Author: Henri Verbeet <hverbeet@gmail.com>
Date:   Tue Aug 9 12:23:47 2011 -0500

    glsl_to_tgsi: improve assignment hack
    
    Fixes StarCraft 2 and Fallout 3 in Wine.

Since e6c64800cc8833fb4083a556c839b51e8ac84a8b this is getting worse. There isn't only high shader setting affected but also low and middle shader quality are now misrendered. This is still the same bug, RADEON_DEBUG=noopt works around this and llvmpipe is not affected at all. CCing Henri, maybe he can bring some light about what his change did to uncover this bug in r300g compiler.

Comment 9 Henri Verbeet 2011-08-30 05:58:18 UTC

(In reply to comment #8)
> around this and llvmpipe is not affected at all. CCing Henri, maybe he can
> bring some light about what his change did to uncover this bug in r300g
> compiler.
I'm afraid I won't have a lot of insights to contribute here. That commit just disabled an optimization in some cases where it wasn't safe to do. I suppose it may be interesting to see if disabling the entire else-if block that patch touches makes it better or worse. Bryan Cain is much more familiar with the glsl-to-tgsi code than I am though.

The typical way to debug things like this is to figure out which shader is responsible for drawing the geometry in question, using something like apitrace or your own debug hacks, and then comparing the GLSL with what gets sent to hardware (and any intermediates).

Comment 10 Pavel Ondračka 2011-09-19 00:06:43 UTC

Unigine Oilrush is also affected.

Comment 11 Tom Stellard 2011-10-14 18:41:51 UTC

The reg rename pass has be re-enabled, can you try again with the latest code from git.

Comment 12 Pavel Ondračka 2011-10-17 08:24:41 UTC

(In reply to comment #11)
> The reg rename pass has be re-enabled, can you try again with the latest code
> from git.

Well, Sanctuary with low shaders seems fine, can't test other setting due to bug 40448. Closing for now.

Some surfaces in Oilrush still seems little bit overbright, however I can't compare because with RADEON_DEBUG=noopt both Sanctuary and Oilrush are now completely broken. And this may be a different bug, will test more. 

BTW the optimizations are now needed for correct output or should I bisect and report it?

Will also do some benchmarks, IIRC there were some slowdowns last time with reg rename pass.

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.