Bug 311319 - Color correction breaks EffectFrames
Summary: Color correction breaks EffectFrames
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: colorcorrection (show other bugs)
Version: git master
Platform: unspecified Other
: NOR normal
Target Milestone: ---
Assignee: Casian Andrei
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-07 19:22 UTC by Thomas Lübking
Modified: 2013-01-13 17:13 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch that prevents color correction from changing the blending function (1.34 KB, patch)
2012-12-16 09:04 UTC, Casian Andrei
Details
For fixing the GL invalid operation (11.47 KB, patch)
2012-12-16 09:26 UTC, Casian Andrei
Details
Fixes the GL invalid operation (12.34 KB, patch)
2013-01-05 04:05 UTC, Casian Andrei
Details
Should fix the effect frames (2.90 KB, patch)
2013-01-05 04:07 UTC, Casian Andrei
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Lübking 2012-12-07 19:22:56 UTC
see bug #296289 - it's apparently similar to the false mimpap invocation

Reproducible: Always
Comment 1 Thomas Lübking 2012-12-07 19:29:32 UTC
nvidia blob behavior:

- effect frames (for QT) don't show up at all.
- after deactivating color correction, there's an opaque white rect in the upper right corner
- after re-enabling color correction, the rect remains and effect frames show up, but either color inverted or at least faaar too bright (using a dark plasma theme "Opaquity", the outline is usually translucent black and now translucent white)
Comment 2 Thomas Lübking 2012-12-08 01:37:41 UTC
Can you guys please inspect your  ~/.xsession-errors?
You'll likely find *masses* of

   kwin(11518) KWin::checkGLError: GL error ( PostPaint ):  "GL_INVALID_OPERATION"

but do you also get sth. like (the PID will oc. differ) one (or more?)

    kwin(6492) KWin::checkGLError: GL error ( setupCCTexture ):  "GL_INVALID_VALUE"

void ColorCorrectionPrivate::setupCCTexture(GLuint texture, const Clut& clut)
glBindTexture(GL_TEXTURE_3D, texture);

fails here for m_dummyCCTexture

Also ::setupCCTextures is called twice.
Comment 3 Janek Bevendorff 2012-12-08 10:05:18 UTC
When I reenable Color Correction I get this:

