Merge lp:~brandontschaefer/compiz/window-max-semi-animation into lp:compiz/0.9.9

[Brandon Schaefer]

Adds an animation to the grid plugin. The animation takes the current window,
renders it to the growing grid box and slowly fades out into the default
orange glow box.

Also adds 2 options ccsm for the grid, one to turn this animation off, and
another to change the grid animation duration.

This also changes the default grid duration from 250 ms, to 350ms.

(LP: #689792)

[MC Return (mc-return)]

LGTM. :)
Compiles, works and is a nice addition.
+1 for making it configurable.
Another +1 for adding an option to adjust the length of the preview.

One note though:
Maybe
"Draw window resize preview animation."
would be better than
"Draw window preview where the window will be resized"

and
"Resize preview animation duration (in ms)."
better than
"Draw Resize Duration in Milliseconds"

I noticed other tooltips could also be formatted nicer,
so this are only cosmetic suggestions of course...

review: Approve

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3566. By Brandon Schaefer on 2013-01-14

* Change the tooltip strings

[Sam Spilsbury (smspillaz)]

The code is fine, asides from some whitespace changes that don't need to be there:

39 -
40 +
41 switch (gw->resizeCount)
42 {
43 case 1:
44 @@ -384,11 +385,11 @@
45 gw->isGridMaximized = false;
46 }
47
48 - int dw = (lastBorder.left + lastBorder.right) -
49 + int dw = (lastBorder.left + lastBorder.right) -
50 (gw->window->border ().left +
51 gw->window->border ().right);
52 -
53 - int dh = (lastBorder.top + lastBorder.bottom) -
54 +
55 + int dh = (lastBorder.top + lastBorder.bottom) -
56 (gw->window->border ().top +
57 gw->window->border ().bottom);

This also never returns NULL:

71 + GLWindow *glWindow = GLWindow::get(cw);
72 +
73 + if (!glWindow)
74 + return;

And better is to use borderRect () here:

95 + int width = cw->width() + cw->border().left + cw->border().right;
96 + int height = cw->height() + cw->border().top + cw->border().bottom;

Also a space between the bracket and function needs to be added here (and above):

animations.at (current).duration = optionGetDrawDuration();

The functionality also works correctly.

However, I note my general opposition to implementing unity-like features inside of compiz. Compiz is an upstream project and its functionality needs to be kept separate. This feature could be implemented fine within unity, and satisfies the requisite that we don't depend on any particular window management paradigm unique to compiz.

As such, my vote is "abstain".

review: Abstain

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3567. By Brandon Schaefer on 2013-01-15

* Fixes reviews comment

3568. By Brandon Schaefer on 2013-01-15

* Missed space between ()

[Daniel van Vugt (vanvugt)]

I think this is in the right place. It's a feature that users of upstream compiz might like. And it makes the most sense to live in the grid plugin. Proper review coming... another day probably. Sorry.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3566
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/14/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/14/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/14//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3566
http://jenkins.qa.ubuntu.com/job/compiz-ci/362/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/362/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/362//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3566
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/13/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/13/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/13//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3568
http://jenkins.qa.ubuntu.com/job/compiz-ci/363/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/363/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/363//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3568
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/15/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/15//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3568
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/14/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/14/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/14//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3568
http://jenkins.qa.ubuntu.com/job/compiz-ci/364/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/364/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/364//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3568
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/15/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/15//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3568
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/16/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/16//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Looks really nice, but...

1. It should be off be default... Maybe? We don't usually like to change default behaviour. And enable it for Ubuntu in debian/patches/ubuntu-config.patch
Maybe...

2. #include "math.h" should be #include <cmath>

3. The option names are not very descriptive. "Window Preview" is too vague. I don't think it needs more words but better words. Something mentioning "stretch" or "expand".

4. The preview should be painted from a custom version of GLWindow::glPaint(), never glPaintOutput.

5. wAttrib needs to be initialized to the attrib from glPaint. So it looks right with windows that are starting translucent etc. Same for the mask and transform parameters. Always initialize from the parameters of glPaint.

6. I don't think it makes sense to limit the duration between 250 and 1000ms. There should be no limit other than zero, unless you have a good computational reason.

