Bug 108355

I start thinking I'm not in the right Bugzilla section as OpenGL does not handle mouse cursors. Sorry if that the case, I would love to know the exact faulting component in the graphics stack.

I do not know if this is a SDL, X11, drm, amdgpu or hardware problem, or the game itself (some kind of surface corruption). But Gnome screenshot program is able to correctly retrieve the cursor image as shown in the attachment.

I wrote a small SDL program that changes my mouse cursor, as this is what Civ6 seems to use, but everything works fine.

I contacted Aspyr support but their response was a dead-end : "AMD an Intel GPUs aren't supported".

I tried to make the game use my own compiled SDL version with no luck. Steam games seem to use some kind of sandbox, LD_PRELOAD seems to be ignored. Or the game may not actually use SDL for the mouse cursor though the related symbols are referenced in the executable.

Please attach the corresponding Xorg log file and output of dmesg.

Created attachment 142058 [details]
dmesg after reboot

Created attachment 142059 [details]
Xorg log after reboot and started briefly the game in windowed mode

I tried to run the game under a Wayland session instead of X11 and the mouse cursor is normal.

Does it still happen with xf86-video-amdgpu 18.1.0?

Does amdgpu.dc=0 on the kernel command line avoid the problem?

amdgpu.dc=0 had no effect, but using xf86-video-amdgpu 18.1.0 indeed fixed the problem :) Thank you Michel.

Hopefully that new version we'll be released somehow for my Linux distribution.

I still do not know if the mouse cursor is displayed as intended (there is a shadow which seems to use additive blending instead of alpha blending) but at least there are no more artifacts.

I guess I should change the Bugzilla Product to "xorg"?

(In reply to Hadrien Nilsson from comment #7)
> amdgpu.dc=0 had no effect, but using xf86-video-amdgpu 18.1.0 indeed fixed
> the problem :) Thank you Michel.

Well, that's a surprising result though. Please attach a new Xorg log file from xf86-video-amdgpu 18.1.0.


> I still do not know if the mouse cursor is displayed as intended (there is a
> shadow which seems to use additive blending instead of alpha blending) but
> at least there are no more artifacts.

For comparison, you can disable the HW cursor with

 Option "SWcursor"

in the /etc/X11/xorg.conf Section "Device".

Created attachment 142083 [details]
Xorg log with amdgpu_drv 18.1.0

I think you're right to be doubtful. I looked at the log and "amdgpu_drv.so" 18.1 was not loaded as it's not ABI compatible with the rest of my Xorg installation. So when the bug disappeared I guess I was running in some kind of fallback mode that does not have the mouse cursor bug :/

Ubuntu 18.10 will be released soon, so maybe I could wait for that version. Xorg code will probably be updated then.

Created attachment 142088 [details] [review]
Detect and fix up non-premultiplied cursor  data

