diff mbox

[RfC,2/3] sdl2: add support for display rendering using opengl.

Message ID 1418295956-22479-3-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Dec. 11, 2014, 11:05 a.m. UTC
Add new sdl2-gl.c file, with display
rendering functions using opengl.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/sdl2.h |  10 ++++
 ui/Makefile.objs  |   4 ++
 ui/sdl2-2d.c      |   6 +++
 ui/sdl2-gl.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ui/sdl2.c         |  50 ++++++++++++++++---
 5 files changed, 206 insertions(+), 7 deletions(-)
 create mode 100644 ui/sdl2-gl.c

Comments

Max Reitz Dec. 11, 2014, 3:57 p.m. UTC | #1
On 2014-12-11 at 12:05, Gerd Hoffmann wrote:
> Add new sdl2-gl.c file, with display
> rendering functions using opengl.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   include/ui/sdl2.h |  10 ++++
>   ui/Makefile.objs  |   4 ++
>   ui/sdl2-2d.c      |   6 +++
>   ui/sdl2-gl.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ui/sdl2.c         |  50 ++++++++++++++++---
>   5 files changed, 206 insertions(+), 7 deletions(-)
>   create mode 100644 ui/sdl2-gl.c
>
> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> new file mode 100644
> index 0000000..30018d4
> --- /dev/null
> +++ b/ui/sdl2-gl.c
> @@ -0,0 +1,143 @@
> +/*
> + * QEMU SDL display driver
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +/* Ported SDL 1.2 code to 2.0 by Dave Airlie. */
> +
> +/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
> +#undef WIN32_LEAN_AND_MEAN
> +
> +#include <SDL.h>
> +#include <SDL_syswm.h>
> +#include <SDL_opengl.h>
> +
> +#include "qemu-common.h"
> +#include "ui/console.h"
> +#include "ui/input.h"
> +#include "ui/sdl2.h"
> +#include "sysemu/sysemu.h"
> +
> +static void sdl2_gl_render_surface(struct sdl2_console *scon)
> +{
> +    int gw, gh, ww, wh, stripe;
> +    float sw, sh;
> +    GLuint tex;
> +
> +    gw = surface_width(scon->surface);
> +    gh = surface_height(scon->surface);
> +    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> +
> +    sw = (float)ww/gw;
> +    sh = (float)wh/gh;
> +    if (sw < sh) {
> +        stripe = wh - wh*sw/sh;
> +        glViewport(0, stripe / 2, ww, wh - stripe);
> +    } else {
> +        stripe = ww - ww*sh/sw;
> +        glViewport(stripe / 2, 0, ww - stripe, wh);
> +    }
> +
> +    glMatrixMode(GL_PROJECTION);
> +    glLoadIdentity();

It's been a surprisingly long time since I last saw the OpenGL builtin 
matrix stack. :-)

> +
> +    glMatrixMode(GL_MODELVIEW);
> +    glLoadIdentity();
> +
> +    glClearColor(0.0, 0.0, 0.0, 0);

The alpha value is a float, too. So I'd either write 0 for everything 
and let the compiler handle the implicit conversion or (better, in my 
opinion) use 0.f (or 0.0f). Which brings me to that I'd rather not use 
doubles when the function takes floats...

> +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

But you don't need glClearColor() at all, and you don't need this 
glClear() either. The depth test is disabled and the quad fills the 
whole screen, thus you can just leave the depth and color buffer as they 
are.

> +
> +    glGenTextures(1, &tex);
> +    glBindTexture(GL_TEXTURE_2D, tex);
> +    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
> +                 0, GL_BGRA_EXT,GL_UNSIGNED_BYTE,

I feared this extensions might not be widespread enough, but EXT_bgra is 
from 1997 so it should be fine. :-)

> +                 surface_data(scon->surface));
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> +
> +    glEnable(GL_TEXTURE_2D);

Shouldn't you call this before doing the first operation on that target? 
(that is, before the glBindTexture())

> +    glBegin(GL_QUADS);
> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
> +    glEnd();

