Bug 95198 - Shadow of Mordor beta has missing geometry with gl 4.3
Summary: Shadow of Mordor beta has missing geometry with gl 4.3
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/radeonsi (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Default DRI bug account
QA Contact: Default DRI bug account
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-28 22:09 UTC by Ernst Sjöstrand
Modified: 2016-05-09 23:11 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
screenshot.png (2.68 MB, image/png)
2016-04-28 22:09 UTC, Ernst Sjöstrand
Details
R600_DEBUG=ps,vs,tcs,tes,gs steam's output. (13.90 MB, text/plain)
2016-05-05 19:48 UTC, John
Details
bandaid patch for SI (1.66 KB, patch)
2016-05-06 16:58 UTC, Nicolai Hähnle
Details | Splinter Review
R600_DEBUG=ps,vs,tcs,tes,gs steam's output with Nicolai's patch (14.10 MB, text/plain)
2016-05-07 01:02 UTC, John
Details
bandaid patch for SI v2 (1.78 KB, patch)
2016-05-07 22:17 UTC, Nicolai Hähnle
Details | Splinter Review

Description Ernst Sjöstrand 2016-04-28 22:09:15 UTC
Created attachment 123332 [details]
screenshot.png

The public version of Shadow of Mordor worked fine for me, so I don't think this is a dupe of #92059.

However there's a beta (v1.1?) that supports compute shaders and GL 4.3.
If I run it with MESA_GL_VERSION_OVERRIDE=4.3 MESA_GLSL_VERSION_OVERRIDE=430 and set texture quality to Medium, I get missing geometry as the attached picture.

I'm trying to apitrace it but unfortunately the error doesn't appear when running under apitrace.
Also, glretrace refuses to work with forced gl-versions?
I either get:
MESA_GL_VERSION_OVERRIDE=4.3 MESA_GLSL_VERSION_OVERRIDE=430 glretrace ShadowOfMordor.2.trace 
error: context mismatch: expected OpenGL 1.0, but got OpenGL 4.3 core

Or:
glretrace ShadowOfMordor.2.trace 
error: xlib: BadMatch (invalid parameter attributes)
error: xlib: BadMatch (invalid parameter attributes)
error: failed to create OpenGL 4.3 core context.
Comment 1 Ernst Sjöstrand 2016-04-28 22:12:14 UTC
Card: Radeon Fury
OS: Ubuntu 16.04
Mesa: 11.3~git160426193800.912ed84
Kernel: From 4.7-wip up to MAINTAINERS: Remove unneded wildcard for the Radeon/AMDGPU drivers
LLVM: 3.9~svn267193-0~x~padoka0
Comment 2 Ernst Sjöstrand 2016-04-29 10:07:55 UTC
I get very different behavior if I patch Mesa to report 4.3 support vs. MESA_GL_VERSION_OVERRIDE=4.3 MESA_GLSL_VERSION_OVERRIDE=430. Is that intended? There's no environment variable that behaves like if I did such a Mesa patch?

Short story, much fewer issues (or harder to repro?) when patching Mesa to report 4.3.
Comment 3 Nicolai Hähnle 2016-04-29 16:24:37 UTC
I'm not sure why that is, but I'd say it's good news. Perhaps you can provide an apitrace of the remaining problems?
Comment 4 Ernst Sjöstrand 2016-04-30 11:41:55 UTC
Indeed!

So here's a trace that shows extreme "shinyness" on the benchmark's loading screen for a few frames just before the benchmark starts. I've seen this ingame also, but I wanted to keep the trace as small as possible:
https://www.dropbox.com/s/2vkesni8o4y03dp/ShadowOfMordor.trace?dl=0
Comment 5 John 2016-05-03 22:16:05 UTC
I was going to create a new bug but I think my issue may be the same.

With llvm-svn up to revision 267861 everything works fine, but starting with revision 268162 I get the same issue as in Ersnt's screenshot.

My console prints many of these :
LLVM triggered Diagnostic Handler: LDS size exceeds device maximum
LLVM failed to compile shader
radeonsi: can't create a shader

Card is 280x so a bit different.

I am on latest mesa-git, and linux 4.5 (patched to get computer shader).
Downgrading llvm-svn fixes the issue right away.
Comment 6 Nicolai Hähnle 2016-05-05 17:51:43 UTC
Hi John, that precise LLVM revision doesn't exist (or rather, it's not a change in LLVM itself), and the commits around it don't look obviously related. Did you arrive there by a bisect?

You could help by providing the output when running with R600_DEBUG=ps,vs,tcs,tes,gs.
Comment 7 Bas Nieuwenhuizen 2016-05-05 18:03:53 UTC
Nicolai, the direct cause is probably in r267922 ("AMDGPU: Emit error if too much LDS is used") which is between his good and bad revision and introduces the error.

If I run the non-beta locally I don't get any shader that uses more than 1024 bytes of LDS.

John, did you get this error using the shadow of mordor beta?
Comment 8 John 2016-05-05 19:45:31 UTC
Hi Nicolai,

Oh no, these are just 2 "versions" of the llvm-svn package I get from lcarlier's mesa-git repository. I assumed they matched the llvm revision I can see when browsing https://llvm.org/svn/llvm-project/cfe/trunk.
I can bisect if needed.
I'll attach the output you've asked for it.


Hi Bas,
I get the same issue whereas on the beta or the official build from steam.
Comment 9 John 2016-05-05 19:48:48 UTC
Created attachment 123504 [details]
R600_DEBUG=ps,vs,tcs,tes,gs steam's output.

I tried to clean up the log up to the start of the game and till the end, but it is still pretty big, and probably include more than needed.

I'd guess you may be able to pinpoint better by looking around "LLVM triggered Diagnostic Handler: LDS size exceeds device maximum".

Thank you!
Comment 10 Nicolai Hähnle 2016-05-06 16:58:27 UTC
Created attachment 123521 [details] [review]
bandaid patch for SI

Hi John, please try the attached patch - it should help slightly.

Generally speaking, it looks like tesselation support is a bit of a mess :(
Comment 11 John 2016-05-07 01:02:15 UTC
Created attachment 123530 [details]
R600_DEBUG=ps,vs,tcs,tes,gs steam's output with Nicolai's patch

I have patched mesa and rebuilt it, but I don't see any difference.
Here's the new log.
Comment 12 Nicolai Hähnle 2016-05-07 22:17:22 UTC
Created attachment 123543 [details] [review]
bandaid patch for SI v2

Sorry, there was a stupid mistake in my proposed patch. This one should work :)
Comment 13 John 2016-05-07 23:00:40 UTC
Hmmm, only the comments seem to have changed between the 2 patches, the code change is the same isn't it?
Comment 14 John 2016-05-07 23:33:49 UTC
I tried to guess the typo and replaced patch_dw_size by lds_dwords in the code change, as patch_dw_size is not used past lds_dwords' declaration.
So far it seems to work fine; I saw no visual issue and there is no error printed in the terminal either.

I'll be glad to test a proper solution when you find one as well.
Are you going to submit this patch in the meantime or should I keep patching my daily mesa-git?

Thank you!
John
Comment 15 Nicolai Hähnle 2016-05-09 17:42:56 UTC
The patch is now in Mesa git master.
Comment 16 John 2016-05-09 23:11:23 UTC
Cool, thank you!


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.