(In reply to Hadrien Nilsson from comment #9)
> I looked at the log and "amdgpu_drv.so" 18.1 was not loaded as it's not ABI
> compatible with the rest of my Xorg installation.

That's because the driver was compiled for Xorg 1.20, whereas you're using 1.19. Simply recompiling the newer driver against your local xserver-xorg-dev should overcome this.

Anyway, given the symptoms, the fundamental issue here is probably that the game uses incorrect cursor data without premultiplied alpha.

However, I guess we need to be robust against this. Can you try the attached xf86-video-amdgpu patch?

Thank you for the patch, Michel.

I spent the last few days trying to understand the situation about color mouse cursors and I think it is rather confusing.

I extracted the mouse cursor asset from Civ6 and I could reproduce the same problem with my SDL test program. When I multiplied the R, G and B components with the Alpha, the blending was correct. So I think you are right about the symptoms.

I tried your patch and the blending was correct in both my SDL program and the game.

Now the question is, what should be the correct behavior? How should the RGBA data be provided, premultiplied or straight? I tried to find specifications, alas:

 - the SDL documentation gives no clue about it;
 - XcursorImage* documentation neither;
 - wl_pointer_set_cursor neither.

I even looked at Windows. No clue there with the exception of what I think is a low level function: DrvSetPointerShape, where finally it gives information about the pixel format: the RGB data should be indeed alpha premultiplied.

As the documentation is not very clear, I had to test implementations. Here are my results with different combinations:

 - Xorg + amdgpu             : premultiplied ok, straight wrong
 - Xorg + intel              : premultiplied ok, straight wrong
 - Xorg + proprietary nVidia : premultiplied ok, straight ok
 - wayland + amdgpu          : premultiplied ok, straight wrong
 - wayland + intel           : premultiplied ok, straight wrong
 - Windows + amdgpu          : premultiplied ok, straight ok
 - Windows + intel           : premultiplied ok, straight ok

So it looks like Windows with any graphics card is able to detect a wrong format (straight alpha) and fix it, as you did in your patch. And also nVidia proprietary X11 drivers have the same workaround. I think it could explain why Aspyr only supports nVidia as nVidia drivers will fix wrong data for them.

I'm very happy your patch fixes Xorg + amdgpu combination, but isn't there a larger problem, where such a workaround should be also done somewhere else in a centralized part of the graphics stack (SDL, Xcursor, wayland?) so any combination could benefit of it?

X uses pre-multiplied, windows uses non-pre-multiplied (or at least used to, not sure if this changed).  Not all games ported from windows fix this up, do the driver attempts to detect and fix this.

Thanks Hadrien for the thorough testing!

AFAIK Linux KMS only supports premultiplied alpha for cursors.

I'll also submit a corresponding change for the xserver repository, which should cover the remaining Linux cases and reduce the overhead of the workaround even with this driver.

(In reply to Michel Dänzer from comment #13)
> I'll also submit a corresponding change for the xserver repository, which
> should cover the remaining Linux cases and reduce the overhead of the
> workaround even with this driver.

That would be great, thanks Michel.

Regarding Wayland, I think that's a subject for another day and another bug ticket (maybe not even on freedesktop.org? I still need to understand how Wayland compositors are made).

(In reply to Hadrien Nilsson from comment #14)
> That would be great, thanks Michel.

Landed already: https://gitlab.freedesktop.org/xorg/xserver/commit/b45c74f0f2868689e7ed695b33e8c60cd378df0b


> Regarding Wayland, I think that's a subject for another day [...]

The above is active in Xwayland as well, so I think it should cover Wayland. Native Wayland apps have to use premultiplied alpha for cursors.

(In reply to Michel Dänzer from comment #15)
> Landed already:
> https://gitlab.freedesktop.org/xorg/xserver/commit/
> b45c74f0f2868689e7ed695b33e8c60cd378df0b

Amazing :)

> Native Wayland apps have to use premultiplied alpha for cursors.

Thank you for confirming the information. I did not find it in the documentation, hopefully it can be improved.

Unfortunately, I had to revert these changes due to bug 108650. Please test https://gitlab.freedesktop.org/daenzer/xf86-video-amdgpu/commit/d4f250aecabd07a524bf6df3d8eb9e00e589dcf0 instead.

(In reply to Michel Dänzer from comment #17)
> Unfortunately, I had to revert these changes due to bug 108650. Please test
> https://gitlab.freedesktop.org/daenzer/xf86-video-amdgpu/commit/
> d4f250aecabd07a524bf6df3d8eb9e00e589dcf0 instead.

It's sad the situation is so confusing. Maybe a new mouse cursor API could be created that accepts normal alpha, as this is the common way to store it (in PNG files for example)? With a conversion to what the hardware expects, of course. It looks like a lot of developers are unable to correctly use an API that expects premultiplied alpha, with the additional difficulty that the documentation does not say it explicitly.

Regarding Wayland, I opened a ticket to ask for a documentation update, as an attempt to avoid the same confusion we have now with X. Hopefully the documentation maintainer will be sensitive to the problem.

Anyway, I tested your branch (cursor-unpremultiply-overflow-no-gamma) and unfortunately it freezes my system as soon as the Civilization VI cursor is about to be shown.

(In reply to Hadrien Nilsson from comment #18)
> Anyway, I tested your branch (cursor-unpremultiply-overflow-no-gamma) and
> unfortunately it freezes my system as soon as the Civilization VI cursor is
> about to be shown.

Weird. Can you still access the system via SSH? Does Xorg die (if so, please get the Xorg log file and/or its stderr output), or enter an infinite loop (if so, can you attach gdb to it and get a backtrace)?

(In reply to Michel Dänzer from comment #19)
> (In reply to Hadrien Nilsson from comment #18)
> > Anyway, I tested your branch (cursor-unpremultiply-overflow-no-gamma) and
> > unfortunately it freezes my system as soon as the Civilization VI cursor is
> > about to be shown.
> 
> Weird. Can you still access the system via SSH? Does Xorg die (if so, please
> get the Xorg log file and/or its stderr output), or enter an infinite loop
> (if so, can you attach gdb to it and get a backtrace)?

I did some tests today: I can still access to my computer from another one via SSH. Xorg is running at 100% of cpu. I could pause the process with gdb. The process is stuck in drmmode_do_load_cursor_argb(). This loop in particular:

retry:
		for (i = 0; i < cursor_size; i++) {
			argb = image[i];
			if (!drmmode_cursor_pixel(crtc, &argb, &premultiplied,
						  &apply_gamma))
				goto retry;

			ptr[i] = cpu_to_le32(argb);

Ah, I made a silly mistake there. :) https://gitlab.freedesktop.org/daenzer/xf86-video-amdgpu/commits/cursor-unpremultiply-overflow-no-gamma updated, please test again.

It does not freeze anymore :)

I do not get the strange saturated RGB artifacts, however the blending is back to the wrong one, and Gnome night mode does not apply anymore. With the previous patch (the one that unfortunately led to regression in some games), the blending was perfect and Gnome night mode was correctly applied.

I wonder how the nVidia proprietary driver does in these cases.

(In reply to Hadrien Nilsson from comment #22)
> I do not get the strange saturated RGB artifacts, however the blending is
> back to the wrong one, and Gnome night mode does not apply anymore.

Yep. The only purpose of this patch is to prevent the artifacts (which occurred due to reading beyond the end of the gamma LUT).

> With the previous patch (the one that unfortunately led to regression in some
> games), the blending was perfect and Gnome night mode was correctly applied.
> 
> I wonder how the nVidia proprietary driver does in these cases.

Does the Civilization VI cursor work correctly with it? If so, maybe it has a quirk database for broken applications. I don't know any other way to work around Civilization VI without breaking other apps at this point.

Another possibility might be that the nvidia driver incorrectly interprets all cursor data as non-premultiplied alpha. This would result in translucent parts of most cursor images being displayed slightly incorrectly (more translucent than intended), but this might be less noticeable than non-premultiplied alpha being interpreted as premultiplied alpha (which results in over-bright colours).

(In reply to Michel Dänzer from comment #23)
> Does the Civilization VI cursor work correctly with it?

The SDL application I wrote gave the same result (a good blending) on an nVidia hardware on Linux I tested on, regardless of the alpha mode.

> If so, maybe it has a quirk database for broken applications.
> I don't know any other way to work around Civilization VI without
> breaking other apps at this point.

Maybe, I heard it is something nVidia does, having specific code paths for specific applications.

It looks like the only thing that can be done at this point is asking for a better documentation in X, Wayland and SDL to warn future developers about these aspects.

I have little hope regarding Aspyr if I ask them to fix their code (they will probably say again that Intel and AMD are not supported).

Thanks for the report and testing. The artifacts are fixed in Git master:

commit 13c94a373b4858a2d2aa14c22b5f98d53c84c0d9
Author: Michel Dänzer <michel.daenzer@amd.com>
Date:   Thu Nov 15 16:40:46 2018 +0100

    Skip gamma correction of cursor data if premultiplied R/G/B > alpha