kwin(681) KWin::ColorServerInterface::callFinishedSlot: 
QDBusError("org.freedesktop.DBus.Error.UnknownObject", "No such object 
path '/modules/kolorserver'")
kwin(681): Update of color profiles failed
kwin(681) KWin::ColorServerInterface::callFinishedSlot: 
QDBusError("org.freedesktop.DBus.Error.UnknownObject", "No such object 
path '/modules/kolorserver'")
kwin(681) KWin::ColorServerInterface::callFinishedSlot: 
QDBusError("org.freedesktop.DBus.Error.UnknownObject", "No such object 
path '/modules/kolorserver'")
kwin(681) KWin::checkGLError: GL error ( setupCCTexture ):  
"GL_INVALID_VALUE"

and when I hit Alt+Tab, I get this:

Object::connect: No such slot IconItem::implicitWidthChanged()
Object::connect:  (sender name:   'kwin')
Object::connect: No such slot IconItem::implicitHeightChanged()
Object::connect:  (sender name:   'kwin')
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:173: 
TypeError: Result of expression 'parent.margins' [undefined] is not an 
object.
Object::connect: No such slot IconItem::implicitWidthChanged()
Object::connect:  (sender name:   'kwin')
Object::connect: No such slot IconItem::implicitHeightChanged()
Object::connect:  (sender name:   'kwin')
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:173: 
TypeError: Result of expression 'parent.margins' [undefined] is not an 
object.
Object::connect: No such slot IconItem::implicitWidthChanged()
Object::connect:  (sender name:   'kwin')
Object::connect: No such slot IconItem::implicitHeightChanged()
Object::connect:  (sender name:   'kwin')
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:173: 
TypeError: Result of expression 'parent.margins' [undefined] is not an 
object.
Object::connect: No such slot IconItem::implicitWidthChanged()
Object::connect:  (sender name:   'kwin')
Object::connect: No such slot IconItem::implicitHeightChanged()
Object::connect:  (sender name:   'kwin')
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:173: 
TypeError: Result of expression 'parent.margins' [undefined] is not an 
object.
Object::connect: No such slot IconItem::implicitWidthChanged()
Object::connect:  (sender name:   'kwin')
Object::connect: No such slot IconItem::implicitHeightChanged()
Object::connect:  (sender name:   'kwin')
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:107:13: 
QML Item: Binding loop detected for property "height"
file:///home-accel/janek/.kde4/share/apps/kwin/tabbox/presentwindowsclone-opaque/contents/code/presentwindowsclone.qml:173: 
TypeError: Result of expression 'parent.margins' [undefined] is not an 
object.

When I drag a window to the edges of the screen of tiling, no special 
message is written.
Comment 4 Dario Cambié 2012-12-08 13:07:29 UTC
@Janek Bevendorff
 kwin(681) KWin::ColorServerInterface::callFinishedSlot: QDBusError("org.freedesktop.DBus.Error.UnknownObject", "No such object path '/modules/kolorserver'")

Have you installed kolor-manager?

(I will check later, I'm on another pc now)
Comment 5 Janek Bevendorff 2012-12-08 13:13:20 UTC
Oh, right. Stupid me. I though I had (switched distributions recently).
So with kolor-manager installed, the first message does not appear. But the second bulk of messages (when invoking Alt+Tab) stays the same.
Comment 6 Thomas Lübking 2012-12-08 13:16:43 UTC
Actually the only important question is whether this:

kwin(681) KWin::checkGLError: GL error ( setupCCTexture ):  
"GL_INVALID_VALUE"

remains.

The messages of alt+tab seem some (but definitively unrelated) defects in th QML code affecting geometries, but not visuals.
Comment 7 Janek Bevendorff 2012-12-08 13:21:17 UTC
Strange. I just realized that the

kwin(681) KWin::ColorServerInterface::callFinishedSlot: 
QDBusError("org.freedesktop.DBus.Error.UnknownObject", "No such object 
path '/modules/kolorserver'")

actually isn't gone, although I have definitely installed kolor-manager.
The GL_INVALID_VALUE message, though, seems to be gone. At least I 
can't reproduce it right now.

The white boxes remain.
Comment 8 Dario Cambié 2012-12-08 18:26:56 UTC
With color correction on I get:

 KWin::checkGLError: GL error ( setupCCTexture ):  "GL_INVALID_VALUE" 
-> Red  1.000, Green  1.000, Blue  1.000
<- Red  1.000, Green  1.000, Blue  1.000
klauncher(12433) KConfigGroup::readXdgListEntry: List entry OnlyShowIn in "/etc/xdg/autostart/gnome-screensaver.desktop" is not compliant with XDG standard (missing trailing semicolon). 
kded(12435) ksOyMessage: WARNING 0,180000: [-1] oyranos_monitor_x11.c:1164 oyX1Monitor_getScreenGeometry_() Xinerama request failed 
kded(12435) ksOyMessage: WARNING 0,180000: [-1] oyranos_monitor_x11.c:912 oyX1MonitorProfileUnset() Error getting atom "_ICC_PROFILE_1" 
QDBusConnection: session D-Bus connection created before QCoreApplication. Application may misbehave.
QDBusConnection: session D-Bus connection created before QCoreApplication. Application may misbehave.
kded(12435) ksOyMessage: WARNING 0,190000: oyOptions_s[165]="oyOptions_s" oyranos_cmm_oyX1.c:875 oyX1Configs_Modify() 
  Could not obtain _ICC_DEVICE_PROFILE(_xxx) information for :0.0 
kded(12435) ksOyMessage: WARNING 0,190000: oyOptions_s[182]="oyOptions_s" oyranos_cmm_oyX1.c:875 oyX1Configs_Modify() 
  Could not obtain _ICC_PROFILE(_xxx) information for :0.0 
QDBusObjectPath: invalid path ""
sh: 1: xcalib: not found
kded(12435) ksOyMessage: WARNING 0,220000: [-1] oyranos_monitor_x11.c:767 oyX1MonitorProfileSetup() No monitor gamma curves by profile: CMO-5926-_edid.icc 
kded(12435) ksOyMessage: WARNING 0,220000: [-1] oyranos_monitor_x11.c:837 oyX1MonitorProfileSetup() found issues bytes_after_return: 1
ImageProvider supports Pixmap type but has not implemented requestPixmap()

on kwin startup (if color correction is enabled), or as soon as color correction is enabled (when disabled at startup).
(I've now installed xcalib to remove the "xcalib: not found" part, but nothing significant changed)

No errors shown at Alt-tab, neither in kwin output nor in .xsession-errors (other then the QML one, on the first alt-tab)
Comment 9 Thomas Lübking 2012-12-08 19:18:34 UTC
> KWin::checkGLError: GL error ( setupCCTexture ):  "GL_INVALID_VALUE"

This alone means that setting up the (dummy, i presume. it's at least here) texture fails, so it's further usage in the shaders isn't safe, esp. since apparently binding it seems to fail with invalid value, actually implying a non glGenTextures created ID - what does not seem the case here, though (no error on glGenTex and the values match)

Another funny thing is that it sometimes(?) creates two dummy textures (there're two screens, but one's disabled) and the second one does not cause this, yet i get masses of "GL_INVALID_OPERATION" for PostPaint related to the color correction
Comment 10 Casian Andrei 2012-12-10 08:38:39 UTC
Sorry for late reply, I was offline since Friday.

Currently I have been able only to quickly read the above comments. My first impression is that there might be some issue for some drivers regarding 3D textures, which triggers that invalid operation, which is not handled correctly within the color correction code.

So, I think it would be useful to identify for which drivers or hardware this is happening.

And the code should handle the error properly, so when there is a gl invalid operation over there, color correction needs to be immediately disabled without ugly side effects.

So, 
kwin(6492) KWin::checkGLError: GL error ( setupCCTexture ): "GL_INVALID_VALUE"
should be treated properly in the code.
Comment 11 Casian Andrei 2012-12-10 08:54:10 UTC
@ kwinglcolorcorrection.cpp:562 ->
// TODO Handle errors (what if a texture isn't generated?)
checkGLError("setupCCTextures");

Ok, this obviously needs to be implemented - I will try to do so during this weekend, and then we should see if the bug still persists after that.
Comment 12 Thomas Lübking 2012-12-10 10:59:34 UTC
janek:
OpenGL vendor string: nouveau
OpenGL renderer string: Gallium 0.4 on NV92
OpenGL version string: 3.0 Mesa 8.1-devel (git-4f109ca)
OpenGL shading language version string: 1.30

dario:
OpenGL renderer string:                 Mesa DRI Intel(R) Sandybridge Mobile 
OpenGL version string:                  3.0 Mesa 9.0
OpenGL shading language version string: 1.30
Driver:                                 Intel
GPU class:                              SandyBridge
OpenGL version:                         3.0
GLSL version:                           1.30
Mesa version:                           9.0

thomas:
nvidia blob 3.10.19 / mesa 9.01

that's pretty much "all but ati" and 3d textures are supported here... (otherwise)
Comment 13 Casian Andrei 2012-12-10 12:50:55 UTC
Makes sense, I only tested with ati and mesa 7 and 9, with the open source driver. This means that there is something wrong with the way the code attempts to use the 3d textures, and it works accidentally on ati stuff. :(
Comment 14 Janek Bevendorff 2012-12-10 20:37:03 UTC
Yep, I just changed Nouveau for the NVIDIA Blob and the problem still 
persists.
Comment 15 Kai-Uwe Behrmann 2012-12-15 15:36:59 UTC
Here a possibly related change inside the  Oyranos/KolorServer stack.

A fix for XRandR with Xinerama setups is now in Oyranos git cmake branch, which affects KolorServer:
http://www.oyranos.org/scm?p=oyranos.git;a=commit;h=a4f157beb6c60855f64d011cb2ad2f93071da461

I am quite astonished to not have seen the XRandR bug n Oyranos earlier. Anyway, the changes are tested with intel and works as usual with the nvidia driver for dual screens. However, I've not yet seen the above mentioned GL errors.
Comment 16 Casian Andrei 2012-12-15 17:01:36 UTC
Investigated a bit today and that gl invalid operation shows up for me too. So, it might not be the cause of the problem. At least I can track down and get rid of it.

I tried implementing the error handling today but eventually I got a bit stuck with a crash and had no time to investigate that, for now.

I wonder whether that XRandR bug is the cause of this bug :-??

Anyway, more investigating is needed.
Comment 17 Casian Andrei 2012-12-16 09:04:59 UTC
Created attachment 75858 [details]
Patch that prevents color correction from changing the blending function

This might change the observed behavior and indicate whether there is a problem with the blending function being changed by the color correction.
Comment 18 Casian Andrei 2012-12-16 09:26:44 UTC
Created attachment 75860 [details]
For fixing the GL invalid operation

This fixes the GL invalid operation, but most likely this is not causing the problems. You need to apply it before the 0001 patch.
Comment 19 Dario Cambié 2012-12-16 10:07:32 UTC
For me with patch attached (order 0002 then 0001) the problem of white windows in Alt+tab is still there, the GL-ERROR no more. Furthermore when enabling the color correction the shadow around the windows became "strange" (will attach a screenshot soon).

Oh, and I get different errors in the console running "kwin --replace"
KWin::x11ErrorHandler: kwin: X Error ( "error: BadDrawable [9], request: X_GetGeometry[14]" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDrawable [9], request: RenderCreatePicture[RENDER+4]" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDrawable [9], request: X_GetGeometry[14]" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDrawable [9], request: RenderCreatePicture[RENDER+4]" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDrawable [9], request: X_GetGeometry[14]" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDrawable [9], request: RenderCreatePicture[RENDER+4]" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDamage [DAMAGE+0], request: XDamageDestroy[DAMAGE+2], resource: 0x380006c" ) 
kwin(2574) KWin::x11ErrorHandler: kwin: X Error ( "error: BadDamage [DAMAGE+0], request: XDamageDestroy[DAMAGE+2], resource: 0x38000af" ) 

And ~/.xsession-errors with *tons* (currently >100MBof
Using fd 4Error opening terminal: unknown.
Comment 20 Dario Cambié 2012-12-16 10:12:27 UTC
(ops, sorry errors in .xsession-errors were unrelated, my own fault)
Comment 21 Casian Andrei 2012-12-16 13:04:45 UTC
The strange shadow around the windows is to be expected with that patch applied, although I currently do not know why it happens :( . So, you should remove the 001 patch if it did not fix anything.

And those errors from kwin --replace I get them too, I don't think they are related to our issue.

So the problem is more complicated, I need to read the code next week and figure out what are the effect frames about and how they work, and then try to see what is happening.

A couple of screenshots would be awesome :) .
Comment 22 Dario Cambié 2012-12-17 13:50:20 UTC
Couldn't attach screenshots due to file size limit (1000kb, png screenshot being 1,8MB ca), so here they are:
http://dl.dropbox.com/u/905879/varie/screenshots.zip
Comment 23 Casian Andrei 2012-12-17 15:23:50 UTC
Thanks for the screenshots!

Now I have a much better idea of what is happening. I think I saw this a couple of times when I was writing the code but I forgot to investigate :(
Comment 24 Casian Andrei 2013-01-05 04:05:13 UTC
Created attachment 76195 [details]
Fixes the GL invalid operation

Updated fix, needed for the premultiplied alpha issue patch.
Comment 25 Casian Andrei 2013-01-05 04:07:33 UTC
Created attachment 76196 [details]
Should fix the effect frames

There was an issue with the premultipled alpha when color correcting.

It should have been obvious...

See https://git.reviewboard.kde.org/r/108189/

This patch should be applied after 76195 ("Fixes the GL invalid operation")
Comment 26 Dario Cambié 2013-01-05 09:38:34 UTC
The two patches fix the problem for me. Thanks :)
Comment 27 Casian Andrei 2013-01-13 17:13:38 UTC
Git commit 6cd210f611b7527922d62bc389dfe38b49566349 by Casian Andrei.
Committed on 05/01/2013 at 04:39.
Pushed by casianandrei into branch 'master'.

Fix premultiplied alpha issue with color correction

When correcting a color that was with premultiplied alpha, the alpha
value was not multiplied back again as a final step. This was breaking
color correction when the blend function was GL_ONE,
GL_ONE_MINUS_SRC_ALPHA. The blend function was changed for normal
windows (a workaround), but not for effect frames, i.e. the effect
frames were broken with color correction enabled.

Removes the blend function workaround.

Removes a useless setupForOutput.
REVIEW: 108189

M  +1    -1    kwin/libkwineffects/kwinglcolorcorrection.cpp
M  +4    -10   kwin/scene_opengl.cpp

http://commits.kde.org/kde-workspace/6cd210f611b7527922d62bc389dfe38b49566349