7. This is redundant. Conversion to float will be automatic and does not need casting:
float scaleX = (float)...

8. Maybe the opacity equation should be configurable. Maybe not...?
powf(25,...

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

9. Please wrap code at column 78 if possible.

[Daniel van Vugt (vanvugt)]

Hmm, maybe we do have to call from GridScreen::glPaintOutput. That's a problem. Because if a window is transformed, translucent, darkened etc, you need to get all that info from glPaint (or glDraw).

[Sam Spilsbury (smspillaz)]

On Wed, Jan 16, 2013 at 9:42 AM, Daniel van Vugt
<email address hidden> wrote:
> Hmm, maybe we do have to call from GridScreen::glPaintOutput. That's a problem. Because if a window is transformed, translucent, darkened etc, you need to get all that info from glPaint (or glDraw).

You can use gWindow->lastPaintAttrib () for that.

> --
> https://code.launchpad.net/~brandontschaefer/compiz/window-max-semi-animation/+merge/143163
> You are reviewing the proposed merge of lp:~brandontschaefer/compiz/window-max-semi-animation into lp:compiz.

--
Sam Spilsbury

[Brandon Schaefer (brandontschaefer)]

1, 2, 3, 6, 7, 8

All those make sense to me, and Ill fix those now.

4.

Im not sure how/what you mean by making a custom version of GLWindow::glPaint() unless you mean I should make a subclass of a GLWindow ... but I need to look if glPaint is virtual function... from there im not sure where it would fit into grid.cpp, as glPaintRectangle is being called in glPaintOutput (which is why I put it there)

5.

Im assuming that is from the params of glPaint/Draw, so that should be easy if we figure out 4.

9.

Do you mean wrap any code that is longer then 78 chars?

[Brandon Schaefer (brandontschaefer)]

Cant I also get the paint attributes directly from the glWindow?

opengl.h GLWindowPaintAttrib & paintAttrib ();

or

        GLWindowPaintAttrib & lastPaintAttrib ();

Though im not srue where I would get the translate data from.

[Sam Spilsbury (smspillaz)]

On Wed, Jan 16, 2013 at 10:06 AM, Brandon Schaefer
<email address hidden> wrote:
> Cant I also get the paint attributes directly from the glWindow?
>
> opengl.h GLWindowPaintAttrib & paintAttrib ();
>
> or
>
> GLWindowPaintAttrib & lastPaintAttrib ();
>
>
> Though im not srue where I would get the translate data from.

You won't need the translation or scale data. Calling glPaint or
glDraw effectively just draws the the window vertex mesh with the
window texture while providing a set of opacity/brightness/saturation
parameters and affine transformation matrix to the fragment and vertex
shader respectively.

> --
> https://code.launchpad.net/~brandontschaefer/compiz/window-max-semi-animation/+merge/143163
> You are reviewing the proposed merge of lp:~brandontschaefer/compiz/window-max-semi-animation into lp:compiz.

--
Sam Spilsbury

[Daniel van Vugt (vanvugt)]

Yeah you need to inherit from GLWindow, implement glPaint and call w->gWindow->glPaintSetEnabled(w, true)
That's what I was suggesting right from the beginning.

It wasn't important for grid before because the old grid rectangle reused none of the attributes of the window in question. However now you need to. Just try loading obs, changing the opacity/brightness/saturation of a window and try it. The preview will look wrong. You need to keep the window attributes (in the least) from glPaint().

Yeah please wrap all code around column 78 because it's not only common industry practice to use 80 column windows, but also improves readability. Think how many paper books you've seen that have lines much longer than 80 columns. Or a newspaper. Keeping that limit aids readability.

[Daniel van Vugt (vanvugt)]

And no you can't use lastPaintAttrib because that's the attribute from before plugins started being called back. You have to use the parameter version of attrib, mask and transform to ensure changes from other plugins still get applied.

[Sam Spilsbury (smspillaz)]

On Wed, Jan 16, 2013 at 10:24 AM, Daniel van Vugt
<email address hidden> wrote:
> And no you can't use lastPaintAttrib because that's the attribute from before plugins started being called back. You have to use the parameter version of attrib, mask and transform to ensure changes from other plugins still get applied.

*shrug* it should be good enough. It will lag one frame behind, sure.

Another alternative is to use paintAttrib () after the call to
gScreen->glPaintOutput () in GridScreen::glPaintOutput. paintAttrib ()
will be up-to-date by that point, as glPaintOutput calls into
paintOutputRegion, which then draws all of the windows.

> --
> https://code.launchpad.net/~brandontschaefer/compiz/window-max-semi-animation/+merge/143163
> You are reviewing the proposed merge of lp:~brandontschaefer/compiz/window-max-semi-animation into lp:compiz.

--
Sam Spilsbury

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3569. By Brandon Schaefer on 2013-01-16

* Fixes 1, 2, 3, 6, 9 from Daniels review
* Number 7 needs to be kept this way because we need to do float divison, not int.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3569
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/18/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/18//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3569
http://jenkins.qa.ubuntu.com/job/compiz-ci/366/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/366/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/366//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3569
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/17/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/17/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/17//rebuild/?

review: Approve (continuous-integration)

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3570. By Brandon Schaefer on 2013-01-16

* Move the glPaintStreachedWindow to GridWindow, and made a custom glPaint instead

3571. By Brandon Schaefer on 2013-01-16

* Opps, missed 2 extra spaces

[Brandon Schaefer (brandontschaefer)]

Alright, everything is fixed expect number:

8. Maybe the opacity equation should be configurable. Maybe not...?
powf(25,...

Soo here... hmm should we consider a few different options or make it completely customizable?

So for options, we could have just Accelerating (x^4), Decelerating (which is what I have now, 25^-x, it could be better...) ... Linear (op * (1.0 - x)) etc. ?

[Daniel van Vugt (vanvugt)]

Yeah don't worry about customization of the curve right now. It's a common issue that all plugins employing some kind of sigmoid curve do not make it configurable.

[Daniel van Vugt (vanvugt)]

From within the GridWindow class, you should change:
    CompWindow *cw = screen->findWindow (screen->activeWindow ());
to:
    CompWindow *cw = this->window;

review: Needs Fixing

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3572. By Brandon Schaefer on 2013-01-16

* Remove grabbing the active window the comp window because the grid window already has one

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3571
http://jenkins.qa.ubuntu.com/job/compiz-ci/367/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/367/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/367//rebuild/?

review: Approve (continuous-integration)

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3573. By Brandon Schaefer on 2013-01-16

* Change to drawing the stretched window ontop of the window.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3571
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/18/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/18//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Also, you should scale the glow/outline opacity by the inverse of the stretch opacity (1.0f - curve). So it's always balanced and adds up to 1.0. Visually, this means the glow does not overpower the start of the stretch animation, so it's more visible. I just prototyped this and it looks better. It's actually what I was suggesting from the beginning.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3571
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/19/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/19//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Also if you cancel the maximize, the glow keeps growing but the stretch suddenly vanishes. They should behave the same.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Back to my first comment about never changing the default appearance. That's just a guideline. In cases like this where we can all agree the change makes things more beautiful then I think just enable it by default.

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3574. By Brandon Schaefer on 2013-01-16

* Apply the inverse of the curve function to the glow box
* Remove the !fadeOut so the stretched window is still drawn with the glow box

3575. By Brandon Schaefer on 2013-01-16

* Enable animation by default

[Brandon Schaefer (brandontschaefer)]

Added the scaling to the glow box, fixed the sudden vanishing of the window, and re-enabled the animation by default :)

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3576. By Brandon Schaefer on 2013-01-16

* Remove magic numbers, and change the curve number from 25 to 35
  as it creates a smother fade.

[Brandon Schaefer (brandontschaefer)]

I also changed the 25 to 35, as its a smother fade when the animation is done.

[Brandon Schaefer (brandontschaefer)]

Also note this change:

109 - duration = 250;
110 + duration = 0;

Setting that to 0, because we are setting this when the animation is made.

Also note I've changed the default time from 250 -> 350. If this should be revert let me know! (It allows more time to see the animation :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3573
http://jenkins.qa.ubuntu.com/job/compiz-ci/368/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/368/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/368//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3573
http://jenkins.qa.ubuntu.com/job/compiz-ci/369/
Executed test runs:

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/369//rebuild/?

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3573
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/19/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/19//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3573
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/20/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/20//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Hmm... this is wrong, but not your fault :)
   colorData[0..2] = alpha * color[0..2];
It should be:
   colorData[0..2] = color[0..2];
Because alpha should only apply to alpha ([3]).

Just worth remembering. Not to be fixed here.

[Brandon Schaefer (brandontschaefer)]

I can make another branch after this one lands to fix this issue.

[Daniel van Vugt (vanvugt)]

That's weird. With slow animations enabled I can see two different textures being stretched. Near the end of the animation a second texture appears briefly.

review: Needs Fixing

[Brandon Schaefer (brandontschaefer)]

Hmm im not seeing this. The only thing I see that should be fixed is when it is fading out and the stretched window doesn't smoothly fade away....other then that I don't see a second texture appear anymore (after rev 3574 of this branch)

[Daniel van Vugt (vanvugt)]

All seems OK now. I will review again tomorrow.

If anything just needs "static" on CURVE_ANIMATION and indentation fixing in grid.h.

review: Needs Fixing

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3577. By Brandon Schaefer on 2013-01-17

* (1.0 - curve) isn't doing anything....the glow box already starts off very
  slow.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3576
http://jenkins.qa.ubuntu.com/job/compiz-ci/370/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/370/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/370//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3576
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/20/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/20//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3576
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/21/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/21//rebuild/?

review: Approve (continuous-integration)

[Sam Spilsbury (smspillaz)]

On Jan 17, 2013 9:12 AM, "Daniel van Vugt" <email address hidden>
wrote:
>
> Hmm... this is wrong, but not your fault :)
> colorData[0..2] = alpha * color[0..2];
> It should be:
> colorData[0..2] = color[0..2];
> Because alpha should only apply to alpha ([3]).

