Bug 75226

What hardware and version of the driver are in use?  If possible, please attach the xorg log, and the output of dmesg and glxinfo.

(In reply to comment #2)
> [...] would happily provide a link to user apitraces if that would help.

It certainly shouldn't hurt. :)

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

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?

Playback looks correct with i965.  It is dark with gallium-based ilo, however.

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.

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.

(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.

Created attachment 94530 [details] [review]
assume sRGB by default

Does this help?

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.

Created attachment 94535 [details] [review]
Add support for sRGB render targets in r600g

r600g will need this patch however.

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.

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.

(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)

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?

(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.

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.

(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 :)