|
| From: | Max Reitz |
| Subject: | Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl. |
| Date: | Fri, 12 Dec 2014 14:34:47 +0100 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
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
| [Prev in Thread] | Current Thread | [Next in Thread] |