Nope, the alpha must be premultiplied.

>
> Just worth remembering. Not to be fixed here.
> --
>
https://code.launchpad.net/~brandontschaefer/compiz/window-max-semi-animation/+merge/143163
> You are reviewing the proposed merge of
lp:~brandontschaefer/compiz/window-max-semi-animation into lp:compiz.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3577
http://jenkins.qa.ubuntu.com/job/compiz-ci/371/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/371/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/371//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3577
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/22/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/22//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3577
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/21/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/21//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3577
http://jenkins.qa.ubuntu.com/job/compiz-ci/372/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/372/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/372//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3577
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/22/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/22//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3577
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/23/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/23/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/23//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Actually, I now realize the function of multiplying RGB by alpha. That is to darken the colour. That's a very bad an inconsistent way to do it. RGB should have just been multiplied by 0.5 (or something), regardless of alpha.

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3578. By Brandon Schaefer on 2013-01-17

* Re-add the (1.0 - curve) to the alpha

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3578
http://jenkins.qa.ubuntu.com/job/compiz-ci/376/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/376/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/376//rebuild/?

review: Approve (continuous-integration)

[Sam Spilsbury (smspillaz)]

On Fri, Jan 18, 2013 at 3:05 AM, Daniel van Vugt
<email address hidden> wrote:
> Actually, I now realize the function of multiplying RGB by alpha. That is to darken the colour. That's a very bad an inconsistent way to do it. RGB should have just been multiplied by 0.5 (or something), regardless of alpha.

