[GSoC] KWin colour management

[Casian Andrei]

I forgot about the KWin ML :(

And I realized the complications for the strange idea regarding
GLShader are not even remotely necessary, bad thinking on my part.
Explanations below.

2012/3/22 Casian Andrei <skeletk13 at gmail.com>:
> Thank you for all this feedback!
>
> Meanwhile I have been documenting myself and looking into relevant
> code (and following this thread). I can't argue on the complex
> technical issues and decisions.
>
> 2012/3/20 Martin Graesslin <mgraesslin at kde.org>:
>> There is more into it: first of all KWin currently does not distinguish between
>> screens during rendering. To properly have screen aware color correction the
>> complete compositor has to be made screen aware. The repaint loop has to be
>> split into multiple rendering passes - one for each screen. This is quite a
>> change in the way how KWin renders, but might be a useful change.
> Okay, this would involve complicated changes, consequently it should
> be like a separate sub-project. At the moment I don't know well enough
> how KWin renders from my limited current understanding of the code,
> but there will be enough time to discuss and see how this could be
> done. It is good if this change is possible without massive intrusive
> modifications throughout KWin.
>
>>
>> As a second step all fragment shaders need to be adjusted to do the color
>> correction. This has to be done extremely efficient. This is a rather critical
>> code path especially for low-end hardware (think of old Intel GPUs). Given the
>> constraints of the GPUs a dynamic feature activation is not possible.
> I have a rather crazy idea about how to achieve this for *all* of the
> fragment shaders without modifying each one. It might be a silly idea,
> but it's worth proposing it.
>
> Every one of these fragment shaders respect the following structure:
> 1. declarations, functions, uniforms, whatever
> 2. void main() {
> 3. fragment shader functionality (whatever the shader does)
> 4. gl_FragColor = some_expression
> 5. } // closing bracket, end of shader
>
> Now, colour correction would require this gl_FragColor result to be
> different, to be altered. If I understand the shader assembly thing in
> CompICC, it would be an alpha demultiply, a 3d texture lookup to
> correct the color, and an alpha remultiply. Let's assume this code is
> kept in a GLSL function, namely "vec4 correctColor(vec4 c)". This
> function will require a sampler3D.
>
> To make the above code do the color correction, we need to do the following:
> * insert the sampler3D declaration and the function definition just
> before main()
> * alter gl_FragColor = some_expression --> gl_FragColor =
> correctColor(some_expression)
>
> These alterations are not *that* complicated, and they shouldn't
> introduce any strange instabilities. Similar, but simpler alterations
> are already made in GLShader::compile() at kwinglutils.cpp around line
> 295. Colour correction could be easily disabled at this point.
>
> The tricky part is how to know what window / screen we're compiling
> the shader for. This can be done with some little complications, maybe
> an extra parameter, or some kind of member for GLShader, or some evil
> static variable / member.
We don't need to know that! Distinguishing between windows and screen
can be done just by binding different lookup textures, no need for
extra parameters or useless complications. The altered shader just
tries to do colour correction. If it gets a default sRGB->sRGB lookup
texture, then no colour correction. And I think there are many ways to
disable it altogether - something like that kwin_shader_debug
definition.
>
> The advantage is that it avoids modifying all the fragment shaders
> everywhere, but it brings these (i hope slight) complications to
> GLShader. What do you think about this?
So, minus the complications.
>
> Oh, and what does "dynamic feature activation" mean? If it means what
> I think it means, then it can be handled by the above idea.
>
>>
>> What is in general important to know is that we have not had the best
>> experience with GSoC students doing work on the core of KWin. Given that I
>> proposed guidelines for future feature additions to KWin by non-core
>> developers [1].
> Those are really strict guidelines, but I guess they are well based
> given the past experiences, so I won't complain. There are a couple of
> points that this possible project does not appear to respect, but
> those are unclear and debatable when applied to this, in my view.
> However, it looks to me as if no further significant features can be
> added to KWin if you respect all those points by the letter.
>
> Probably the colour correction code can be kept isolated in a couple
> of separate files, without interfering much with the normal KWin
> workings. This code would be responsible with getting the lookup
> textures for the monitors (or screens) and the windows (possibly). It
> may turn complicated and large, but at least it would be isolated.
>
>>
>> Given recent discussions on this mailinglist about Oyranos and colord I am
>> very unsure whether I want any color management relevant code in KWin at the
>> moment. I will definitely not accept any code supporting only one of the two
>> systems and any additional build or run-time dependency to KWin will not be
>> accepted.
> Alright, so this colord is a must, eventually. I should document
> myself about colord too. But at least I could ask not to make it part
> of the objectives for the summer.
>>
>> In general there seems to be agreement that color management has to be done
>> inside the toolkit/application and not inside the compositor. A fully color
>> corrected compositor seems feasible to me, but one where some applications
>> need to start opting-out of being color corrected is nothing I want to see in
>> KWin as it adds significant complexity and overhead to the rendering process.
>> This is something the Oyranos community has to decide on how they want to have
>> that handled.
> This is still a hot topic so I would prefer not to comment at the
> moment, and the decision is yours anyway.
>
> Best regards,
> Casian