I've been trained to hate direct mode, but it should be fine for just 
this quad.

First, you may consider using glVertex2f().

Second, as hinted above, I don't like giving ints where floats are 
expected. So I'd like this to be "glTexCoord2f(0.f, 1.f)" etc., or 
(maybe even better because it prevents a discussion about whether to use 
0.f or 0 :-)) just glTexCoord2i() and glVertex2i().

> +
> +    SDL_GL_SwapWindow(scon->real_window);
> +
> +    glDisable(GL_TEXTURE_2D);
> +    glDeleteTextures(1, &tex);
> +}

Also, it hurts to always enable textures, generate one, load it, disable 
textures and delete them just to render a single quad with a texture 
loaded from main memory... (not to mention glViewport(), the matrix 
operations and SDL_GL_MakeCurrent())

Would it be possible to just enable GL_TEXTURE_2D once, store the GL ID 
of the texture in the sdl2_console object and either use glTexImage2D() 
or, technically better, glTexSubImage2D() here?

Using glTexSubImage2D() would give us the advantage of being able to 
perform partial updates on the texture; but it seems to fit pretty bad 
into the existing code. To make it fit, I'd call glTexSubImage2D() 
directly in sdl2_gl_update() and just draw the quad here.

Also, I'd move glViewport() into some function which is called on resize 
(not sdl2_gl_redraw()), and the matrix operations into some 
initialization code. We probably cannot do a whole lot about 
SDL_GL_MakeCurrent(), though, which is too bad (depending on what it 
actually does, the SDL Wiki is not very clear about it...).

Max
Gerd Hoffmann Dec. 12, 2014, 11:04 a.m. UTC | #2
Hi,

> > +static void sdl2_gl_render_surface(struct sdl2_console *scon)
> > +{
> > +    int gw, gh, ww, wh, stripe;
> > +    float sw, sh;
> > +    GLuint tex;
> > +
> > +    gw = surface_width(scon->surface);
> > +    gh = surface_height(scon->surface);
> > +    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> > +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> > +
> > +    sw = (float)ww/gw;
> > +    sh = (float)wh/gh;
> > +    if (sw < sh) {
> > +        stripe = wh - wh*sw/sh;
> > +        glViewport(0, stripe / 2, ww, wh - stripe);
> > +    } else {
> > +        stripe = ww - ww*sh/sw;
> > +        glViewport(stripe / 2, 0, ww - stripe, wh);
> > +    }
> > +
> > +    glMatrixMode(GL_PROJECTION);
> > +    glLoadIdentity();
> 
> It's been a surprisingly long time since I last saw the OpenGL builtin 
> matrix stack. :-)

--verbose please.

> > +
> > +    glMatrixMode(GL_MODELVIEW);
> > +    glLoadIdentity();
> > +
> > +    glClearColor(0.0, 0.0, 0.0, 0);
> 
> The alpha value is a float, too. So I'd either write 0 for everything 
> and let the compiler handle the implicit conversion or (better, in my 
> opinion) use 0.f (or 0.0f). Which brings me to that I'd rather not use 
> doubles when the function takes floats...

Ok.

> > +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> 
> But you don't need glClearColor() at all, and you don't need this 
> glClear() either. The depth test is disabled and the quad fills the 
> whole screen, thus you can just leave the depth and color buffer as they 
> are.

The quad fills the whole screen only in case host window and guest
screen have the same aspect ratio, otherwise there is padding top/bottom
or left/right so we don't change the guests screen aspect ratio.

> > +
> > +    glGenTextures(1, &tex);
> > +    glBindTexture(GL_TEXTURE_2D, tex);
> > +    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
> > +                 0, GL_BGRA_EXT,GL_UNSIGNED_BYTE,
> 
> I feared this extensions might not be widespread enough, but EXT_bgra is 
> from 1997 so it should be fine. :-)

Note to self:  This also needs to be extended to handle other surface
formats.