Default blending function, screen.cpp:840: glBlendFunc (GL_ONE,
GL_ONE_MINUS_SRC_ALPHA);

https://www.opengl.org/sdk/docs/man/xhtml/glBlendFunc.xml

-> GL_ONE: scale factors are 1, 1, 1, 1

Eg, not premultiplied for us.

Porter-duff blending operations: https://en.wikipedia.org/wiki/Alpha_compositing

-> "If an alpha channel is used in an image, it is common to also
multiply the color by the alpha value, to save on additional
multiplications during compositing. This is usually referred to as
premultiplied alpha."
-> "if this pixel uses premultiplied alpha, all of the RGB values (0,
1, 0) are multiplied by 0.5 and then the alpha is appended to the end
to yield (0, 0.5, 0, 0.5). In this case, the 0.5 value for the G
channel actually indicates 100% green intensity (with 50% opacity)."

Many other places in the source code also do alpha premultiplication,
because that's what opengl expects.

resize.cpp:

 fc[3] = fillColor[3];
 fc[0] = fillColor[0] * (unsigned long)fc[3] / 65535;
 fc[1] = fillColor[1] * (unsigned long)fc[3] / 65535;
 fc[2] = fillColor[2] * (unsigned long)fc[3] / 65535;

cube.cpp:1122

    colorData.push_back (color[0] * opacity / 0xffff);
    colorData.push_back (color[1] * opacity / 0xffff);
    colorData.push_back (color[2] * opacity / 0xffff);
    colorData.push_back (opacity);

