In ColorD::outputChanged it is checked if the current screen is the primary one and if that is the case (modulo some heuristics) the _ICC_PROFILE X atom is set. This is incorrect behaviour. According to the X specifications [0,1]: "The atom name for the first monitor in a root window is _ICC_PROFILE. For root windows spanning more than one monitor, as typical in Xinerama and XRandR multihead configurations, a atom for each monitor is added holding the appropriate ICC profile. The first monitor uses the _ICC_PROFILE atom name. All monitors in a root window starting from number one use _ICC_PROFILE as atom name extended with an underscore plus the monitor number, e.g. _ICC_PROFILE_1 . Monitor counting starts with zero. Thus a _ICC_PROFILE_0 atom should not appear. The counting follows the Xinerama API." This means that no X atom is set for any but the primary monitor (which is a missing feature and not a real bug) but it also means that the wrong X atom is set when the primary monitor isn't the 0th one and its profile gets changed. That is clearly a bug. Attached you will find a patch that sets the correct X atom for the changed screen. I hope that this is the correct way, but at least it seems that xiccd handles this in a similar way and I couldn't find any problems with my patch after testing it for a while. I took the liberty to set the severity to Major since most color managed applications (for example GIMP) don't use colord to get their screen profiles but use the X atom, rendering the color corrections useless. [0] the original specs seem to have gone with the Freedesktop Wiki: http://web.archive.org/web/20111227085244/http://www.freedesktop.org/wiki/OpenIcc/ICC_Profiles_in_X_Specification_0.4 [1] the same information still lives in the Oyranos Wiki: http://www.oyranos.org/wiki/index.php?title=ICC_Profiles_in_X_Specification_0.4 Reproducible: Always
Created attachment 83836 [details] Proposed patch to set the correct X atom
I think this problem manifests on my Fedora 20 system. The correct profile will be applied to the built-in display when booting up / logging in, but hotplugging a second display to the laptop will result in the profile that was assigned to the internal display to be re-assigned to the external display (and it looks really bad). I can work around the problem by issuing a 'service colord restart'.
This bug still applies for colord-kde 0.5.3. I did a quick port of the proposed patch by houz@gmx.de to the current code base, but at least in my setup there is a mixup: colord has the correct profiles for both outputs, but color-kde pushes the profile for the second output into _ICC_PROFILE and the profile for the first output into _ICC_PROFILE_1.
Created attachment 111516 [details] New patch I have used a new patch for a while now. It uses a slightly different enumeration of monitors and follows what Graeme Gill suggested on the Openicc mailing list as an update of the specifications [0] and subsequent discussions in different places. [0] https://lists.freedesktop.org/archives/openicc/2016q4/005205.html
The new patch applies cleanly against the latest colord-kde git master and seems to work perfectly, at least darktable-cmstest reports that both colord and the X atoms now hold the same profile for each of my three monitors. This was not the case before. I guess this should be merged?
Git commit e0deacd8efc4c2447337c0b62761f3a84a8382a0 by Valeriy Malov, on behalf of Tobias Ellinghaus. Committed on 29/04/2018 at 19:46. Pushed by valeriymalov into branch 'master'. Change _ICC_PROFILE atom assignment to properly support multiscreen In ColorD::addOutput: handle case when ColorD already has the device we're trying to add Detect connected output atom IDs and apply set _ICC_PROFILE for each This is a largerly unchanged patch from corresponding bugzilla page Changes to patch: split getAtomIds a bit, remove unneeded pointer wrap I don't really have calibrated screens to test how correct the set atoms are, but this patch makes darktable-cmstest happy and it seems to compare colord profile content to _ICC_PROFILE atoms, so I guess it should be good M +162 -30 colord-kded/ColorD.cpp M +14 -3 colord-kded/ColorD.h https://commits.kde.org/colord-kde/e0deacd8efc4c2447337c0b62761f3a84a8382a0
Sorry beeing quite busy lately, but I'd like to ask if this works on Gnome? Last time I discussed this issue with colord author, he said _ICC_PROFILE atoms for outputs were completely broken and would not work on all cases, thus I forgot about the issue, while I do have a multi monitor setup I don't use often so couldn't test this.
Well, the patch at hand is for the KDE frontend which obviously does nothing for GNOME. The real question is probably more about the usefulness of the additional X atoms. Currently most programs are not using them, darktable [0] being one of the few exceptions I am aware of. Once X11 is gone and Wayland will have taken over the whole thing will be moot anyway as Wayland doesn't support those legacy settings in the first place. So I guess the answer is that it's not a question of KDE/GNOME but what program is supposed to use the color management settings. [0] https://darktable.org/
It is a question for Gnome because we must behave consistently, the problem is not this being legacy but the fact that enumerating X outputs is just broken this way, might work for you but won't work properly on a plug and play basis. I not entirely against this patch, but would like to know how's g-c-m is doing.
In fact darktable should be patched to ask colord about what is being used ATM, this would then make is just work on Wayland. Which ATM has no support for color correction as that needs to be done in KWin now.
darktable is one of the few applications which actually uses both colord and the X11 atoms. The darktable-cmstest command line utility is often used as a convenient shortcut to check which values are stored for which display in both colord and the X11 atoms, and because darktable is also one of the even fewer applications which try to handle multiple monitors correctly, it also serves as an example for how the enumeration of X displays and storing ICC profiles for multiple displays in X11 atoms might work in a real-world implementation. That doesn't mean I agree with the way darktable enumerates displays, but I have multiple multi-monitor setups and the way it handles displays seems to work in all cases. The patch for colord-kde committed in this bug report seems to line up with what darktable is expecting, so now there seem to be two real-world implementations of the specification which agree with each other. I reported the same bug against GNOME Color Manager in March, https://bugzilla.gnome.org/show_bug.cgi?id=794486. Quoting the maintainer: "I don't think it's possible to solve this bug. In my opinion, the ICC-profiles-in-X specification is just impossible to implement fully on a modern desktop stack." I think Wayland should be left out of the discussion since this bug report only concerns the X atoms, X will still be with us for many years even inside Wayland sessions, and I have the feeling that if applications even support system-wide color management (GIMP and RawTherapee don't) they seem to rely on the X11 atoms more than on colord. Genview and digiKam fall in the latter category, for example, as does Chrome.
Ok thanks for the info, that aligns up with what I was expecting from GCM. I'm now just curious why darktable doesn't simply ignores the XAtom if it knows about colord... Whatever, I'll leave this as is, and if I find some time roll another release... Thanks
I think darktable actually gives higher precedence to colord if it finds both an active colord profile and an X atom for the same display. But it has to support and check both.