From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53999) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBrIv-000225-SK for qemu-devel@nongnu.org; Thu, 15 Jan 2015 15:46:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YBrIs-0003oZ-Ik for qemu-devel@nongnu.org; Thu, 15 Jan 2015 15:46:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59127) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBrIs-0003oS-BC for qemu-devel@nongnu.org; Thu, 15 Jan 2015 15:46:06 -0500 Message-ID: <54B8270A.8050007@redhat.com> Date: Thu, 15 Jan 2015 15:46:02 -0500 From: Max Reitz MIME-Version: 1.0 References: <1421066126-25737-1-git-send-email-kraxel@redhat.com> <1421066126-25737-3-git-send-email-kraxel@redhat.com> In-Reply-To: <1421066126-25737-3-git-send-email-kraxel@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/4] console: add opengl rendering helper functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann , qemu-devel@nongnu.org Cc: Anthony Liguori On 2015-01-12 at 07:35, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann > --- > include/ui/console.h | 23 ++++++++++ > ui/Makefile.objs | 5 ++ > ui/console-gl.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+) > create mode 100644 ui/console-gl.c > > diff --git a/include/ui/console.h b/include/ui/console.h > index 22ef8ca..5cb169c 100644 > --- a/include/ui/console.h > +++ b/include/ui/console.h > @@ -9,6 +9,11 @@ > #include "qapi-types.h" > #include "qapi/error.h" > > +#ifdef CONFIG_OPENGL > +#include > +#include > +#endif > + > /* keyboard/mouse support */ > > #define MOUSE_EVENT_LBUTTON 0x01 > @@ -118,6 +123,11 @@ struct DisplaySurface { > pixman_format_code_t format; > pixman_image_t *image; > uint8_t flags; > +#ifdef CONFIG_OPENGL > + GLenum glformat; > + GLenum gltype; > + GLuint texture; > +#endif > }; > > typedef struct QemuUIInfo { > @@ -320,6 +330,19 @@ void qemu_console_copy(QemuConsole *con, int src_x, int src_y, > DisplaySurface *qemu_console_surface(QemuConsole *con); > DisplayState *qemu_console_displaystate(QemuConsole *console); > > +#ifdef CONFIG_OPENGL > +/* console-gl.c */ > +bool console_gl_check_format(DisplayChangeListener *dcl, > + pixman_format_code_t format); > +void surface_gl_create_texture(DisplaySurface *surface); > +void surface_gl_update_texture(DisplaySurface *surface, > + int x, int y, int w, int h); > +void surface_gl_render_texture(DisplaySurface *surface); > +void surface_gl_destroy_texture(DisplaySurface *surface); > +void surface_gl_setup_viewport(DisplaySurface *surface, > + int ww, int wh); > +#endif > + > /* sdl.c */ > void sdl_display_init(DisplayState *ds, int full_screen, int no_frame); > > diff --git a/ui/Makefile.objs b/ui/Makefile.objs > index 13b5cfb..3173778 100644 > --- a/ui/Makefile.objs > +++ b/ui/Makefile.objs > @@ -24,4 +24,9 @@ sdl.mo-objs := sdl2.o sdl2-input.o sdl2-2d.o > endif > sdl.mo-cflags := $(SDL_CFLAGS) > > +ifeq ($(CONFIG_OPENGL),y) > +common-obj-y += console-gl.o > +libs_softmmu += $(OPENGL_LIBS) > +endif > + > gtk.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS) > diff --git a/ui/console-gl.c b/ui/console-gl.c > new file mode 100644 > index 0000000..470dd61 > --- /dev/null > +++ b/ui/console-gl.c > @@ -0,0 +1,127 @@ > +/* > + * QEMU graphical console -- opengl helper bits > + * > + * Copyright (c) 2004 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. > + */ > +#include "qemu-common.h" > +#include "ui/console.h" > + > +bool console_gl_check_format(DisplayChangeListener *dcl, > + pixman_format_code_t format) > +{ > + switch (format) { > + case PIXMAN_x8r8g8b8: > + case PIXMAN_a8r8g8b8: > + case PIXMAN_r5g6b5: > + return true; > + default: > + return false; > + } > +} What is this function supposed to be used for? > + > +void surface_gl_create_texture(DisplaySurface *surface) > +{ > + switch (surface->format) { > + case PIXMAN_x8r8g8b8: > + case PIXMAN_a8r8g8b8: > + surface->glformat = GL_BGRA; Why does the format code seem to imply ARGB order but you're using the exact opposite? I could imagine something like endianness being the reason, but you're using RGB below where the format code says exactly that. > + surface->gltype = GL_UNSIGNED_BYTE; > + break; > + case PIXMAN_r5g6b5: > + surface->glformat = GL_RGB; > + surface->gltype = GL_UNSIGNED_SHORT_5_6_5; > + break; > + default: > + g_assert_not_reached(); > + } > + > + glGenTextures(1, &surface->texture); > + glEnable(GL_TEXTURE_2D); > + glBindTexture(GL_TEXTURE_2D, surface->texture); > + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, Discarding the alpha channel is intentional, I suppose? > + surface_width(surface), > + surface_height(surface), > + 0, surface->glformat, surface->gltype, > + surface_data(surface)); Is surface_stride(surface) specified to be surface_width(surface) * bytes_per_pixel (I don't know pixman so I don't know)? If it isn't then this may not work. > + > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR); > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); > +} > + > +void surface_gl_update_texture(DisplaySurface *surface, > + int x, int y, int w, int h) > +{ > + uint8_t *data = (void *)surface_data(surface); > + > + glTexSubImage2D(GL_TEXTURE_2D, 0, > + 0, y, > + surface_width(surface), h, > + surface->glformat, surface->gltype, > + data + surface_stride(surface) * y); > +} > + > +void surface_gl_render_texture(DisplaySurface *surface) > +{ > + glClearColor(0.0, 0.0, 0.0, 0); I still find this strange (the RGB values are doubles, the alpha channel is an integer; and actually all parameters are expected to be floats), but I can live with it. > + glClear(GL_COLOR_BUFFER_BIT); > + > + glBegin(GL_QUADS); > + glTexCoord2i(0, 1); glVertex3i(-1, -1, 0); > + glTexCoord2i(0, 0); glVertex3i(-1, 1, 0); > + glTexCoord2i(1, 0); glVertex3i(1, 1, 0); > + glTexCoord2i(1, 1); glVertex3i(1, -1, 0); I don't know whether I've said it before, but you can use glVertex2i() here. > + glEnd(); > +} > + > +void surface_gl_destroy_texture(DisplaySurface *surface) > +{ > + if (!surface || !surface->texture) { > + return; > + } > + glDeleteTextures(1, &surface->texture); > + surface->texture = 0; > +} > + > +void surface_gl_setup_viewport(DisplaySurface *surface, > + int ww, int wh) > +{ > + int gw, gh, stripe; > + float sw, sh; > + > + gw = surface_width(surface); > + gh = surface_height(surface); > + > + 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(); > +} Looks fine to me in general. Max