resizeinfo, text, etc:

 colorData[0] = opacity * 65535;
 colorData[1] = opacity * 65535;
 colorData[2] = opacity * 65535;
 colorData[3] = opacity * 65535;

If you don't want to use premultiplied alpha, you ask opengl to
multiply the alpha value with the color value:

scaleaddon.cpp:

    glBlendFunc (GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);

Do you really want to do a state switch though?

If you don't believe me then remove the pre-multiplication and enjoy
the eye-burning.

> --
> https://code.launchpad.net/~brandontschaefer/compiz/window-max-semi-animation/+merge/143163
> You are reviewing the proposed merge of lp:~brandontschaefer/compiz/window-max-semi-animation into lp:compiz.

--
Sam Spilsbury

[Daniel van Vugt (vanvugt)]

It's off topic so sorry I mentioned it. Clearly there are many ways to modify a colour. But the simplest way to darken is to multiple RGB by N where N < 1.0.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3578
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/27/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/27/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/27//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3578
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/26/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/26/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/26//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

OK, needs fixing:

Sometimes multiple textures are visible in the stretch. Seems the issue is the wrong window can have its glPaintSetEnabled sometimes. Brandon is working on moving glPaintSetEnabled to safer locations.

review: Needs Fixing

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3579. By Brandon Schaefer on 2013-01-18

* Fix double texture drawing (filter the animations based on the window id).
* Moved from glPaint to glDraw
* Move glDrawSetEnabled(ture/false) to ::preparePaint and ::donePaint, so if it
  hits a donePaint, and never hits the prepartPaint it will stop drawing.

[Brandon Schaefer (brandontschaefer)]

Alright, fixed that issue by adding the window id to each animation so we can filter which window belongs to which animation.

review: Approve

[Brandon Schaefer (brandontschaefer)]

opps...didn't mean to approve...

[Daniel van Vugt (vanvugt)]

Now seeing a new bug: The stretching animation is missing the title bar and decorations... !?

review: Needs Fixing

[Brandon Schaefer (brandontschaefer)]

That is strange...sorry, I keep having you go through this and you keep finding new things...

[Brandon Schaefer (brandontschaefer)]

Hmm when I switch back to glPaint everything works fine...

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3579
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/28/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/./build=pbuilder,distribution=quantal,flavor=amd64/28/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-clang-ci/28//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3579
http://jenkins.qa.ubuntu.com/job/compiz-ci/377/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/377/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/377//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3579
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/27/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=quantal,flavor=amd64/27/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/27//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Hmm, yeah go back to glPaint if it keeps decor happier.

[lp:~brandontschaefer/compiz/window-max-semi-animation]

3580. By Brandon Schaefer on 2013-01-18

* Switch back to glPaint, for the decor

[Brandon Schaefer (brandontschaefer)]

Decor is now happy.

[Daniel van Vugt (vanvugt)]

Nice. Approved.

review: Approve

=== modified file 'plugins/grid/grid.xml.in'
--- plugins/grid/grid.xml.in	2012-10-15 10:31:51 +0000
+++ plugins/grid/grid.xml.in	2013-01-18 18:12:20 +0000
@@ -539,6 +539,17 @@
 			<_long>Draw Window Resize Indicator</_long>
 			<default>true</default>
 		</option>
