D21076: ICC Color Correction Effect
Philipp Knechtges
noreply at phabricator.kde.org
Fri Sep 25 21:09:36 BST 2020
knechtges added a comment.
At first, I very much appreciate that somebody starts working on color correction in KWin again. There is certainly a need for it. However, and although I am by no means an expert in this area, I have certain concerns about the approach.
The current state of the art in the X Windowing System world seems to be that the application either queries colord or reads the _ICC_PROFILE xatom <http://www.burtonini.com/computing/x-icc-profiles-spec-0.2.html>, and then subsequently does the conversion on its own. The window manager seesm to have more a passive role.
Examples are:
- Darktable uses either colord or the xatom (cf. here <https://github.com/darktable-org/darktable/blob/fa4742023f65334d3c2fecc3062424ea4fda0fc5/src/common/colorspaces.c#L1804>).
- Inkscape watches for changes in the _ICC_PROFILE xatom (cf. here <https://gitlab.com/inkscape/inkscape/-/blob/master/src/ege-color-prof-tracker.cpp#L441>)
- Gimp also reads the xatom (cf. here <https://github.com/GNOME/gimp/blob/f2ef0ec6a5c57c6c2e51c2018c0377376cb714a8/libgimpwidgets/gimpwidgetsutils.c#L521>)
It seems that there once existed plans to offload the color conversion to the window manager, or to at least indicate the regions for which the compositor does not need to color correct [ 1 <https://lists.freedesktop.org/archives/openicc/2010q2/002180.html> ].
Regarding wayland, I have no clue whether there is something similar worked on already. At least the example client applications listed above seem to be totally oblivious of wayland in that regard.
INLINE COMMENTS
> vitaliyf wrote in icc.cpp:113-118
> OK, I changed uint8_t* to std::vector<uint8_t>.
>
> But how to do it with cmsHPROFILE? It's probably a pointer type, but it's internal to the LCMS2 library. How to wrap it in a QScopedPointer, and are you sure it must be wrapped? I think it will only complicate the code... It's local to this exact function, so is it a big problem that it's allocated with custom code?..
FYI, we have a similar construction in poppler with std::shared_ptr and a custom deleter to handle cmsHPROFILEs, cf. https://gitlab.freedesktop.org/poppler/poppler/-/commit/5927e0b08f1ad6868d04cb209ee1e17b4ac07b70#966756b758fb5d7e46114c6835faf491d5b92582_195_195.
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D21076
To: vitaliyf, zzag, rjvbb
Cc: knechtges, rjvbb, graesslin, davidedmundson, anthonyfieroni, zzag, ngraham, kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20200925/1c66fa7b/attachment.htm>
More information about the kwin
mailing list