Code review comment for lp:~bitzesmichail/compiz/splash-gles

Revision history for this message
Michail Bitzes (bitzesmichail) wrote :

> This all looks great, all according to how it should work. My only two general
> comments:
>
> 262 + stream->colorDefault ();
>
> That is probably not necessary (the new shader based rendering model isn't
> stateful, I believe the active color is reset on GLVertexBuffer::begin ()

Weird. In my code, I used color4f () before begin () and it works. Is this wrong?

As an experiment, I changed color4f(1.0,1.0,1.0,alpha) to color4f(1.0,0.0,0.0,alpha). Without colorDefault (), the red color is leaked outside the splash logo, and when the animation finishes, the screen is left a red mess.

>
> 2. Generally speaking, its a lot easier to port old code that uses GL_QUADS to
> use GL_TRIANGLE_STRIP instead of GL_TRIANGLES as the primitive type, since the
> vertices are often arranged to wind in right-angled triangles (eg, x1y1, x1y2,
> x2y1, x2y2), and then you can just generate the first triangle and only take
> the fourth-point for every other triangle after that.

Thanks, I'll consider this approach next time.

« Back to merge proposal