[MPlayer-dev-eng] patch for quadbuffered stereo using "-vo gl2:stereo"

[Reimar Döffinger]

Hello,
On Wed, Nov 28, 2007 at 03:16:03PM -0600, Stuart Levy wrote:
> On Wed, Nov 28, 2007 at 08:59:23PM +0100, Reimar Döffinger wrote:
>  [...]
> > > +static void drawTextureDisplay ( float tex0, float tex1 )
> > >  {
>  [...]
> > > +  itx0 = (int) (tex0 * texnumx);
> > > +  itx1 = (int) ceilf(tex1 * texnumx);
> > > +
> > 
> > Instead of this "mess", why not just adding a proper transformation
> > matrix? You wouldn't even have to change drawTextureDisplay, just change
> > the matrix one level above. In addition this would be much easier to
> > adjust for e.g. left image above right image and similar things.
> 
> drawTextureDisplay() does need the extra information passed in,
> since in stereo mode, it must draw only the left-half pixels of
> the source image when drawing to GL_BACK_LEFT, and then the
> right half pixels in the second pass, instead of the usual case
> which draws all the pixels.  So just a transform matrix
> wouldn't do.  Right?  And I need to compute which texture tiles
> are covered by the necessary piece of source image, etc.
> I could do the fxoff/fxscale conversions in a matrix -- something like
>   glPushMatrix();
>   glScalef( 1.0f/(tex1-tex0), 1.0f, 1.0f );
>   glTranslatef( -tex0, 0.0f, 0.0f );
>   ...
>     glDrawTex(square->fx, square->fy, square->fw, square->fh, ... );
>   ...
>   glPopMatrix();
> but is that any clearer?

Well,
glPushMatrix();
glScalef(2, 1, 1);
drawTextureDisplay();
glTranslatef(-0.5, 0, 0);
drawTextureDisplay();
glPopMatrix();
(obviously I forgot the parts setting right/left backbuffer)

sure is not any more complex than fiddling with drawTextureDisplay!
Not to mention that it can be sped up more easily via DisplayLists, it
is less of a hindrance to further extending and maintaining (among other
things, due to containing the code to support stereo in one place), is
trivial to port to e.g. vo_gl etc.
Not to mention it doesn't add a math.h dependency...

[...]
> > The old message was wrong anyway, and the one you add might not be right
> > either, there can well be quadbuffer stereo support and it still will fail,
> > e.g. because there is no doublebuffer support.
> > It should just be replaced by something like "no suitable GLX visual found",
> > and that is independent of this patch.
> 
> There's a new variant in the new attached patch -- how do you like this?
> I think it might be nice to mention that quadbuffered stereo is required
> in case of error, since most opengl implementations have all the
> other requirements.

I don't know. I simply loathe having any kind of logic in
message-printing code. I somewhat doubt it is helpful, since it does not
say what exactly was the problem, and I somewhat doubt that
"quadbuffered stereo" will mean anything at all to someone who stumbles
over it by accident.


[...]
> > I guess you give the wrong -monitoraspect. If you want it automated you
> > probably at least have to hack VOCTRL_UPDATE_SCREENINFO
> 
> Could you explain more about what should be updated?  I don't know even
> what the fields mean, let alone which ones I'm free to change at what point.

I don't know a good automatic solution. Just use -monitorpixelaspect 2

[...]
> > >    if (use_glFinish)
> > > -  glFinish();
> > > +    glFinish();
> > 
> > Cosmetic.
> 
> OK.  I hope you agree that code like
> 
>    if (use_glFinish)
>    glFinish();
> 
> (with matching indentation on both lines)
> is uglier than it should be,
> but won't try to change that in this patch.

The point is it is irrelevant to this patch and obfuscates the main
part. And it only remained like this because for now, gl2 is only
a legacy that did not seem worth proper cleanup to me.

> +// QuadBuffer Stereo
> +static unsigned char quadbuffer_stereo = 0;

Comments should be doxygen compatible. Or removed if there are as
utterly useless as this one...

> @@ -732,7 +789,9 @@
>    swapGlBuffers();
>  
>    if (vo_fs) // Avoid flickering borders in fullscreen mode
> +  {
>      glClear (GL_COLOR_BUFFER_BIT);
> +  }
>  }

That part is now useless...

Greetings,
Reimar Döffinger