+		<option name="draw_stretched_window" type="bool">
+			<_short>Draw Stretched Window</_short>
+			<_long>Draw stretched window animation</_long>
+			<default>true</default>
+		</option>
+		<option name="animation_duration" type="int">
+			<_short>Animation Duration</_short>
+			<_long>Grid animation duration (in ms)</_long>
+			<default>350</default>
+			<min>0</min>
+		</option>
 		<option name="outline_color" type="color">
 			<_short>Outline Color</_short>
 			<_long>Color of the resize indicator outline</_long>
 
=== modified file 'plugins/grid/src/grid.cpp'
--- plugins/grid/src/grid.cpp	2013-01-03 15:43:56 +0000
+++ plugins/grid/src/grid.cpp	2013-01-18 18:12:20 +0000
@@ -23,6 +23,7 @@
  */
 
 #include <boost/bind.hpp>
+#include <cmath>
 #include "grid.h"
 #include "grabhandler.h"
 
@@ -31,6 +32,8 @@
 
 static std::map <unsigned int, GridProps> gridProps;
 
+static int const CURVE_ANIMATION = 35;
+
 void
 GridScreen::handleCompizEvent(const char*    plugin,
 			      const char*    event,
@@ -453,6 +456,8 @@
 	for (iter = animations.begin (); iter != animations.end () && animating; ++iter)
 	{
 		Animation& anim = *iter;
+
+	float curve = powf (CURVE_ANIMATION, -anim.progress);
 		float alpha = ((float) optionGetFillColorAlpha () / 65535.0f) * anim.opacity;
 	color = optionGetFillColor ();
 
@@ -461,6 +466,9 @@
 	colorData[2] = alpha * color[2];
 	colorData[3] = alpha * 65535.0f;
 
+	if (optionGetDrawStretchedWindow ())
+	    colorData[3] *= (1.0 - curve);
+
 	vertexData[0]  = anim.currentRect.x1 ();
 	vertexData[1]  = anim.currentRect.y1 ();
 	vertexData[2]  = 0.0f;
@@ -496,6 +504,9 @@
 	colorData[2] = alpha * color[2];
 	colorData[3] = alpha * 65535.0f;
 
+	if (optionGetDrawStretchedWindow ())
+	    colorData[3] *= (1.0 - curve);
+
 	vertexData[0]  = anim.currentRect.x1 ();
 	vertexData[1]  = anim.currentRect.y1 ();
 	vertexData[3]  = anim.currentRect.x1 ();
@@ -782,8 +793,10 @@
 				    int current = animations.size () - 1;
 				    animations.at (current).fromRect	= cw->serverBorderRect ();
 				    animations.at (current).currentRect	= cw->serverBorderRect ();
+				    animations.at (current).duration = optionGetAnimationDuration ();
 				    animations.at (current).timer = animations.at (current).duration;
 				    animations.at (current).targetRect = desiredSlot;
+				    animations.at (current).window = cw->id();
 
 				    if (lastEdge == NoEdge || !animating)
 				    {
@@ -1023,6 +1036,14 @@
 		anim.progress =	(anim.duration - anim.timer) / anim.duration;
 	}
 
+	if (optionGetDrawStretchedWindow ())
+	{
+		CompWindow *cw = screen->findWindow (screen->activeWindow ());
+		GRID_WINDOW (cw);
+
+		gw->gWindow->glPaintSetEnabled (gw, true);
+	}
+
     cScreen->preparePaint (msSinceLastPaint);
 }
 
@@ -1050,6 +1071,14 @@
 		animating = false;
 	}
 
+	if (optionGetDrawStretchedWindow ())
+	{
+		CompWindow *cw = screen->findWindow (screen->activeWindow ());
+		GRID_WINDOW (cw);
+
+		gw->gWindow->glPaintSetEnabled (gw, false);
+	}
+
 	cScreen->damageScreen ();
 
     cScreen->donePaint ();
@@ -1063,9 +1092,10 @@
 	currentRect = CompRect (0, 0, 0, 0);
 	opacity = 0.0f;
 	timer = 0.0f;
-	duration = 250;
+	duration = 0;
 	complete = false;
 	fadingOut = false;
+	window = 0;
 }
 
 
@@ -1131,6 +1161,7 @@
 GridWindow::GridWindow (CompWindow *window) :
     PluginClassHandler <GridWindow, CompWindow> (window),
     window (window),
