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

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Right approach, some pointers:

76 +// Based on Scale plugin code
77 +CompWindow* UnityScreen::checkForWindowAt (int x, int y)
78 +{
79 + int x1, y1, x2, y2;
80 + CompWindowList::reverse_iterator rit = screen->windows ().rbegin ();
81 +
82 + for (; rit != screen->windows ().rend (); ++rit)
83 + {
84 + CompWindow *w = *rit;
85 + SCALE_WINDOW (w);
86 +
87 + ScalePosition pos = sw->getCurrentPosition ();
88 + if (sw->hasSlot ())
89 + {
90 + x1 = w->x () - w->input ().left * pos.scale;
91 + y1 = w->y () - w->input ().top * pos.scale;
92 + x2 = w->x () + (w->width () + w->input ().right) * pos.scale;
93 + y2 = w->y () + (w->height () + w->input ().bottom) * pos.scale;
94 + x1 += pos.x ();
95 + y1 += pos.y ();
96 + x2 += pos.x ();
97 + y2 += pos.y ();
98 + if (x1 <= x && y1 <= y && x2 > x && y2 > y)
99 + return w;
100 + }
101 + }
102 + return NULL;
103 +}

Is this necessary?

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.

151 + gWindow->geometry ().reset ();
152 + if (width && height)
153 + gWindow->glAddGeometry (ml, iconReg, iconReg);
154 +
155 + if (gWindow->geometry ().vCount)
156 + {
157 + GLFragment::Attrib fragment (attrib);
158 + GLMatrix wTransform (transform);
159 +
160 + wTransform.translate (x, y, 0.0f);
161 +
162 + glPushMatrix ();
163 + glLoadMatrixf (wTransform.getMatrix ());
164 + gWindow->glDrawTexture (icon, fragment, mask);
165 + glPopMatrix ();
166 + }

You will need to put this inside of #if USE_MODERN_COMPIZ_GL ifdefs. Sorry :( Here's how to do it:

#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 + }

172 + // BG
173 + glColor3f (0.0f, 0.0f, 0.0f);
174 + glRectf (x, y2, x2, y);

Preferably use client side buffers for this, eg

#ifndef USE_MODERN_COMPIZ_GL
172 + // BG
173 + glColor3f (0.0f, 0.0f, 0.0f);
174 + glRectf (x, y2, x2, y);
#else
GLVertexBuffer *vertexBuffer = GLVertexBuffer::streamingBuffer ();
vertexBuffer->begin (GL_TRIANGLE_STRIP)
const GLfloat vertices[] =
{
    x, y2, 0.0f,
    x, y, 0.0f,
    x2, y, 0.0f,
    x2, y2, 0.0f
};

vertexBuffer->addVertices (4, vertices);
vertexBuffer->color4f (0.0f, 0.0f, 0.0f, 1.0f);
vertexBuffer->end ();
vertexBuffer->render (transform, attrib);

209 + x + CLOSE_ICON_SPACE, iconY,
210 + maxWidth , maxHeight);

Make this a constant

190 + if (!sWindow->hasSlot() || // animation finished
191 + attrib.opacity != 65535) // no focus
192 + return;
193 +
194 + ScalePosition pos = sWindow->getCurrentPosition ();
195 + int maxHeight, maxWidth;
196 + int width = window->width () * pos.scale;
197 + float x = pos.x () + window->x ();
198 + float y = pos.y () + window->y ();
199 + float iconY = y + ((SCALE_WINDOW_TITLE_SIZE - CLOSE_ICON_SIZE) / 2.0);
200 +
201 + maxHeight = maxWidth = 0;

That needs to be under test.

Also avoid the 65535, use OPAQUE

121 + CompString name (PKGDATADIR"/close_dash.png");

Is that the correct asset?

review: Needs Fixing

« Back to merge proposal