> > +                 surface_data(scon->surface));
> > +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> > +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> > +
> > +    glEnable(GL_TEXTURE_2D);
> 
> Shouldn't you call this before doing the first operation on that target? 
> (that is, before the glBindTexture())

Probably ...

> > +    glBegin(GL_QUADS);
> > +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
> > +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
> > +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
> > +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
> > +    glEnd();
> 
> I've been trained to hate direct mode, but it should be fine for just 
> this quad.

--verbose please.  Guess for longer sequences it would be much more
efficient to compile this into a shader program?

> First, you may consider using glVertex2f().
> 
> Second, as hinted above, I don't like giving ints where floats are 
> expected. So I'd like this to be "glTexCoord2f(0.f, 1.f)" etc., or 
> (maybe even better because it prevents a discussion about whether to use 
> 0.f or 0 :-)) just glTexCoord2i() and glVertex2i().

Using gl*i makes sense indeed.

> > +
> > +    SDL_GL_SwapWindow(scon->real_window);
> > +
> > +    glDisable(GL_TEXTURE_2D);
> > +    glDeleteTextures(1, &tex);
> > +}
> 
> Also, it hurts to always enable textures, generate one, load it, disable 
> textures and delete them just to render a single quad with a texture 
> loaded from main memory... (not to mention glViewport(), the matrix 
> operations and SDL_GL_MakeCurrent())
> 
> Would it be possible to just enable GL_TEXTURE_2D once, store the GL ID 
> of the texture in the sdl2_console object and either use glTexImage2D() 
> or, technically better, glTexSubImage2D() here?

Probably.  I don't want tie this into sdl too much though.  My
longer-term plan is to have some generic gl helper functions in the
console code and have all uis with gl support use these.

> Using glTexSubImage2D() would give us the advantage of being able to 
> perform partial updates on the texture; but it seems to fit pretty bad 
> into the existing code. To make it fit, I'd call glTexSubImage2D() 
> directly in sdl2_gl_update() and just draw the quad here.

Yes, that should work.

cheers,
  Gerd
Max Reitz Dec. 12, 2014, 1:34 p.m. UTC | #3
On 2014-12-12 at 12:04, Gerd Hoffmann wrote:
>    Hi,
>
>>> +static void sdl2_gl_render_surface(struct sdl2_console *scon)
>>> +{
>>> +    int gw, gh, ww, wh, stripe;
>>> +    float sw, sh;
>>> +    GLuint tex;
>>> +
>>> +    gw = surface_width(scon->surface);
>>> +    gh = surface_height(scon->surface);
>>> +    SDL_GetWindowSize(scon->real_window, &ww, &wh);
>>> +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>>> +
>>> +    sw = (float)ww/gw;
>>> +    sh = (float)wh/gh;
>>> +    if (sw < sh) {
>>> +        stripe = wh - wh*sw/sh;
>>> +        glViewport(0, stripe / 2, ww, wh - stripe);
>>> +    } else {
>>> +        stripe = ww - ww*sh/sw;
>>> +        glViewport(stripe / 2, 0, ww - stripe, wh);
>>> +    }
>>> +
>>> +    glMatrixMode(GL_PROJECTION);
>>> +    glLoadIdentity();
>> It's been a surprisingly long time since I last saw the OpenGL builtin
>> matrix stack. :-)
> --verbose please.

Well, I'm mostly used to OpenGL 3/4 Core now where that builtin matrix 
stack does not exist anymore.

>>> +
>>> +    glMatrixMode(GL_MODELVIEW);
>>> +    glLoadIdentity();
>>> +
>>> +    glClearColor(0.0, 0.0, 0.0, 0);
>> The alpha value is a float, too. So I'd either write 0 for everything
>> and let the compiler handle the implicit conversion or (better, in my
>> opinion) use 0.f (or 0.0f). Which brings me to that I'd rather not use
>> doubles when the function takes floats...
> Ok.
>
>>> +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
>> But you don't need glClearColor() at all, and you don't need this
>> glClear() either. The depth test is disabled and the quad fills the
>> whole screen, thus you can just leave the depth and color buffer as they
>> are.
> The quad fills the whole screen only in case host window and guest
> screen have the same aspect ratio, otherwise there is padding top/bottom
> or left/right so we don't change the guests screen aspect ratio.