+    gWindow (GLWindow::get(window)),
     gScreen (GridScreen::get (screen)),
     isGridResized (false),
     isGridMaximized (false),
@@ -1138,9 +1169,11 @@
     pointerBufDx (0),
     pointerBufDy (0),
     resizeCount (0),	
-    lastTarget (GridUnknown)
+    lastTarget (GridUnknown),
+    sizeHintsFlags (0)
 {
     WindowInterface::setHandler (window);
+    GLWindowInterface::setHandler (gWindow, false);
 }
 
 GridWindow::~GridWindow ()
@@ -1153,6 +1186,57 @@
 	gScreen->o[0].value ().set (0);
 }
 
+bool
+GridWindow::glPaint (const GLWindowPaintAttrib& attrib, const GLMatrix& matrix,
+    	    	     const CompRegion& region, const unsigned int mask)
+{
+    bool status = gWindow->glPaint (attrib, matrix, region, mask);
+
+    std::vector<Animation>::iterator iter;
+
+    for (iter = gScreen->animations.begin ();
+    	 iter != gScreen->animations.end () && gScreen->animating; ++iter)
+    {
+    	Animation& anim = *iter;
+
+    	if (anim.timer > 0.0f && anim.window == window->id())
+    	{
+    	    GLWindowPaintAttrib wAttrib(attrib);
+    	    GLMatrix wTransform (matrix);
+    	    unsigned int wMask(mask);
+
+    	    float curve = powf (CURVE_ANIMATION, -anim.progress);
+    	    wAttrib.opacity *= curve;
+
+    	    wMask |= PAINT_WINDOW_TRANSFORMED_MASK;
+    	    wMask |= PAINT_WINDOW_TRANSLUCENT_MASK;
+    	    wMask |= PAINT_WINDOW_BLEND_MASK;
+
+    	    float scaleX = (anim.currentRect.x2 () - anim.currentRect.x1 ()) /
+    	    	    	   (float) window->borderRect ().width ();
+
+    	    float scaleY = (anim.currentRect.y2 () - anim.currentRect.y1 ()) /
+    	    	    	   (float) window->borderRect ().height ();
+
+    	    float translateX = (anim.currentRect.x1 () - window->x ()) +
+    	    	     	    	window->border ().left * scaleX;
+
+    	    float translateY = (anim.currentRect.y1 () - window->y ()) +
+    	    	    	    	window->border ().top * scaleY;
+
+    	    wTransform.translate (window->x (), window->y (), 0.0f);
+    	    wTransform.scale (scaleX, scaleY, 1.0f);
+    	    wTransform.translate (translateX / scaleX - window->x (),
+    	    	    	    	  translateY / scaleY - window->y (), 0.0f);
+
+
+    	    gWindow->glPaint (wAttrib, wTransform, region, wMask);
+    	}
+    }
+
+    return status;
+}
+
 /* Initial plugin init function called. Checks to see if we are ABI
  * compatible with core, otherwise unload */
 
 
=== modified file 'plugins/grid/src/grid.h'
--- plugins/grid/src/grid.h	2012-11-05 00:54:07 +0000
+++ plugins/grid/src/grid.h	2013-01-18 18:12:20 +0000
@@ -99,6 +99,7 @@
 	CompRect currentRect;
 	GLfloat opacity;
 	GLfloat timer;
+	Window window;
 	int duration;
 	bool complete;
 	bool fadingOut;
@@ -169,6 +170,7 @@
 
 class GridWindow :
     public WindowInterface,
+    public GLWindowInterface,
     public PluginClassHandler <GridWindow, CompWindow>
 {
     public:
@@ -176,6 +178,7 @@
 	GridWindow (CompWindow *);
 	~GridWindow ();
 	CompWindow *window;
+    	GLWindow *gWindow;
 	GridScreen *gScreen;
 
 	bool isGridResized;
@@ -189,6 +192,9 @@
 	GridType lastTarget;
 	unsigned int sizeHintsFlags;
 
+	bool glPaint (const GLWindowPaintAttrib&, const GLMatrix&,
+		      const CompRegion&, unsigned int);
+
 	void grabNotify (int, int, unsigned int, unsigned int);
 
 	void ungrabNotify ();