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