Right; additionally, I thought glClear() will be limited by the 
viewport. It doesn't, it is indeed necessary, yes. (Well, you could drop 
the GL_DEPTH_BUFFER_BIT, but if you're already clearing the color buffer 
it shouldn't really matter)

>>> +
>>> +    glGenTextures(1, &tex);
>>> +    glBindTexture(GL_TEXTURE_2D, tex);
>>> +    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
>>> +                 0, GL_BGRA_EXT,GL_UNSIGNED_BYTE,
>> I feared this extensions might not be widespread enough, but EXT_bgra is
>> from 1997 so it should be fine. :-)
> Note to self:  This also needs to be extended to handle other surface
> formats.
>
>>> +                 surface_data(scon->surface));
>>> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>>> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>>> +
>>> +    glEnable(GL_TEXTURE_2D);
>> Shouldn't you call this before doing the first operation on that target?
>> (that is, before the glBindTexture())
> Probably ...
>
>>> +    glBegin(GL_QUADS);
>>> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
>>> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
>>> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
>>> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
>>> +    glEnd();
>> I've been trained to hate direct mode, but it should be fine for just
>> this quad.
> --verbose please.  Guess for longer sequences it would be much more
> efficient to compile this into a shader program?

Well, again, I'm used to OpenGL 3/4 Core now which doesn't have the 
immediate mode any more. Now, there are vertex arrays which are just 
arrays of the vertex data which are transferred to the GPU and then stay 
there.

For OpenGL 2 at least, there is something which is called vertex arrays 
as well which works nearly the same way. The main difference is that 
here there are not only vertex arrays, but also texture coordinate 
arrays, normal arrays and color arrays (whereas in 3/4 Core there is 
only opaque vertex data, which OpenGL does not care about whether that's 
a position or a texture coordinate or whatever).

So, I'm not sure whether these exist for OpenGL 1 as well... There are 
display lists, but they were just a dead end. So, immediate mode is dead 
and buried today, but it shouldn't be too bad here and maybe for some 
reason there are people which want to use qemu with OpenGL acceleration 
on a pre OpenGL 2 machine.

>> First, you may consider using glVertex2f().
>>
>> Second, as hinted above, I don't like giving ints where floats are
>> expected. So I'd like this to be "glTexCoord2f(0.f, 1.f)" etc., or
>> (maybe even better because it prevents a discussion about whether to use
>> 0.f or 0 :-)) just glTexCoord2i() and glVertex2i().
> Using gl*i makes sense indeed.
>
>>> +
>>> +    SDL_GL_SwapWindow(scon->real_window);
>>> +
>>> +    glDisable(GL_TEXTURE_2D);
>>> +    glDeleteTextures(1, &tex);
>>> +}
>> Also, it hurts to always enable textures, generate one, load it, disable
>> textures and delete them just to render a single quad with a texture
>> loaded from main memory... (not to mention glViewport(), the matrix
>> operations and SDL_GL_MakeCurrent())
>>
>> Would it be possible to just enable GL_TEXTURE_2D once, store the GL ID
>> of the texture in the sdl2_console object and either use glTexImage2D()
>> or, technically better, glTexSubImage2D() here?
> Probably.  I don't want tie this into sdl too much though.  My
> longer-term plan is to have some generic gl helper functions in the
> console code and have all uis with gl support use these.

Well, then you could create an opaque gl_state object or something which 
must be passed by all UI implementations which want to use OpenGL.

Max

>> Using glTexSubImage2D() would give us the advantage of being able to
>> perform partial updates on the texture; but it seems to fit pretty bad
>> into the existing code. To make it fit, I'd call glTexSubImage2D()
>> directly in sdl2_gl_update() and just draw the quad here.
> Yes, that should work.
>
> cheers,
>    Gerd
Gerd Hoffmann Jan. 12, 2015, 12:46 p.m. UTC | #4
Hi,

