From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34086) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDEpR-0005Ls-7C for qemu-devel@nongnu.org; Mon, 19 Jan 2015 11:05:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDEpN-0007Ys-0p for qemu-devel@nongnu.org; Mon, 19 Jan 2015 11:05:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44114) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDEpM-0007Yi-N8 for qemu-devel@nongnu.org; Mon, 19 Jan 2015 11:05:20 -0500 Message-ID: <54BD2B3A.1090508@redhat.com> Date: Mon, 19 Jan 2015 11:05:14 -0500 From: Max Reitz MIME-Version: 1.0 References: <1421674603-31575-1-git-send-email-kraxel@redhat.com> <1421674603-31575-5-git-send-email-kraxel@redhat.com> In-Reply-To: <1421674603-31575-5-git-send-email-kraxel@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 4/7] console-gl: 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-19 at 08:36, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann > --- > configure | 2 +- > include/ui/console.h | 31 ++++++ > ui/Makefile.objs | 5 + > ui/console-gl.c | 286 ++++++++++++++++++++++++++++++++++++++++++= +++++++++ > 4 files changed, 323 insertions(+), 1 deletion(-) > create mode 100644 ui/console-gl.c > > diff --git a/configure b/configure > index 6d673c1..e6b29a6 100755 > --- a/configure > +++ b/configure > @@ -3069,7 +3069,7 @@ libs_softmmu=3D"$libs_softmmu $fdt_libs" > ########################################## > # opengl probe (for sdl2, milkymist-tmu2) > if test "$opengl" !=3D "no" ; then > - opengl_pkgs=3D"gl" > + opengl_pkgs=3D"gl glesv2" I don't know anything about GLES except for it being (as I've been told)=20 mostly similar to the normal OpenGL. I hope that'll suffice=E2=80=A6 > if $pkg_config $opengl_pkgs x11; then > opengl_cflags=3D"$($pkg_config --libs $opengl_pkgs) $x11_cflags" > opengl_libs=3D"$($pkg_config --libs $opengl_pkgs) $x11_libs" > diff --git a/include/ui/console.h b/include/ui/console.h > index 22ef8ca..9157b64 100644 > --- a/include/ui/console.h > +++ b/include/ui/console.h > @@ -9,6 +9,11 @@ > #include "qapi-types.h" > #include "qapi/error.h" > =20 > +#ifdef CONFIG_OPENGL > +# include > +# include > +#endif > + > /* keyboard/mouse support */ > =20 > #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 > }; > =20 > typedef struct QemuUIInfo { > @@ -320,6 +330,27 @@ void qemu_console_copy(QemuConsole *con, int src_x= , int src_y, > DisplaySurface *qemu_console_surface(QemuConsole *con); > DisplayState *qemu_console_displaystate(QemuConsole *console); > =20 > +/* console-gl.c */ > +typedef struct ConsoleGLState ConsoleGLState; > +#ifdef CONFIG_OPENGL > +ConsoleGLState *console_gl_init_context(void); > +void console_gl_fini_context(ConsoleGLState *gls); > +bool console_gl_check_format(DisplayChangeListener *dcl, > + pixman_format_code_t format); > +void surface_gl_create_texture(ConsoleGLState *gls, > + DisplaySurface *surface); > +void surface_gl_update_texture(ConsoleGLState *gls, > + DisplaySurface *surface, > + int x, int y, int w, int h); > +void surface_gl_render_texture(ConsoleGLState *gls, > + DisplaySurface *surface); > +void surface_gl_destroy_texture(ConsoleGLState *gls, > + DisplaySurface *surface); > +void surface_gl_setup_viewport(ConsoleGLState *gls, > + DisplaySurface *surface, > + int ww, int wh); > +#endif > + > /* sdl.c */ > void sdl_display_init(DisplayState *ds, int full_screen, int no_frame= ); > =20 > 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 :=3D sdl2.o sdl2-input.o sdl2-2d.o > endif > sdl.mo-cflags :=3D $(SDL_CFLAGS) > =20 > +ifeq ($(CONFIG_OPENGL),y) > +common-obj-y +=3D console-gl.o > +libs_softmmu +=3D $(OPENGL_LIBS) > +endif > + > gtk.o-cflags :=3D $(GTK_CFLAGS) $(VTE_CFLAGS) > diff --git a/ui/console-gl.c b/ui/console-gl.c > new file mode 100644 > index 0000000..589c682 > --- /dev/null > +++ b/ui/console-gl.c > @@ -0,0 +1,286 @@ > +/* > + * QEMU graphical console -- opengl helper bits > + * > + * Copyright (c) 2014 Red Hat > + * > + * Authors: > + * Gerd Hoffmann > + * > + * Permission is hereby granted, free of charge, to any person obtaini= ng a copy > + * of this software and associated documentation files (the "Software"= ), to deal > + * in the Software without restriction, including without limitation t= he rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/o= r 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 incl= uded in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXP= RESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABI= LITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT S= HALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES O= R OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARI= SING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALI= NGS IN > + * THE SOFTWARE. > + */ > +#include "qemu-common.h" > +#include "ui/console.h" > + > +struct ConsoleGLState { > + GLint texture_blit_prog; > +}; > + > +/* -------------------------------------------------------------------= --- */ > + > +static GLchar texture_blit_vert_src[] =3D > + "\n" > + "#version 300 es\n" > + "\n" > + "in vec2 in_position;\n" > + "in vec2 in_tex_coord;\n" You could calculate the texture coordinate from the position in the=20 shader, but this is mostly my premature optimization instinct kicking in=20 instead of a real performance difference considering how few vertices=20 there are in this case. > + "out vec2 ex_tex_coord;\n" > + "\n" > + "void main(void) {\n" > + " gl_Position =3D vec4(in_position.x, in_position.y, 0.0, 1.0);= \n" vec4(in_position, 0.0, 1.0) *cough* Okay, perhaps it's OCD and not a premature optimization instinct. > + " ex_tex_coord =3D in_tex_coord;\n" > + "}\n" > + "\n"; > + > +static GLchar texture_blit_frag_src[] =3D > + "\n" > + "#version 300 es\n" > + "\n" > + "uniform sampler2D image;\n" > + "in highp vec2 ex_tex_coord;\n" > + "out highp vec4 out_frag_color;\n" > + "\n" > + "void main(void) {\n" > + " out_frag_color =3D texture(image, ex_tex_coord);\n" > + "}\n" > + "\n"; > + > +static void gl_run_texture_blit(ConsoleGLState *gls) > +{ > + GLfloat in_position[] =3D { > + -1, 1, > + -1, -1, > + 1, -1, > + -1, 1, > + 1, 1, > + 1, -1, This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.=20 For GL_TRIANGLE_STRIP, you first define one whole triangle (by=20 specifying each of the three vertices) and after that, each vertex=20 results in a new triangle drawn (whose two other vertices are the two=20 vertices preceding the last one). Therefore, for a quad, you only need four vertices in GL_TRIANGLE_STRIP=20 mode. You will find that the following works just fine: GLfloat in_position[] =3D { -1, -1, 1, -1, -1, 1, 1, 1, }; (1) > + }; > + GLfloat in_tex_coord[] =3D { > + 0, 0, > + 0, 1, > + 1, 1, > + 0, 0, > + 1, 0, > + 1, 1, > + }; (1) and GLfloat in_tex_coord[] =3D { 0, 1, 1, 1, 0, 0, 1, 0, }; here. > + GLint l_position; > + GLint l_tex_coord; > + > + glUseProgram(gls->texture_blit_prog); > + l_position =3D glGetAttribLocation(gls->texture_blit_prog, "in_pos= ition"); > + l_tex_coord =3D glGetAttribLocation(gls->texture_blit_prog, "in_te= x_coord"); > + glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_pos= ition); > + glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_te= x_coord); I think it is disallowed to refer to non-OpenGL buffers here in the core=20 profile. The 4.4 core specification states (section 10.3, vertex=20 arrays): "Vertex data are placed into arrays that are stored in the=20 server=E2=80=99s address space"; the 4.4 compatibility specification stat= es:=20 "Vertex data may also be placed into arrays that are stored in the=20 client=E2=80=99s address space (described here) or in the server=E2=80=99= s address space". ("client" refers to the main memory, "server" refers to the video=20 memory, as far as I know) As I said before, though, I am completely fine with going for the=20 compatibility profile for now and making it core compliant later on. > + glEnableVertexAttribArray(l_position); > + glEnableVertexAttribArray(l_tex_coord); > + glDrawArrays(GL_TRIANGLE_STRIP, l_position, 6); (1) and of course s/6/4/ here. > +} > + > +/* -------------------------------------------------------------------= --- */ > + > +static GLuint gl_create_compile_shader(GLenum type, const GLchar *src) > +{ > + GLuint shader; > + GLint status, length; > + char *errmsg; > + > + shader =3D glCreateShader(type); > + glShaderSource(shader, 1, &src, 0); > + glCompileShader(shader); > + > + glGetShaderiv(shader, GL_COMPILE_STATUS, &status); > + if (!status) { > + glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &length); > + errmsg =3D malloc(length); > + glGetShaderInfoLog(shader, length, &length, errmsg); > + fprintf(stderr, "%s: compile %s error\n%s\n", __func__, > + (type =3D=3D GL_VERTEX_SHADER) ? "vertex" : "fragment"= , > + errmsg); > + free(errmsg); > + return 0; > + } > + return shader; > +} > + > +static GLuint gl_create_link_program(GLuint vert, GLuint frag) > +{ > + GLuint program; > + GLint status, length; > + char *errmsg; > + > + program =3D glCreateProgram(); > + glAttachShader(program, vert); > + glAttachShader(program, frag); > + glLinkProgram(program); > + > + glGetProgramiv(program, GL_LINK_STATUS, &status); > + if (!status) { > + glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length); > + errmsg =3D malloc(length); > + glGetProgramInfoLog(program, length, &length, errmsg); > + fprintf(stderr, "%s: link program: %s\n", __func__, errmsg); > + free(errmsg); > + return 0; > + } > + return program; > +} > + > +static GLuint gl_create_compile_link_program(const GLchar *vert_src, > + const GLchar *frag_src) > +{ > + GLuint vert_shader, frag_shader; > + > + vert_shader =3D gl_create_compile_shader(GL_VERTEX_SHADER, vert_sr= c); > + frag_shader =3D gl_create_compile_shader(GL_FRAGMENT_SHADER, frag_= src); > + if (!vert_shader || !frag_shader) { > + return 0; > + } > + > + return gl_create_link_program(vert_shader, frag_shader); Minor thing: You are free to delete the shaders after they have been=20 linked into the program (because the references will be lost when this=20 function returns). > +} > + > +/* -------------------------------------------------------------------= --- */ > + > +ConsoleGLState *console_gl_init_context(void) > +{ > + ConsoleGLState *gls =3D g_new0(ConsoleGLState, 1); > + > + gls->texture_blit_prog =3D gl_create_compile_link_program > + (texture_blit_vert_src, texture_blit_frag_src); > + if (!gls->texture_blit_prog) { > + exit(1); > + } > + > + return gls; > +} > + > +void console_gl_fini_context(ConsoleGLState *gls) > +{ > + if (!gls) { > + return; > + } > + g_free(gls); > +} > + > +bool console_gl_check_format(DisplayChangeListener *dcl, > + pixman_format_code_t format) > +{ > + switch (format) { > + case PIXMAN_BE_b8g8r8x8: > + case PIXMAN_BE_b8g8r8a8: > + case PIXMAN_r5g6b5: > + return true; > + default: > + return false; > + } > +} > + > +void surface_gl_create_texture(ConsoleGLState *gls, > + DisplaySurface *surface) > +{ > + assert(gls); > + > + switch (surface->format) { > + case PIXMAN_BE_b8g8r8x8: > + case PIXMAN_BE_b8g8r8a8: > + surface->glformat =3D GL_BGRA_EXT; If you want to avoid the _EXT, you could use GL_RGBA here and=20 texture().bgra in the fragment shader. However, this would require a=20 different shader for the 32 bit and the 16 bit formats (because the 16=20 bit format has the right order, apparently). I'm voting for keeping GL_BGRA_EXT until someone complains. > + surface->gltype =3D GL_UNSIGNED_BYTE; > + break; > + case PIXMAN_r5g6b5: > + surface->glformat =3D GL_RGB; > + surface->gltype =3D GL_UNSIGNED_SHORT_5_6_5; I haven't been able to really test 16 bit mode (forcing 16 bit mode in=20 hw/display/vga.c doesn't count...), so I'm trusting you this works. > + break; > + default: > + g_assert_not_reached(); > + } > + > + glGenTextures(1, &surface->texture); > + glEnable(GL_TEXTURE_2D); > + glBindTexture(GL_TEXTURE_2D, surface->texture); > + glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, > + surface_stride(surface) / surface_bytes_per_pixel(su= rface)); > + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, > + surface_width(surface), > + surface_height(surface), > + 0, surface->glformat, surface->gltype, > + surface_data(surface)); > + > + 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(ConsoleGLState *gls, > + DisplaySurface *surface, > + int x, int y, int w, int h) > +{ > + uint8_t *data =3D (void *)surface_data(surface); > + > + assert(gls); > + > + glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, > + surface_stride(surface) / surface_bytes_per_pixel(su= rface)); Maybe we should assert that surface_stride(surface) %=20 surface_bytes_per_pixel(surface) =3D=3D 0 here. > + glTexSubImage2D(GL_TEXTURE_2D, 0, > + x, y, w, h, > + surface->glformat, surface->gltype, > + data + surface_stride(surface) * y > + + surface_bytes_per_pixel(surface) * x); > +} > + > +void surface_gl_render_texture(ConsoleGLState *gls, > + DisplaySurface *surface) > +{ > + assert(gls); > + > + glClearColor(0.1f, 0.1f, 0.1f, 0.0f); > + glClear(GL_COLOR_BUFFER_BIT); > + > + gl_run_texture_blit(gls); > +} > + > +void surface_gl_destroy_texture(ConsoleGLState *gls, > + DisplaySurface *surface) > +{ > + if (!surface || !surface->texture) { > + return; > + } > + glDeleteTextures(1, &surface->texture); > + surface->texture =3D 0; > +} > + > +void surface_gl_setup_viewport(ConsoleGLState *gls, > + DisplaySurface *surface, > + int ww, int wh) > +{ > + int gw, gh, stripe; > + float sw, sh; > + > + assert(gls); > + > + gw =3D surface_width(surface); > + gh =3D surface_height(surface); > + > + sw =3D (float)ww/gw; > + sh =3D (float)wh/gh; > + if (sw < sh) { > + stripe =3D wh - wh*sw/sh; > + glViewport(0, stripe / 2, ww, wh - stripe); > + } else { > + stripe =3D ww - ww*sh/sw; > + glViewport(stripe / 2, 0, ww - stripe, wh); > + } > +} With the vertex data changed to make use of GL_TRIANGLE_STRIP and with=20 or without the stride assertion and with or without deleting the shaders=20 after the program has been linked: Reviewed-by: Max Reitz