<table><tr><td style="">knechtges added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D21076">View Revision</a></tr></table><br /><div><div><p>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.</p>

<p>The current state of the art in the X Windowing System world seems to be that the application either queries colord or reads the <a href="http://www.burtonini.com/computing/x-icc-profiles-spec-0.2.html" class="remarkup-link" target="_blank" rel="noreferrer">_ICC_PROFILE xatom</a>, and then subsequently does the conversion on its own. The window manager seesm to have more a passive role.</p>

<p>Examples are:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">Darktable uses either colord or the xatom (cf. <a href="https://github.com/darktable-org/darktable/blob/fa4742023f65334d3c2fecc3062424ea4fda0fc5/src/common/colorspaces.c#L1804" class="remarkup-link" target="_blank" rel="noreferrer">here</a>).</li>
<li class="remarkup-list-item">Inkscape watches for changes in the _ICC_PROFILE xatom (cf. <a href="https://gitlab.com/inkscape/inkscape/-/blob/master/src/ege-color-prof-tracker.cpp#L441" class="remarkup-link" target="_blank" rel="noreferrer">here</a>)</li>
<li class="remarkup-list-item">Gimp also reads the xatom (cf. <a href="https://github.com/GNOME/gimp/blob/f2ef0ec6a5c57c6c2e51c2018c0377376cb714a8/libgimpwidgets/gimpwidgetsutils.c#L521" class="remarkup-link" target="_blank" rel="noreferrer">here</a>)</li>
</ul>

<p>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 [ <a href="https://lists.freedesktop.org/archives/openicc/2010q2/002180.html" class="remarkup-link" target="_blank" rel="noreferrer">1</a> ].</p>

<p>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.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21076#inline-118887">View Inline</a><span style="color: #4b4d51; font-weight: bold;">vitaliyf</span> wrote in <span style="color: #4b4d51; font-weight: bold;">icc.cpp:113-118</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">OK, I changed uint8_t* to std::vector<uint8_t>.</p>

<p style="padding: 0; margin: 8px;">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?..</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">FYI, we have a similar construction in poppler with std::shared_ptr and a custom deleter to handle cmsHPROFILEs, cf. <a href="https://gitlab.freedesktop.org/poppler/poppler/-/commit/5927e0b08f1ad6868d04cb209ee1e17b4ac07b70#966756b758fb5d7e46114c6835faf491d5b92582_195_195" class="remarkup-link" target="_blank" rel="noreferrer">https://gitlab.freedesktop.org/poppler/poppler/-/commit/5927e0b08f1ad6868d04cb209ee1e17b4ac07b70#966756b758fb5d7e46114c6835faf491d5b92582_195_195</a>.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21076">https://phabricator.kde.org/D21076</a></div></div><br /><div><strong>To: </strong>vitaliyf, zzag, rjvbb<br /><strong>Cc: </strong>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<br /></div>