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

Revision history for this message
Sam Spilsbury (smspillaz) 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.

This should probably be fixed in the GLVertexBuffer implementation.

>
> >
> > 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