> >>> +    glBegin(GL_QUADS);
> >>> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
> >>> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
> >>> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
> >>> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
> >>> +    glEnd();
> >> I've been trained to hate direct mode, but it should be fine for just
> >> this quad.
> > --verbose please.  Guess for longer sequences it would be much more
> > efficient to compile this into a shader program?
> 
> Well, again, I'm used to OpenGL 3/4 Core now which doesn't have the 
> immediate mode any more. [ ... ]

> [ ... ] and maybe for some 
> reason there are people which want to use qemu with OpenGL acceleration 
> on a pre OpenGL 2 machine.

For virtio-gpu we'll need OPENGL 3 anyway, so I don't feel like caring
too much about old opengl versions.  How would the opengl 3/4 version of
the above would look like?

> >> Using glTexSubImage2D() would give us the advantage of being able to
> >> perform partial updates on the texture; but it seems to fit pretty bad
> >> into the existing code. To make it fit, I'd call glTexSubImage2D()
> >> directly in sdl2_gl_update() and just draw the quad here.
> > Yes, that should work.

Done, also factoring this into helper functions so gtk can use this too
some day, new series sent out, please have a look.

thanks,
  Gerd
Gerd Hoffmann Jan. 15, 2015, 11:15 a.m. UTC | #5
Hi,

> How would the opengl 3/4 version of
> the above would look like?

Ok, scratch that one.  Looks like this is a seriously non-trivial update
and my rusty opengl knowledge needs a major update for modern-style
opengl ...

cheers,
  Gerd
Paolo Bonzini Jan. 15, 2015, 12:17 p.m. UTC | #6
On 15/01/2015 12:15, Gerd Hoffmann wrote:
>> How would the opengl 3/4 version of
>> > the above would look like?
> Ok, scratch that one.  Looks like this is a seriously non-trivial update
> and my rusty opengl knowledge needs a major update for modern-style
> opengl ...

Unless we want to support OpenGLES, which doesn't have glBegin/glEnd
(you need to use vertex buffers IIRC, but I last played with OpenGL in
2008...), the old style is still fine.  Even the latest OpenGL additions
in GTK+ use glBegin(GL_QUADS).

Paolo
Peter Maydell Jan. 15, 2015, 12:23 p.m. UTC | #7
On 15 January 2015 at 12:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Unless we want to support OpenGLES, which doesn't have glBegin/glEnd
> (you need to use vertex buffers IIRC, but I last played with OpenGL in
> 2008...), the old style is still fine.  Even the latest OpenGL additions
> in GTK+ use glBegin(GL_QUADS).

OTOH OpenGLES support would be nice, maybe :-)

-- PMM
Gerd Hoffmann Jan. 15, 2015, 2:30 p.m. UTC | #8
On Do, 2015-01-15 at 12:23 +0000, Peter Maydell wrote:
> On 15 January 2015 at 12:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Unless we want to support OpenGLES, which doesn't have glBegin/glEnd
> > (you need to use vertex buffers IIRC, but I last played with OpenGL in
> > 2008...), the old style is still fine.  Even the latest OpenGL additions
> > in GTK+ use glBegin(GL_QUADS).
> 
> OTOH OpenGLES support would be nice, maybe :-)

That's exactly my current plan: go for gles ...

cheers,
  Gerd
Max Reitz Jan. 15, 2015, 4:49 p.m. UTC | #9
On 2015-01-12 at 07:46, Gerd Hoffmann wrote:
>    Hi,
>
>>>>> +    glBegin(GL_QUADS);
>>>>> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
>>>>> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
>>>>> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
>>>>> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
>>>>> +    glEnd();
>>>> I've been trained to hate direct mode, but it should be fine for just
>>>> this quad.
>>> --verbose please.  Guess for longer sequences it would be much more
>>> efficient to compile this into a shader program?
>> Well, again, I'm used to OpenGL 3/4 Core now which doesn't have the
>> immediate mode any more. [ ... ]
>> [ ... ] and maybe for some
>> reason there are people which want to use qemu with OpenGL acceleration
>> on a pre OpenGL 2 machine.
> For virtio-gpu we'll need OPENGL 3 anyway, so I don't feel like caring
> too much about old opengl versions.  How would the opengl 3/4 version of
> the above would look like?

