Bug 75226

Summary: Dark rendering of War for the Overworld
Product: Mesa Reporter: Erich Hoover <erich.e.hoover>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: commiethebeastie, grantipak, step2back+freedesktop
Version: unspecified   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Requested end user logs
assume sRGB by default
Add support for sRGB render targets in r600g
Use linear formats for TFP textures
Other user's logs
make winsys fbo sRGB-capable when supported

Description Erich Hoover 2014-02-19 20:22:05 UTC
I am doing volunteer tech support for Linux users of War for the Overworld and I've had a couple radeon users report problems with the game being rendered very dark ( https://forum.subterraneangames.com/attachments/menu2-png.2894/ ):
https://forum.subterraneangames.com/threads/ingame-colors-to-dark-to-play.4941/

When I replay their apitrace on my own machine (nVidia card) I do not see such issues, and have not had reports from any non-radeon users, so I suspect that something is being done incorrectly in the driver.  I'm happy to work as an intermediary for these users in order to collect more information and would happily provide a link to user apitraces if that would help.
Comment 1 Alex Deucher 2014-02-19 22:38:23 UTC
What hardware and version of the driver are in use?  If possible, please attach the xorg log, and the output of dmesg and glxinfo.
Comment 2 Michel Dänzer 2014-02-20 03:57:57 UTC
(In reply to comment #2)
> [...] would happily provide a link to user apitraces if that would help.

It certainly shouldn't hurt. :)
Comment 3 Erich Hoover 2014-02-20 22:00:50 UTC
Created attachment 94458 [details]
Requested end user logs

I've attached the requested logs for one of the users (Radeon HD 6870).  This is the user's apitrace (same trace, second link is uncompressed):
1) http://95.47.140.249/trace.tar.gz
2) https://dl.dropboxusercontent.com/u/195059/wfto/WFTO.x86_64.trace.1

The apitrace I have for the other user can be downloaded from (same trace, alternative download):
1) http://www.unet.univie.ac.at/~a0500740/WFTO.x86_64.trace
2) https://dl.dropboxusercontent.com/u/195059/wfto/WFTO.x86_64.trace
Comment 4 Michel Dänzer 2014-02-21 09:38:08 UTC
Playing back the traces looks the same using llvmpipe, which indicates that the problem is in a component shared at least by Gallium drivers (which includes the nouveau drivers for nVidia cards).

Did you get any specific reports of users not being affected by this using nouveau or intel Mesa drivers?
Comment 5 Chia-I Wu 2014-02-21 12:54:59 UTC
Playback looks correct with i965.  It is dark with gallium-based ilo, however.
Comment 6 Emil Velikov 2014-02-21 12:58:42 UTC
Indeed the replaying the trace under nouveau(nv96) produces the same dark experience. FWIW retracing produces a couple of warnings, with the one that struck me is the lack of current context during glClear.
Comment 7 Chia-I Wu 2014-02-21 15:42:11 UTC
I believe i965 fixed the issue with this commit

commit e15c21a957b62ab856ab286e8253dd1151a3386e
Author: Eric Anholt <eric@anholt.net>
Date:   Fri Feb 15 07:41:42 2013 -0800

    i965: Make sRGB-capable framebuffers by default.

