This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107754/

On December 23rd, 2012, 8:55 a.m., Martin Gräßlin wrote:

kwin/scene_opengl.cpp (Diff revision 3)
void SceneOpenGL2::slotColorCorrectionError()
582
void SceneOpenGL2::slotColorCorrectionError()
583
{
584
    // This should also trigger slotColorCorrectedChanged()
585
    options->setColorCorrected(false);
586
}
why do you not define this slot in Options? Oh and please add a:
// TODO: Qt5: replace by lambda

On January 3rd, 2013, 7:04 p.m., Casian Andrei wrote:

It looks to me like it does not fit in Options - there are no slots for errors there, I wouldn't know even where to place it there. SceneOpenGL2 owns the color correction object, so it makes sense for it to handle the error.

With that lambda feature, this slot will dissappear with Qt5 anyway :)
Q_SLOTS:
void disableColorCorrection();

under all those Q_SIGNALS: and above private.

The reason that there's no slot atm. is that we don't alter the *options* at runtime at all.

I'd say the scene should know (and tell on request dbug dbus interface request pending ...) whether it's color corrected and initiallize that from the options and not touch options.

@Martin
The pointer should (in this context) maybe ro to stress that?

- Thomas


On January 3rd, 2013, 7:04 p.m., Casian Andrei wrote:

Review request for kwin, Thomas Lübking and Martin Gräßlin.
By Casian Andrei.

Updated Jan. 3, 2013, 7:04 p.m.

Description

Abort color correction initialization and disable it in case of errors
    
    Checks are now performed for GL errors and in case of errors everything
    is aborted. The error handling mechanism introduced for this purpose
    somewhat improves the color correction code.

Tracked down and fixed a GL invalid operation when first attempting to set the lookup texture uniform for color correction.

Testing

Tested when a GL invalid operation error occurs - color correction is disabled and will not be enabled again because of the error. However, the check box in KCM remains checked - but this is ok I think.

Tested when no GL invalid operation is detected when doing color correction stuff. Everything works fine from my point of view.

Diffs

  • kwin/libkwineffects/kwinglcolorcorrection.h (ec080c0)
  • kwin/libkwineffects/kwinglcolorcorrection.cpp (b5127e6)
  • kwin/libkwineffects/kwinglcolorcorrection_p.h (f54aa49)
  • kwin/scene_opengl.h (e6142c0)
  • kwin/scene_opengl.cpp (147a248)

View Diff