Regarding OpenGL 3/4 Core, you'd need some shaders and a buffer for the 
vertex data.

So, regarding the vertex buffers, you'd have to create a buffer with 
glGenBuffers(), bind it to GL_ARRAY_BUFFER with glBindBuffer() and fill 
it with glBufferData(). Then, for every (vertex) attribute your vertex 
shader has, you call glVertexAttribPointer() (after 
glEnableVertexArray()) to specify the part of the buffer to be used for 
that attribute. You can receive the ID required for 
glVertexAttribPointer() by using glGetAttribLocation() on the linked 
shader program.

It's probably best to just go for the OpenGL 1 (or 3/4 Compatibility) 
version for now and I'll see to a patch to make it 3/4 Core later on. 
Then we can decide what to do and don't have to decide now.

Max

>>>> Using glTexSubImage2D() would give us the advantage of being able to
>>>> perform partial updates on the texture; but it seems to fit pretty bad
>>>> into the existing code. To make it fit, I'd call glTexSubImage2D()
>>>> directly in sdl2_gl_update() and just draw the quad here.
>>> Yes, that should work.
> Done, also factoring this into helper functions so gtk can use this too
> some day, new series sent out, please have a look.
>
> thanks,
>    Gerd
>
>
diff mbox

Patch

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 9e9a92d..ba90a91 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -8,6 +8,9 @@  struct sdl2_console {
     int last_vm_running; /* per console for caption reasons */
     int x, y;
     int hidden;
+    int opengl;
+    int updates;
+    SDL_GLContext winctx;
 };
 
 void sdl2_window_create(struct sdl2_console *scon);
@@ -25,3 +28,10 @@  void sdl2_2d_switch(DisplayChangeListener *dcl,
                     DisplaySurface *new_surface);
 void sdl2_2d_refresh(DisplayChangeListener *dcl);
 void sdl2_2d_redraw(struct sdl2_console *scon);
+
+void sdl2_gl_update(DisplayChangeListener *dcl,
+                    int x, int y, int w, int h);
+void sdl2_gl_switch(DisplayChangeListener *dcl,
+                    DisplaySurface *new_surface);
+void sdl2_gl_refresh(DisplayChangeListener *dcl);
+void sdl2_gl_redraw(struct sdl2_console *scon);
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 13b5cfb..b86bdb1 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -21,6 +21,10 @@  sdl.mo-objs := sdl.o sdl_zoom.o
 endif
 ifeq ($(CONFIG_SDLABI),2.0)
 sdl.mo-objs := sdl2.o sdl2-input.o sdl2-2d.o
