Bug 286933 - Some comboboxes in GIMP have wrong background
Summary: Some comboboxes in GIMP have wrong background
Status: CLOSED FIXED
Alias: None
Product: Oxygen
Classification: Plasma
Component: gtk2-engine (show other bugs)
Version: unspecified
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-18 16:09 UTC by Ruslan Kabatsayev
Modified: 2012-05-15 04:09 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
alternative patch (1021 bytes, application/octet-stream)
2011-11-18 18:36 UTC, Hugo Pereira Da Costa
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ruslan Kabatsayev 2011-11-18 16:09:08 UTC
Version:           unspecified
OS:                Linux

See screenshot: http://simplest-image-hosting.net/png-0-screenshot-11182011-080421-pm

It's GIMP 2.4. Maybe the same is for 2.6, I didn't check.
It's a regression:

a584e63fcd2582264ecbe042956e34e0ed518202 is the first bad commit
commit a584e63fcd2582264ecbe042956e34e0ed518202
Author: Hugo Pereira Da Costa <hugo@oxygen-icons.org>
Date:   Sun May 22 21:36:00 2011 +0200

    Check for parent groupboxes when rendering eventBox.
    CCBUG: 273815

:040000 040000 1b9940219f9a7963c2470b51dbae68bf228f6866 bb0431f0ab1400878306c267ff6dfe2bc0bfc7ff M      src

I'm not sure what exactly is wrong in this commit. Oxygen-gtk seems to have detected combo's parent as a groupbox while it's not (it's just a GtkFrame without frame :) ).

Reproducible: Always

Steps to Reproduce:
Launch GIMP (at least 2.4)
Open Preferences

Actual Results:  
Wrong background for some comboboxes

Expected Results:  
Correct bg
Comment 1 Hugo Pereira Da Costa 2011-11-18 17:51:23 UTC
interestingly, can't reproduce with gimp-2.6.11
See: 
http://wstaw.org/m/2011/11/18/plasma-desktoppN2811.jpg
Comment 2 Hugo Pereira Da Costa 2011-11-18 17:56:36 UTC
Checking our code, I wonder if there is not some "custom" widget involved on the gimp side: a GtkFrame, with a label, and with gtk_frame_get_shadow_type == GTK_SHADOW_OUT even though it is not rendered. (that's the only way a parent "groupbox" could be detected). 

Could that be ?
Comment 3 Ruslan Kabatsayev 2011-11-18 18:08:50 UTC
Yeah, shadow_type is OUT, and the labels are present.
But we can blacklist GimpFrame, which is the name for this widget (returned by G_OBJECT_TYPE_NAME()).
I think I'll push this.
Comment 4 Ruslan Kabatsayev 2011-11-18 18:18:05 UTC
Git commit 7283bcde2d8463caf16403063a8aad0e4b88d4fd by Ruslan Kabatsayev.
Committed on 18/11/2011 at 19:17.
Pushed by kabatsayev into branch 'master'.

Don't consider GimpFrame to be group box
CCBUG: 286933

M  +2    -1    src/oxygengtkutils.h

http://commits.kde.org/oxygen-gtk/7283bcde2d8463caf16403063a8aad0e4b88d4fd
Comment 5 Ruslan Kabatsayev 2011-11-18 18:18:58 UTC
The above fixes it.
Comment 6 Hugo Pereira Da Costa 2011-11-18 18:20:46 UTC
ok.
Comment 7 Hugo Pereira Da Costa 2011-11-18 18:24:29 UTC
... mmm. So I'm not so much in favor of yet another blacklist :(
Maybe, we can actually better tag groupboxes, as frames that satisfy the above conditions, and that have (at least once) been actually rendered as groupboxes... Since I have gimp2.4 locally compiled here, and can reproduce the issue, I'll see if I can do that. (this could possibly fix other similar bugs).
Comment 8 Hugo Pereira Da Costa 2011-11-18 18:36:57 UTC
Created attachment 65828 [details]
alternative patch

this patch uses the existing (but unused so far) Oxygen::GroupBoxEngine to keep track of flagged groupboxes for which the frame is actually rendered at least once.
This is then used to check on renderGroupBackground.
Ruslan: could you test in place of your change ? 
Double check for regressions in other places ? 
So far I've found that 
- it does fix the issue with gimp-2.4
- it does not introduce regressions
Since it is more generic than the previous patch, and can possibly fix other issues, I'd rather revert your change and commit + backport this change.
Comments ?
Comment 9 Hugo Pereira Da Costa 2011-11-18 18:41:05 UTC
ok. In fact after double checking, this strategy is already used (succesfully) four groupbox *holes*.
So if it works here, should work elsewhere.
Will push.
Comment 10 Ruslan Kabatsayev 2011-11-18 18:49:30 UTC
Yes, it works, and in fact is a nicer way to fix this. In fact, I wanted to suggest similar idea, but thought it'd take too much work for a single bug (didn't know/think of existing infrastructure).
Comment 11 Hugo Pereira Da Costa 2011-11-18 18:57:13 UTC
Git commit 6c93357f1c44ef76f9382465c214d87d496f0c4d by Hugo Pereira Da Costa.
Committed on 18/11/2011 at 19:39.
Pushed by hpereiradacosta into branch 'master'.

check that parent groupbox is registered to engine before rendering groupbox
background.

CCBUG: 286933

M  +6    -13   src/oxygenstyle.cpp
M  +1    -0    src/oxygenstylewrapper.cpp

http://commits.kde.org/oxygen-gtk/6c93357f1c44ef76f9382465c214d87d496f0c4d
Comment 12 Hugo Pereira Da Costa 2011-11-18 18:59:42 UTC
Git commit b749ebef93ce808fb89376f22ff3bb1dda72c482 by Hugo Pereira Da Costa.
Committed on 18/11/2011 at 19:39.
Pushed by hpereiradacosta into branch '1.1'.

check that parent groupbox is registered to engine before rendering groupbox
background.

CCBUG: 286933

M  +6    -13   src/oxygenstyle.cpp
M  +1    -0    src/oxygenstylewrapper.cpp

http://commits.kde.org/oxygen-gtk/b749ebef93ce808fb89376f22ff3bb1dda72c482