Merge lp:~mc-return/compiz/compiz.merge-plugin-simple-animations into lp:compiz/0.9.8
| Status: | Superseded |
|---|---|
| Proposed branch: | lp:~mc-return/compiz/compiz.merge-plugin-simple-animations |
| Merge into: | lp:compiz/0.9.8 |
| Diff against target: |
1778 lines (+1716/-0) 12 files modified
plugins/simple-animations/CMakeLists.txt (+8/-0) plugins/simple-animations/animationsim.xml.in (+235/-0) plugins/simple-animations/src/animationsim.cpp (+160/-0) plugins/simple-animations/src/animationsim.h (+405/-0) plugins/simple-animations/src/bounce.cpp (+76/-0) plugins/simple-animations/src/expand-piecewise.cpp (+90/-0) plugins/simple-animations/src/expand.cpp (+70/-0) plugins/simple-animations/src/fan.cpp (+51/-0) plugins/simple-animations/src/flyin.cpp (+86/-0) plugins/simple-animations/src/pulse.cpp (+50/-0) plugins/simple-animations/src/rotatein.cpp (+210/-0) plugins/simple-animations/src/sheet.cpp (+275/-0) |
| To merge this branch: | bzr merge lp:~mc-return/compiz/compiz.merge-plugin-simple-animations |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Daniel van Vugt | Needs Fixing on 2012-08-28 | ||
| MC Return | Resubmit on 2012-08-11 | ||
| Sam Spilsbury | 2012-07-16 | Needs Fixing on 2012-08-11 | |
|
Review via email:
|
|||
This proposal has been superseded by a proposal from 2013-04-02.
Commit Message
Added the plug-in "Simple Animations" converted from git to bzr (including full history) to lp:compiz
Description of the Change
Adds the plug-in "Simple Animations" converted from git to bzr (including full history) to lp:compiz
| MC Return (mc-return) wrote : | # |
| Sam Spilsbury (smspillaz) wrote : | # |
You just need to implement requiresTransfo
just something like
bool requiresTransfo
| MC Return (mc-return) wrote : | # |
Now it compiles :)
Some warnings remain:
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
[100%] Building CXX object plugins/
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
- 3288. By MC Return on 2012-07-16
-
Rebased on latest lp:compiz (r3286)
- 3289. By MC Return on 2012-07-16
-
Fixed -Wmaybe-
uninitialized build warnings - 3290. By MC Return on 2012-07-18
-
Fixed variables 'originX' and 'originY' set but not used [-Wunused-
but-set- variable] compiler warnings in RotateInAnim: :prePaintWindow () and RotateInAnim: :postPaintWindo w () (rotatein.cpp) - 3291. By MC Return on 2012-07-18
-
Reverted change in last commit
| MC Return (mc-return) wrote : | # |
Remaining warnings:
compiz.
compiz.
compiz.
compiz.
compiz.
compiz.
- 3292. By MC Return on 2012-07-22
-
Fixed variables 'originX' and 'originY' set but not used [-Wunused-
but-set- variable] compiler warnings in RotateInAnim: :prePaintWindow () and RotateInAnim: :postPaintWindo w () (rotatein.cpp) by removing the 'originX' and 'originY' variables and the unnecessary, unused calculations with those variables
| MC Return (mc-return) wrote : | # |
I've also fixed the [-Wmaybe-
| MC Return (mc-return) wrote : | # |
Tested all of the simple-animations (even with slow animations on).
Everything works smooth and the animations all look really nice. :)
The "Sheet" animation seems to "simplify" the window content - that is the only (minor) animation issue I've found with those.
Also somehow in the "Animation" plug-in the new simple-animations are just available for opening or closing animations, but not for minimizing, but I guess it is designed this way (have not analyzed this behavior yet).
Seems this branch is quite mature now.
| Sam Spilsbury (smspillaz) wrote : | # |
Code wise this looks OK, could you tidy up the following things?
15 === added file 'plugins/
16 --- plugins/
17 +++ plugins/
18 @@ -0,0 +1,1 @@
19 +0.9.5.0
That can be removed
565 + void step () { TransformAnim::step (); }
566 + bool updateBBUsed () { return true; }
567 + void updateBB (CompOutput &output) { TransformAnim:
568 + void applyTransform ();
569 + bool requiresTransfo
570 +
The indentation here is not correct, it can be 2 indents (1 8 wide tab) for updateBBUsed and requiresTransfo
+ bool requiresTransfo
Ditto
| MC Return (mc-return) wrote : | # |
> Code wise this looks OK, could you tidy up the following things?
Sure.
> 15 === added file 'plugins/
> 16 --- plugins/
> 17 +++ plugins/
> 18 @@ -0,0 +1,1 @@
> 19 +0.9.5.0
>
> That can be removed
Removed in r3294.
> 565 + void step () { TransformAnim::step (); }
> 566 + bool updateBBUsed () { return true; }
> 567 + void updateBB (CompOutput &output) { TransformAnim:
> (output); }
> 568 + void applyTransform ();
> 569 + bool requiresTransfo
> 570 +
>
> The indentation here is not correct, it can be 2 indents (1 8 wide tab) for
> updateBBUsed and requiresTransfo
>
> + bool requiresTransfo
>
> Ditto
Done.
| MC Return (mc-return) wrote : | # |
I also ran cppcheck on this plug-in and found a few issues I've now fixed also.
1. Reduced the scope of the float distance
2. Removed redundant condition check of the angle (if the angle is more than 270 it will always be bigger than 90), but please check that again, because it might be a typo and it could also mean that the angle should be between 90 and 270.
The rotatein animation works perfectly though...
3. Not fixed yet: (warning) Member variable 'ExtensionPlugi
Should this be fixed ?
- 3299. By MC Return on 2012-08-11
-
Fixed begginning -> beginning typo in tooltip
- 3300. By MC Return on 2012-08-12
-
Removed redundant newlines
- 3301. By MC Return on 2012-08-27
-
Added support for unminimize animations and simplified the code by also using the new class AnimEffectUsedFor like it was done for animationaddon.cpp already
| Daniel van Vugt (vanvugt) wrote : | # |
/home/dan/
/home/dan/
/home/dan/
/home/dan/
/home/dan/
/home/dan/
/home/dan/
make[2]: *** [plugins/
make[1]: *** [plugins/
make[1]: *** Waiting for unfinished jobs....
Unmerged revisions
- 3301. By MC Return on 2012-08-27
-
Added support for unminimize animations and simplified the code by also using the new class AnimEffectUsedFor like it was done for animationaddon.cpp already
- 3300. By MC Return on 2012-08-12
-
Removed redundant newlines
- 3299. By MC Return on 2012-08-11
-
Fixed begginning -> beginning typo in tooltip
- 3298. By MC Return on 2012-08-11
-
Fixed redundant condition checks (x4): If xRot/yRot > 270.0f, the comparison xRot/yRot > 90.0f is always true.
- 3297. By MC Return on 2012-08-11
-
Reduced the scope of the variable 'distance'
- 3296. By MC Return on 2012-08-11
-
Hopefully fixed indentation
- 3295. By MC Return on 2012-08-11
-
Merged lp:compiz
- 3294. By MC Return on 2012-08-11
-
Removed plugins/
simple- animations/ VERSION - 3293. By MC Return on 2012-08-06
-
Merged lp:compiz
- 3292. By MC Return on 2012-07-22
-
Fixed variables 'originX' and 'originY' set but not used [-Wunused-
but-set- variable] compiler warnings in RotateInAnim: :prePaintWindow () and RotateInAnim: :postPaintWindo w () (rotatein.cpp) by removing the 'originX' and 'originY' variables and the unnecessary, unused calculations with those variables


Worked in Compiz 0.9.5+ versions. Currently unfortunately fails to build:
In file included from /home/mcr2010/ src/compiz/ plugins/ simple- animations/ src/bounce. cpp:37: 0: src/compiz/ plugins/ simple- animations/ src/animationsi m.h:114: 7: error: no unique final overrider for ‘virtual bool Animation: :requiresTransf ormedWindow( ) const’ in ‘FlyInAnim’ src/compiz/ plugins/ simple- animations/ src/animationsi m.h:225: 7: error: no unique final overrider for ‘virtual bool Animation: :requiresTransf ormedWindow( ) const’ in ‘BounceAnim’ src/compiz/ plugins/ simple- animations/ src/animationsi m.h:311: 7: error: no unique final overrider for ‘virtual bool Animation: :requiresTransf ormedWindow( ) const’ in ‘PulseSingleAnim’ src/compiz/ plugins/ simple- animations/ src/animationsi m.h:351: 7: error: no unique final overrider for ‘virtual bool Animation: :requiresTransf ormedWindow( ) const’ in ‘FanSingleAnim’ animationsim. dir/src/ bounce. cpp.o] Error 1 animationsim. dir/all] Error 2
/home/mcr2010/
/home/mcr2010/
/home/mcr2010/
/home/mcr2010/
make[2]: *** [CMakeFiles/
make[1]: *** [CMakeFiles/
make: *** [all] Error 2