+ifeq ($(CONFIG_OPENGL),y)
+sdl.mo-objs += sdl2-gl.o
+libs_softmmu += $(OPENGL_LIBS)
+endif
 endif
 sdl.mo-cflags := $(SDL_CFLAGS)
 
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 9c60075..85f1be4 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -42,6 +42,8 @@  void sdl2_2d_update(DisplayChangeListener *dcl,
     DisplaySurface *surf = qemu_console_surface(dcl->con);
     SDL_Rect rect;
 
+    assert(!scon->opengl);
+
     if (!surf) {
         return;
     }
@@ -67,6 +69,8 @@  void sdl2_2d_switch(DisplayChangeListener *dcl,
     DisplaySurface *old_surface = scon->surface;
     int format = 0;
 
+    assert(!scon->opengl);
+
     scon->surface = new_surface;
 
     if (scon->texture) {
@@ -114,6 +118,8 @@  void sdl2_2d_refresh(DisplayChangeListener *dcl)
 
 void sdl2_2d_redraw(struct sdl2_console *scon)
 {
+    assert(!scon->opengl);
+
     if (!scon->surface) {
         return;
     }
diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
new file mode 100644
index 0000000..30018d4
--- /dev/null
+++ b/ui/sdl2-gl.c
@@ -0,0 +1,143 @@ 
+/*
+ * QEMU SDL display driver
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/* Ported SDL 1.2 code to 2.0 by Dave Airlie. */
+
+/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
+#undef WIN32_LEAN_AND_MEAN
+
+#include <SDL.h>
+#include <SDL_syswm.h>
+#include <SDL_opengl.h>
+
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "ui/input.h"
+#include "ui/sdl2.h"
+#include "sysemu/sysemu.h"
+
+static void sdl2_gl_render_surface(struct sdl2_console *scon)
+{
+    int gw, gh, ww, wh, stripe;
+    float sw, sh;
+    GLuint tex;
+
+    gw = surface_width(scon->surface);
+    gh = surface_height(scon->surface);
+    SDL_GetWindowSize(scon->real_window, &ww, &wh);
+    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+
+    sw = (float)ww/gw;
+    sh = (float)wh/gh;
+    if (sw < sh) {
+        stripe = wh - wh*sw/sh;
+        glViewport(0, stripe / 2, ww, wh - stripe);
+    } else {
+        stripe = ww - ww*sh/sw;
+        glViewport(stripe / 2, 0, ww - stripe, wh);
+    }
+
+    glMatrixMode(GL_PROJECTION);
+    glLoadIdentity();
+
+    glMatrixMode(GL_MODELVIEW);
+    glLoadIdentity();
+
+    glClearColor(0.0, 0.0, 0.0, 0);
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+
+    glGenTextures(1, &tex);
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
+                 0, GL_BGRA_EXT, GL_UNSIGNED_BYTE,
+                 surface_data(scon->surface));
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+
+    glEnable(GL_TEXTURE_2D);
+    glBegin(GL_QUADS);
+    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
+    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
+    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
+    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
+    glEnd();
+
+    SDL_GL_SwapWindow(scon->real_window);
+
+    glDisable(GL_TEXTURE_2D);
+    glDeleteTextures(1, &tex);
+}
+
+void sdl2_gl_update(DisplayChangeListener *dcl,
+                    int x, int y, int w, int h)
+{
+    struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+    assert(scon->opengl);
+    scon->updates++;
+}
+
+void sdl2_gl_switch(DisplayChangeListener *dcl,
+                    DisplaySurface *new_surface)
+{
+    struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+    DisplaySurface *old_surface = scon->surface;
+
+    assert(scon->opengl);
+
+    scon->surface = new_surface;
+
+    if (!new_surface) {
+        sdl2_window_destroy(scon);
+        return;
+    }
+
+    if (!scon->real_window) {
+        sdl2_window_create(scon);
+    } else if (old_surface &&
+               ((surface_width(old_surface)  != surface_width(new_surface)) ||
+                (surface_height(old_surface) != surface_height(new_surface)))) {
+        sdl2_window_resize(scon);
+    }
+}
+
+void sdl2_gl_refresh(DisplayChangeListener *dcl)
+{
+    struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+    assert(scon->opengl);
+    graphic_hw_update(dcl->con);
+    if (scon->updates && scon->surface) {
+        scon->updates = 0;
+        sdl2_gl_render_surface(scon);
+    }
+    sdl2_poll_events(scon);
+}
+
+void sdl2_gl_redraw(struct sdl2_console *scon)
+{
+    assert(scon->opengl);
+    if (scon->surface) {
+        sdl2_gl_render_surface(scon);
+    }
+}
diff --git a/ui/sdl2.c b/ui/sdl2.c
index a1def81..1e14a2a 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -92,6 +92,9 @@  void sdl2_window_create(struct sdl2_console *scon)
                                          surface_height(scon->surface),
                                          flags);
     scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
+    if (scon->opengl) {
+        scon->winctx = SDL_GL_GetCurrentContext();
+    }
     sdl_update_caption(scon);
 }
 
@@ -118,6 +121,17 @@  void sdl2_window_resize(struct sdl2_console *scon)
                       surface_height(scon->surface));
 }
 
