Merge lp:~nwkj86/compiz/add-background-conf-to-switcher into lp:compiz/0.9.12
Proposed by
Artur
Status: | Merged |
---|---|
Approved by: | MC Return |
Approved revision: | 3974 |
Merged at revision: | 3975 |
Proposed branch: | lp:~nwkj86/compiz/add-background-conf-to-switcher |
Merge into: | lp:compiz/0.9.12 |
Diff against target: |
161 lines (+66/-2) 6 files modified
plugins/compiztoolbox/include/compiztoolbox/compiztoolbox.h (+2/-1) plugins/compiztoolbox/src/compiztoolbox.cpp (+18/-0) plugins/staticswitcher/src/staticswitcher.cpp (+5/-0) plugins/staticswitcher/staticswitcher.xml.in (+18/-0) plugins/switcher/src/switcher.cpp (+5/-1) plugins/switcher/switcher.xml.in (+18/-0) |
To merge this branch: | bzr merge lp:~nwkj86/compiz/add-background-conf-to-switcher |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
MC Return | Approve | ||
Review via email: mp+268556@code.launchpad.net |
Commit message
Allow to select background color for switcher and staticswitcher.
Description of the change
Allow user to set & select background color for switcher and staticswitcher.
Right now setBackground() function is duplicated. I'm looking for the good common place to put it there.
To post a comment you must log in.
Hey Artur,
Thanks for your contribution.
Here a review:
1. You need to click "Set commit message" on this page above and enter a commit message there, otherwise this won't merge correctly.
2. A good common place to put setBackground () to would be the BaseSwitchScreen class in compiztoolbox.cpp to avoid code duplication.
You could then use the optionGet () variables as function arguments in your setBackground () function and pass those via update notification to your function like this:
optionSetUs eBackgroundColo rNotify (boost::bind (&BaseSwitchScr een::updateBack ground, this, optionGetUseBac kgroundColor (), optionGetBackgr oundColor ())); ckgroundColorNo tify (boost::bind (&BaseSwitchScr een::updateBack ground, this, optionGetUseBac kgroundColor (), optionGetBackgr oundColor ()));
optionSetBa
The function in BaseSwitchScreen could then look like this: n::updateBackgr ound (bool useBackgoundColor,
void
BaseSwitchScree
unsigned short backgroundColor [])
3. unsigned short alpha seems to be unused. I know XColor does not care about transparency, but it would be nice if the alpha value would not be ignored, so semi-transparency of the background would also be possible...
4. Please insert a whitespace between function name and brackets as this is the style Compiz (mostly ;)) uses.
5. Typo here: 87 + <_short>Bacground Color</_short>
6. It seems that once the background color has been enabled, it cannot be disabled anymore - setting unsigned long background_pixel = 0ul; seems not to be enough to reset the background back...
Also you should not set background_pixel to 0ul in any case as this value gets overwritten before usage anyway in the case the background color gets used...
Does disabling the background color work for you ?