Sam, first I want to say thanks for finding the time for reviewing this. +1

> 848     +    float sx = (float) screen->width ()  / output->width ();
> 849     +    float sy = (float) screen->height () / output->height ();
> 
> The denominator should be casted and not the numerator.
> 
AFAIR I kept what was there already, see:

825	-    float     sx = (float) screen->width () / output->width ();
826	-    float     sy = (float) screen->height () / output->height ();

...but I can change that -> no problem.

> 948     -       gScreen->setTextureFilter (GL_LINEAR_MIPMAP_LINEAR);
> 949     +    {
> 950     +       /* check the actual filtering */
> 951     +       oldFilter = gScreen->textureFilter ();
> 952     +
> 953     +       /* just change the global filter if necessary */
> 954     +       if (oldFilter != GL_LINEAR_MIPMAP_LINEAR)
> 955     +       {
> 956     +           gScreen->setTextureFilter (GL_LINEAR_MIPMAP_LINEAR);
> 957     +           filterChanged = true;
> 958     +       }
> 959     +    }
> 
> This change has no effect - setTextureFilter only changes the *next* texture
> filter any textures will be configured with the next time that they are bound
> to a texture unit, so checking if the filter is already
> GL_LINEAR_MIPMAP_LINEAR will not save any calls into OpenGL, as
> setTextureFilter doesn't make any.
>
Ah -> good to know, thanks for the info. I did not dive that deep into that
part of the code yet...
So setTextureFilter won't make any changes if the filter we want to set is 
already set ?
 
> 1393    -       zoomAnim = eScreen->optionGetExpoAnimation () ==
> 1394    -                  ExpoScreen::ExpoAnimationZoom;
> 1395    -       hide     = eScreen->optionGetHideDocks () &&
> 1396    -                  (window->wmType () & CompWindowTypeDockMask);
> 1397    -
> 1398            if (eScreen->expoCam > 0.0)
> 1399                mask |= PAINT_WINDOW_TRANSLUCENT_MASK;
> 1400
> 1401    +       float opacity  = 1.0;
> 1402    +       bool  zoomAnim = eScreen->optionGetExpoAnimation () ==
> ExpoScreen::ExpoAnimationZoom;
> 
> 1412    -               opacity = attrib.opacity *
> 1413    -                         (1 - sigmoidProgress (eScreen->expoCam));
> 1414    +               opacity = attrib.opacity * (1 - sigmoidProgress
> (eScreen->expoCam));
> 
> 1861    +    int   winRealWidth      = w->width ()  + 2 * w->geometry
> ().border () + w->border ().left + w->border ().right;
> 1862    +    int   winRealHeight     = w->height () + 2 * w->geometry
> ().border () + w->border ().top  + w->border ().bottom;
> 1863    +
> 
> The preference for compiz is to where possible keep line lengths under 80
> characters.
>
Yes, I know that of course ;)
Normally I use the rule to just extend over 80 characters, if it massively
improves readability.
Maybe I messed up here -> I'll investigate.

> It is a somewhat artificially small limit, but it assists those who are
> working on the code in text-based editors.
> 
> 1407    -       if (hide)
> 1408    +       if (eScreen->optionGetHideDocks () &&
> 1409    +           (window->wmType () & CompWindowTypeDockMask))
> 
> Prefer to keep temporary booleans where they are there for the purpose of
> aiding readability. The compiler gets rid of them at the optimization phase.
>
I'll bring back hide.
 
> 1834    +    /* TODO: Make glowSize configurable via CCSM */
> 1835    +    int glowSize   = 48;
> 
> There is no good reason to make this configurable, especially where the
> texture itself is hardcoded and scales poorly.
>
Well, you are right about the hardcoded texture, but the size of 48 gets downscaled
to about 3 or 4 pixels in Expo, so a configuration value of 24-96 should be acceptable
in this case...

> 
> 1861    +    int   winRealWidth      = w->width ()  + 2 * w->geometry
> ().border ()
> 
> compiz::window::Geometry has a helper function widthIncBorders () which does
> exactly this with less error prone typing.
>
I will use that function in this case. Thanks 4 the tip.
 
> The other changes seem fine so far. Keep in mind that that size of this review
> means that I might have missed things and that I might need to go through it
> again later.

Sure, I won't globally approve this until it gets your green light. :)
Thanks for the review and sorry about the length, it accumulated over time ;)