We would need a similar treatment in st/dri and st/mesa.
Comment 8 Erich Hoover 2014-02-21 17:15:16 UTC
(In reply to comment #4)
> Playing back the traces looks the same using llvmpipe, which indicates that
> the problem is in a component shared at least by Gallium drivers (which
> includes the nouveau drivers for nVidia cards).
> 
> Did you get any specific reports of users not being affected by this using
> nouveau or intel Mesa drivers?

No, I think everyone on nVidia that I have heard from is using the proprietary drivers.  People using some versions the Intel drivers are having a different rendering issue (the menu is being rendered small and in the wrong spot), but that appears to be bad code from the UI software rather than the driver.
Comment 9 Chia-I Wu 2014-02-21 18:11:36 UTC
Created attachment 94530 [details] [review]
assume sRGB by default

Does this help?
Comment 10 Fredrik Höglund 2014-02-21 21:02:20 UTC
Comment on attachment 94530 [details] [review]
assume sRGB by default

Review of attachment 94530 [details] [review]:
-----------------------------------------------------------------

I think this should be done for PIPE_FORMAT_BGRX8888_UNORM as well.
Comment 11 Fredrik Höglund 2014-02-21 21:22:17 UTC
Created attachment 94535 [details] [review]
Add support for sRGB render targets in r600g

r600g will need this patch however.
Comment 12 Fredrik Höglund 2014-02-22 19:49:23 UTC
Created attachment 94581 [details] [review]
Use linear formats for TFP textures

After restarting the compositor with the sRGB patches applied, I realized that we need this change too.
Comment 13 Erich Hoover 2014-02-24 18:00:29 UTC
Created attachment 94676 [details]
Other user's logs

Attached are the logs for the other user, it sounds like you guys already have things figured out - but maybe more information is better.

(In reply to comment #9)
> Created attachment 94530 [details] [review] [review]
> assume sRGB by default
> 
> Does this help?

I'm trying to think of a way to easily help the end users test this, do they only need this patch or do they need the others as well?  Based on distro I initially assumed one of them had built their graphics stack from source and, unfortunately, my assumption was incorrect :/

Also, sorry for the delayed reply - things have been a little crazy lately.
Comment 14 Chia-I Wu 2014-02-26 07:08:00 UTC
(In reply to comment #12)
> Created attachment 94581 [details] [review] [review]
> Use linear formats for TFP textures
> 
> After restarting the compositor with the sRGB patches applied, I realized
> that we need this change too.
That would defeat the original patch: the code path is shared by the game and the compositor.

It feels like we need to fix the bug solely from st/mesa, without touching st/dri.

(Sorry for the slow response.  I was sick in bed for the last few days, and have to catch up with $job first)
Comment 15 Chia-I Wu 2014-03-03 08:35:13 UTC
Created attachment 95005 [details] [review]
make winsys fbo sRGB-capable when supported

This new patch is tested with llvmpipe and ilo, and is also sent to the list

  http://lists.freedesktop.org/archives/mesa-dev/2014-March/055172.html

For r600g, Fredrik Höglund's patch is likely still be needed.  Not sure what is the best way to get Tested-by and Reviewed-by for the patch.

@Fredrik Höglund: do you want to send your patch as an independent change?
Comment 16 Fredrik Höglund 2014-03-04 22:00:16 UTC
(In reply to comment #15)
> Created attachment 95005 [details] [review] [review]
> make winsys fbo sRGB-capable when supported
> 
> This new patch is tested with llvmpipe and ilo, and is also sent to the list
> 
>   http://lists.freedesktop.org/archives/mesa-dev/2014-March/055172.html
> 
> For r600g, Fredrik Höglund's patch is likely still be needed.  Not sure what
> is the best way to get Tested-by and Reviewed-by for the patch.
> 
> @Fredrik Höglund: do you want to send your patch as an independent change?

I think the format translation rewrite Marek pushed also fixes this, so that patch is probably not needed anymore.
Comment 17 Chia-I Wu 2014-03-06 03:04:59 UTC
I've pushed the fix to master (commit 4c68c6dcf).  It is verified with the trace on radeonsi, ilo, and llvmpipe.  I am closing the bug now.  In case it does not fix the real game, feel free to reopen it.
Comment 18 Erich Hoover 2014-03-06 20:36:33 UTC
(In reply to comment #17)
> I've pushed the fix to master (commit 4c68c6dcf).  It is verified with the
> trace on radeonsi, ilo, and llvmpipe.  I am closing the bug now.  In case it
> does not fix the real game, feel free to reopen it.

Thanks so much!  I'll see what I can do about getting the users to test it :)

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.