Code review comment for lp://staging/~renatofilho/unity/unity-lp876017

> > I think you can just overload scaleSelectWindow to determine what the
> > currently hovered window is. From there you will need to damage and re-
> render
> > both the new and old window. Have a look at the scaleaddon plugin.
> > #ifdef USE_MODERN_COMPIZ_GL
> > gWindow->vertexBuffer ()->begin ();
> > #else
> > 151 + gWindow->geometry ().reset ();
> > #endif
> > 152 + if (width && height)
> > 153 + gWindow->glAddGeometry (ml, iconReg, iconReg);
> > 154 +
> > #ifdef USE_MODERN_COMPIZ_GL
> > gWindow->vertexBuffer ().end ();
> > if (gWindow->vertexBuffer ().countVertices ())
> > #else
> > 155 + if (gWindow->geometry ().vCount)
> > #endif
> > 156 + {
> > #ifdef USE_MODERN_COMPIZ_GL
> > 157 + GLFragment::Attrib fragment (attrib);
> > #endif
> > 158 + GLMatrix wTransform (transform);
> > 159 +
> > 160 + wTransform.translate (x, y, 0.0f);
> > 161 +
> > #ifdef USE_MODERN_COMPIZ_GL
> > gWindow->glDrawTexture (icon, wTransform, attrib, mask);
> > #else
> > 162 + glPushMatrix ();
> > 163 + glLoadMatrixf (wTransform.getMatrix ());
> > 164 + gWindow->glDrawTexture (icon, fragment, mask);
> > 165 + glPopMatrix ();
> > #endif
> > 166 + }
> Fixed on rev: 2577
> I have implemented the "scaleSelectWindow" based on scaleaddon plugin, but I
> did not understand why I need this code. (The implementation is working well
> without this)

Hmm, what do you mean by this? If you're saying that you don't need either I guess you can remove them both? Its just that overriding scaleSelectWindow is the preferred way to finding out the currently highlighted window as opposed to reimplementing checkForWindowAt.

>
> >
> > 172 + // BG
> > 173 + glColor3f (0.0f, 0.0f, 0.0f);
> > 174 + glRectf (x, y2, x2, y);
> >
> > Preferably use client side buffers for this
> Fixed on rev: 2574
> Thanks for the example code.
>
> > Make this a constant
> Fixed on rev: 2575
>
> >
> > Also avoid the 65535, use OPAQUE
> Fixed on rev: 2576

Thanks.

>
> >
> > 121 + CompString name (PKGDATADIR"/close_dash.png");
> >
> > Is that the correct asset?
> Based on designer docs, yes this is the correct icon.

+1

>
> >
> > That needs to be under test.
> Any suggestion or example how to test it?

My suggestion to make the code testable is split unityshell.(h/cpp) file into different files (UnityWindow/UnityWindowPrivate, UntiyScree/UnityScreenPrivate and UnityShell), with this we can instantiate only the private class inside of the tests and call/test the "private" functions.

What do you think?

^ This suggestion is the right way to do it :)