+static void sdl2_redraw(struct sdl2_console *scon)
+{
+    if (scon->opengl) {
+#ifdef CONFIG_OPENGL
+        sdl2_gl_redraw(scon);
+#endif
+    } else {
+        sdl2_2d_redraw(scon);
+    }
+}
+
 static void sdl_update_caption(struct sdl2_console *scon)
 {
     char win_title[1024];
@@ -316,7 +330,7 @@  static void toggle_full_screen(struct sdl2_console *scon)
         }
         SDL_SetWindowFullscreen(scon->real_window, 0);
     }
-    sdl2_2d_redraw(scon);
+    sdl2_redraw(scon);
 }
 
 static void handle_keydown(SDL_Event *ev)
@@ -364,8 +378,10 @@  static void handle_keydown(SDL_Event *ev)
         case SDL_SCANCODE_U:
             sdl2_window_destroy(scon);
             sdl2_window_create(scon);
-            /* re-create texture */
-            sdl2_2d_switch(&scon->dcl, scon->surface);
+            if (!scon->opengl) {
+                /* re-create scon->texture */
+                sdl2_2d_switch(&scon->dcl, scon->surface);
+            }
             gui_keysym = 1;
             break;
 #if 0
@@ -384,7 +400,7 @@  static void handle_keydown(SDL_Event *ev)
                 fprintf(stderr, "%s: scale to %dx%d\n",
                         __func__, width, height);
                 sdl_scale(scon, width, height);
-                sdl2_2d_redraw(scon);
+                sdl2_redraw(scon);
                 gui_keysym = 1;
             }
 #endif
@@ -518,10 +534,10 @@  static void handle_windowevent(struct sdl2_console *scon, SDL_Event *ev)
             info.height = ev->window.data2;
             dpy_set_ui_info(scon->dcl.con, &info);
         }
-        sdl2_2d_redraw(scon);
+        sdl2_redraw(scon);
         break;
     case SDL_WINDOWEVENT_EXPOSED:
-        sdl2_2d_redraw(scon);
+        sdl2_redraw(scon);
         break;
     case SDL_WINDOWEVENT_FOCUS_GAINED:
     case SDL_WINDOWEVENT_ENTER:
@@ -664,8 +680,22 @@  static const DisplayChangeListenerOps dcl_2d_ops = {
     .dpy_cursor_define = sdl_mouse_define,
 };
 
+#ifdef CONFIG_OPENGL
+static const DisplayChangeListenerOps dcl_gl_ops = {
+    .dpy_name          = "sdl2-gl",
+    .dpy_gfx_update    = sdl2_gl_update,
+    .dpy_gfx_switch    = sdl2_gl_switch,
+    .dpy_refresh       = sdl2_gl_refresh,
+    .dpy_mouse_set     = sdl_mouse_warp,
+    .dpy_cursor_define = sdl_mouse_define,
+};
+#endif
+
 void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
 {
+#ifdef CONFIG_OPENGL
+    int opengl = 1;
+#endif
     int flags;
     uint8_t data = 0;
     char *filename;
@@ -709,10 +739,16 @@  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
         if (!qemu_console_is_graphic(con)) {
             sdl2_console[i].hidden = true;
         }
+        sdl2_console[i].idx = i;
+#ifdef CONFIG_OPENGL
+        sdl2_console[i].opengl = opengl;
+        sdl2_console[i].dcl.ops = opengl ? &dcl_gl_ops : &dcl_2d_ops;
+#else
+        sdl2_console[i].opengl = 0;
         sdl2_console[i].dcl.ops = &dcl_2d_ops;
+#endif
         sdl2_console[i].dcl.con = con;
         register_displaychangelistener(&sdl2_console[i].dcl);
-        sdl2_console[i].idx = i;
     }
 
     /* Load a 32x32x4 image. White pixels are transparent. */