Bug 70327 - Casting floating point variable to integer not working properly while constant gets converted properly
Summary: Casting floating point variable to integer not working properly while constan...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r600 (show other bugs)
Version: git
Hardware: All Linux (All)
: medium normal
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-09 21:17 UTC by tony.wasserka
Modified: 2013-10-14 06:45 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Attachments
Full rendered scene with bug visible (780.91 KB, text/plain)
2013-10-09 21:17 UTC, tony.wasserka 		Details
Full rendered scene with software renderer (775.68 KB, image/png)
2013-10-09 21:24 UTC, tony.wasserka 		Details
Full rendered scene with bug visible (780.91 KB, image/png)
2013-10-09 21:24 UTC, tony.wasserka 		Details
dff file to reproduce the issue by rendering a subset of the reference scene in Dolphin (2.32 MB, application/octet-stream)
2013-10-09 21:30 UTC, tony.wasserka 		Details
Output of R600_DEBUG=vs,ps,fs,sbdisasm when rendering the reduced scene (955.08 KB, text/plain)
2013-10-09 21:36 UTC, tony.wasserka 		Details
Sample GLSL shader that is used during rendering the glitched character (5.07 KB, text/plain)
2013-10-09 21:51 UTC, tony.wasserka 		Details
R600_DEBUG=vs,ps,fs,sbdisasm for small scene with the shader that we actually use (487.38 KB, text/plain)
2013-10-09 22:16 UTC, tony.wasserka 		Details
R600_DEBUG=vs,ps,fs,sbdisasm for small scene with the shader where I replaced "fog" with "0.0" in the problematic line (478.77 KB, text/plain)
2013-10-09 22:18 UTC, tony.wasserka 		Details
patch (2.71 KB, patch)
2013-10-10 01:15 UTC, Vadim Girlin 		Details | Splinter Review
Show Obsolete (1) View All

Description tony.wasserka 2013-10-09 21:17:52 UTC 
Created attachment 87353 [details]
Full rendered scene with bug visible

I've recently been working on converting the shaders used by the Dolphin emulator to use integers instead of floating point numbers (for better emulation accuracy). This seems to have exposed a bug in the r600g driver possibly related to float->int casting.

The core of the issue is this GLSL code line:
"iprev.rgb = (iprev.rgb * (int(256.0-fog))) / 256;" (*)

where fog has been initialized before as 
"float fog = clamp(ze - fog_uniform.z, 0.0, 1.0);"

The point is, while "fog" seems to be zero (a "fog==0.0" condition in the code will return true), the pixel shader result of (*) is different than the one that I get by substituting "fog" with 0.0. This gets very apparent in the rendered scene, cf
Comment 1 tony.wasserka 2013-10-09 21:22:20 UTC 
...cf. first attachment "Full rendered scene with bug visible". Second attachement "Full rendered scene with software renderer" shows the same scene rendered properly with "LIBGL_ALWAYS_SOFTWARE=1" (I don't know if it was using llvmpipe or swrast, took halve a minute to render if that says anything).

I'll provide a way to reproduce the issue in a followup comment.
Comment 2 tony.wasserka 2013-10-09 21:24:12 UTC 
Created attachment 87355 [details]
Full rendered scene with software renderer
Comment 3 tony.wasserka 2013-10-09 21:24:53 UTC 
Created attachment 87356 [details]
Full rendered scene with bug visible
Comment 4 tony.wasserka 2013-10-09 21:30:56 UTC 
Created attachment 87357 [details]
dff file to reproduce the issue by rendering a subset of the reference scene in Dolphin
Comment 5 tony.wasserka 2013-10-09 21:36:45 UTC 
Created attachment 87359 [details]
Output of R600_DEBUG=vs,ps,fs,sbdisasm when rendering the reduced scene
Comment 6 tony.wasserka 2013-10-09 21:43:40 UTC 
Steps the reproduce the issue:
1. Clone the dolphin repository from the url provided at http://code.google.com/p/dolphin-emu/source/checkout
2. Checkout the tev_fixes_new branch at commit 5d8cfade42ce .
3. Build Dolphin with the build instructions outlined at http://code.google.com/p/dolphin-emu/wiki/Linux_Build . We use CMake for our build system, so if you're familiar with that you should be able to get it running quickly.
4. run "dolphin-emu -b -e <path to the attached dff file>". Alternatively, run Dolphin normally and open the dff file manually.
5. You should be able to see Mario (the main character of the game in the rendered scene), and only Mario (I removed all unimportant stuff from the scene). If you don't, try to make sure things work all by running any gamecube demo elf (sorry, got no link) and following the instructions at http://wiki.dolphin-emu.org/index.php?title=FifoPlayer under "How to play back a fifo log?"


Basically, r600g renders the broken screenshot that I attached, while the software renderer renders the scene fine. Replacing "fog" in the "iprev.rgb = (iprev.rgb * (int(256.0-fog))) / 256;" line with "0.0" makes the scene render correctly.
Comment 7 tony.wasserka 2013-10-09 21:51:48 UTC 
Created attachment 87360 [details]
Sample GLSL shader that is used during rendering the glitched character

Attached you find one of the glitched GLSL shaders which uses the mentioned line at line 149.

Sorry for the dumb source layout, that's because the shaders aren't hand written but automatically generated.
Comment 8 tony.wasserka 2013-10-09 22:16:57 UTC 
Created attachment 87361 [details]
R600_DEBUG=vs,ps,fs,sbdisasm for small scene with the shader that we actually use
Comment 9 tony.wasserka 2013-10-09 22:18:43 UTC 
Created attachment 87362 [details]
R600_DEBUG=vs,ps,fs,sbdisasm for small scene with the shader where I replaced "fog" with "0.0" in the problematic line
Comment 10 Vadim Girlin 2013-10-10 01:15:51 UTC 
Created attachment 87364 [details] [review]
patch

This patch should fix the issue (only compile-tested so far, I can't test with r600g card right now, but hopefully it's simple enough to be correct).
Comment 11 tony.wasserka 2013-10-10 08:55:12 UTC 
I can confirm that the patch fixes the issue, thanks :)

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.