* [Qemu-devel] [PATCH v2 1/7] configure: opengl overhaul
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 14:43 ` Max Reitz
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 2/7] Allow the use of X11 from a non standard location Gerd Hoffmann
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Michael Walle, Gerd Hoffmann, Anthony Liguori, Max Reitz
Rename config option from "glx" to "opengl", glx will not be the only
option for opengl in near future. Also switch over to pkg-config for
opengl support detection.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
configure | 39 +++++++++++++++++----------------------
default-configs/lm32-softmmu.mak | 2 +-
hw/display/Makefile.objs | 2 +-
hw/lm32/milkymist-hw.h | 4 ++--
include/sysemu/sysemu.h | 1 +
vl.c | 1 +
6 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/configure b/configure
index 7539645..8216410 100755
--- a/configure
+++ b/configure
@@ -309,7 +309,7 @@ rbd=""
smartcard_nss=""
libusb=""
usb_redir=""
-glx=""
+opengl=""
zlib="yes"
lzo=""
snappy=""
@@ -1026,9 +1026,9 @@ for opt do
;;
--enable-vhost-scsi) vhost_scsi="yes"
;;
- --disable-glx) glx="no"
+ --disable-opengl) opengl="no"
;;
- --enable-glx) glx="yes"
+ --enable-opengl) opengl="yes"
;;
--disable-rbd) rbd="no"
;;
@@ -3056,23 +3056,18 @@ fi
libs_softmmu="$libs_softmmu $fdt_libs"
##########################################
-# GLX probe, used by milkymist-tmu2
-if test "$glx" != "no" ; then
- glx_libs="-lGL -lX11"
- cat > $TMPC << EOF
-#include <X11/Xlib.h>
-#include <GL/gl.h>
-#include <GL/glx.h>
-int main(void) { glBegin(0); glXQueryVersion(0,0,0); return 0; }
-EOF
- if compile_prog "" "-lGL -lX11" ; then
- glx=yes
+# opengl probe (for sdl2, milkymist-tmu2)
+if test "$opengl" != "no" ; then
+ opengl_pkgs="gl"
+ if $pkg_config $opengl_pkgs; then
+ opengl_libs="$($pkg_config --libs $opengl_pkgs) -lX11"
+ opengl=yes
else
- if test "$glx" = "yes" ; then
- feature_not_found "glx" "Install GL devel (e.g. MESA)"
+ if test "$opengl" = "yes" ; then
+ feature_not_found "opengl" "Install GL devel (e.g. MESA)"
fi
- glx_libs=
- glx=no
+ opengl_libs=""
+ opengl=no
fi
fi
@@ -4320,7 +4315,7 @@ echo "xfsctl support $xfs"
echo "nss used $smartcard_nss"
echo "libusb $libusb"
echo "usb net redir $usb_redir"
-echo "GLX support $glx"
+echo "OpenGL support $opengl"
echo "libiscsi support $libiscsi"
echo "libnfs support $libnfs"
echo "build guest agent $guest_agent"
@@ -4682,9 +4677,9 @@ if test "$usb_redir" = "yes" ; then
echo "CONFIG_USB_REDIR=y" >> $config_host_mak
fi
-if test "$glx" = "yes" ; then
- echo "CONFIG_GLX=y" >> $config_host_mak
- echo "GLX_LIBS=$glx_libs" >> $config_host_mak
+if test "$opengl" = "yes" ; then
+ echo "CONFIG_OPENGL=y" >> $config_host_mak
+ echo "OPENGL_LIBS=$opengl_libs" >> $config_host_mak
fi
if test "$lzo" = "yes" ; then
diff --git a/default-configs/lm32-softmmu.mak b/default-configs/lm32-softmmu.mak
index 7df58c8..4889348 100644
--- a/default-configs/lm32-softmmu.mak
+++ b/default-configs/lm32-softmmu.mak
@@ -2,7 +2,7 @@
CONFIG_LM32=y
CONFIG_MILKYMIST=y
-CONFIG_MILKYMIST_TMU2=$(CONFIG_GLX)
+CONFIG_MILKYMIST_TMU2=$(CONFIG_OPENGL)
CONFIG_FRAMEBUFFER=y
CONFIG_PTIMER=y
CONFIG_PFLASH_CFI01=y
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 7ed76a9..e18ea57 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -20,7 +20,7 @@ common-obj-$(CONFIG_ZAURUS) += tc6393xb.o
ifeq ($(CONFIG_MILKYMIST_TMU2),y)
common-obj-y += milkymist-tmu2.o
-libs_softmmu += $(GLX_LIBS)
+libs_softmmu += $(OPENGL_LIBS)
endif
obj-$(CONFIG_OMAP) += omap_dss.o
diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h
index 5317ce6..8d20cac 100644
--- a/hw/lm32/milkymist-hw.h
+++ b/hw/lm32/milkymist-hw.h
@@ -86,7 +86,7 @@ static inline DeviceState *milkymist_pfpu_create(hwaddr base,
return dev;
}
-#ifdef CONFIG_GLX
+#ifdef CONFIG_OPENGL
#include <X11/Xlib.h>
#include <GL/glx.h>
static const int glx_fbconfig_attr[] = {
@@ -100,7 +100,7 @@ static const int glx_fbconfig_attr[] = {
static inline DeviceState *milkymist_tmu2_create(hwaddr base,
qemu_irq irq)
{
-#ifdef CONFIG_GLX
+#ifdef CONFIG_OPENGL
DeviceState *dev;
Display *d;
GLXFBConfig *configs;
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 748d059..310078c 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -117,6 +117,7 @@ extern int graphic_width;
extern int graphic_height;
extern int graphic_depth;
extern DisplayType display_type;
+extern int display_opengl;
extern const char *keyboard_layout;
extern int win2k_install_hack;
extern int alt_grab;
diff --git a/vl.c b/vl.c
index fbf4240..0bf2acb 100644
--- a/vl.c
+++ b/vl.c
@@ -129,6 +129,7 @@ static int data_dir_idx;
const char *bios_name = NULL;
enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
DisplayType display_type = DT_DEFAULT;
+int display_opengl;
static int display_remote;
const char* keyboard_layout = NULL;
ram_addr_t ram_size;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/7] configure: opengl overhaul
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 1/7] configure: opengl overhaul Gerd Hoffmann
@ 2015-01-19 14:43 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2015-01-19 14:43 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Paolo Bonzini, Michael Walle, Anthony Liguori
On 2015-01-19 at 08:36, Gerd Hoffmann wrote:
> Rename config option from "glx" to "opengl", glx will not be the only
> option for opengl in near future. Also switch over to pkg-config for
> opengl support detection.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> configure | 39 +++++++++++++++++----------------------
> default-configs/lm32-softmmu.mak | 2 +-
> hw/display/Makefile.objs | 2 +-
> hw/lm32/milkymist-hw.h | 4 ++--
> include/sysemu/sysemu.h | 1 +
> vl.c | 1 +
> 6 files changed, 23 insertions(+), 26 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 2/7] Allow the use of X11 from a non standard location.
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 1/7] configure: opengl overhaul Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 14:50 ` Max Reitz
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 3/7] pixman: add a bunch of PIXMAN_BE_* defines for 32bpp Gerd Hoffmann
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Jeremy White, Gerd Hoffmann, Max Reitz
From: Jeremy White <jwhite@codeweavers.com>
Signed-off-by: Jeremy White <jwhite@codeweavers.com>
[ kraxel: solve opengl patch conflicts ]
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
configure | 24 +++++++++++++++++++-----
hw/display/Makefile.objs | 1 +
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index 8216410..6d673c1 100755
--- a/configure
+++ b/configure
@@ -2034,6 +2034,15 @@ if test "$sparse" != "no" ; then
fi
##########################################
+# X11 probe
+x11_cflags=
+x11_libs=-lX11
+if $pkg_config --exists "x11"; then
+ x11_cflags=`$pkg_config --cflags x11`
+ x11_libs=`$pkg_config --libs x11`
+fi
+
+##########################################
# GTK probe
if test "$gtkabi" = ""; then
@@ -2060,7 +2069,8 @@ if test "$gtk" != "no"; then
gtk_cflags=`$pkg_config --cflags $gtkpackage`
gtk_libs=`$pkg_config --libs $gtkpackage`
if $pkg_config --exists "$gtkx11package >= $gtkversion"; then
- gtk_libs="$gtk_libs -lX11"
+ gtk_cflags="$gtk_cflags $x11_cflags"
+ gtk_libs="$gtk_libs $x11_libs"
fi
libs_softmmu="$gtk_libs $libs_softmmu"
gtk="yes"
@@ -2185,8 +2195,9 @@ if test "$sdl" = "yes" ; then
#endif
int main(void) { return 0; }
EOF
- if compile_prog "$sdl_cflags" "$sdl_libs" ; then
- sdl_libs="$sdl_libs -lX11"
+ if compile_prog "$sdl_cflags $x11_cflags" "$sdl_libs $x11_libs" ; then
+ sdl_cflags="$sdl_cflags $x11_cflags"
+ sdl_libs="$sdl_libs $x11_libs"
fi
libs_softmmu="$sdl_libs $libs_softmmu"
fi
@@ -3059,13 +3070,15 @@ libs_softmmu="$libs_softmmu $fdt_libs"
# opengl probe (for sdl2, milkymist-tmu2)
if test "$opengl" != "no" ; then
opengl_pkgs="gl"
- if $pkg_config $opengl_pkgs; then
- opengl_libs="$($pkg_config --libs $opengl_pkgs) -lX11"
+ if $pkg_config $opengl_pkgs x11; then
+ opengl_cflags="$($pkg_config --libs $opengl_pkgs) $x11_cflags"
+ opengl_libs="$($pkg_config --libs $opengl_pkgs) $x11_libs"
opengl=yes
else
if test "$opengl" = "yes" ; then
feature_not_found "opengl" "Install GL devel (e.g. MESA)"
fi
+ opengl_cflags=""
opengl_libs=""
opengl=no
fi
@@ -4679,6 +4692,7 @@ fi
if test "$opengl" = "yes" ; then
echo "CONFIG_OPENGL=y" >> $config_host_mak
+ echo "OPENGL_CFLAGS=$opengl_cflags" >> $config_host_mak
echo "OPENGL_LIBS=$opengl_libs" >> $config_host_mak
fi
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index e18ea57..e73cb7d 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_ZAURUS) += tc6393xb.o
ifeq ($(CONFIG_MILKYMIST_TMU2),y)
common-obj-y += milkymist-tmu2.o
+milkymist-tmu2.o-cflags := $(OPENGL_CFLAGS)
libs_softmmu += $(OPENGL_LIBS)
endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH v2 3/7] pixman: add a bunch of PIXMAN_BE_* defines for 32bpp
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 1/7] configure: opengl overhaul Gerd Hoffmann
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 2/7] Allow the use of X11 from a non standard location Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 14:54 ` Max Reitz
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions Gerd Hoffmann
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Max Reitz
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/ui/qemu-pixman.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index 381969d..37f9bc2 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -27,8 +27,24 @@
#ifdef HOST_WORDS_BIGENDIAN
# define PIXMAN_BE_r8g8b8 PIXMAN_r8g8b8
+# define PIXMAN_BE_x8r8g8b8 PIXMAN_x8r8g8b8
+# define PIXMAN_BE_a8r8g8b8 PIXMAN_a8r8g8b8
+# define PIXMAN_BE_b8g8r8x8 PIXMAN_b8g8r8x8
+# define PIXMAN_BE_b8g8r8a8 PIXMAN_b8g8r8a8
+# define PIXMAN_BE_r8g8b8x8 PIXMAN_r8g8b8x8
+# define PIXMAN_BE_r8g8b8a8 PIXMAN_r8g8b8a8
+# define PIXMAN_BE_x8b8g8r8 PIXMAN_x8b8g8r8
+# define PIXMAN_BE_a8b8g8r8 PIXMAN_a8b8g8r8
#else
# define PIXMAN_BE_r8g8b8 PIXMAN_b8g8r8
+# define PIXMAN_BE_x8r8g8b8 PIXMAN_b8g8r8x8
+# define PIXMAN_BE_a8r8g8b8 PIXMAN_b8g8r8a8
+# define PIXMAN_BE_b8g8r8x8 PIXMAN_x8r8g8b8
+# define PIXMAN_BE_b8g8r8a8 PIXMAN_a8r8g8b8
+# define PIXMAN_BE_r8g8b8x8 PIXMAN_x8b8g8r8
+# define PIXMAN_BE_r8g8b8a8 PIXMAN_a8b8g8r8
+# define PIXMAN_BE_x8b8g8r8 PIXMAN_r8g8b8x8
+# define PIXMAN_BE_a8b8g8r8 PIXMAN_r8g8b8a8
#endif
/* -------------------------------------------------------------------- */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
` (2 preceding siblings ...)
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 3/7] pixman: add a bunch of PIXMAN_BE_* defines for 32bpp Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 16:05 ` Max Reitz
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 5/7] console-gl: externalize shader programs Gerd Hoffmann
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori, Max Reitz
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
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="$libs_softmmu $fdt_libs"
##########################################
# opengl probe (for sdl2, milkymist-tmu2)
if test "$opengl" != "no" ; then
- opengl_pkgs="gl"
+ opengl_pkgs="gl glesv2"
if $pkg_config $opengl_pkgs x11; then
opengl_cflags="$($pkg_config --libs $opengl_pkgs) $x11_cflags"
opengl_libs="$($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"
+#ifdef CONFIG_OPENGL
+# include <GLES2/gl2.h>
+# include <GLES2/gl2ext.h>
+#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,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);
+/* 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);
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..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 <kraxel@redhat.com>
+ *
+ * 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"
+
+struct ConsoleGLState {
+ GLint texture_blit_prog;
+};
+
+/* ---------------------------------------------------------------------- */
+
+static GLchar texture_blit_vert_src[] =
+ "\n"
+ "#version 300 es\n"
+ "\n"
+ "in vec2 in_position;\n"
+ "in vec2 in_tex_coord;\n"
+ "out vec2 ex_tex_coord;\n"
+ "\n"
+ "void main(void) {\n"
+ " gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n"
+ " ex_tex_coord = in_tex_coord;\n"
+ "}\n"
+ "\n";
+
+static GLchar texture_blit_frag_src[] =
+ "\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 = texture(image, ex_tex_coord);\n"
+ "}\n"
+ "\n";
+
+static void gl_run_texture_blit(ConsoleGLState *gls)
+{
+ GLfloat in_position[] = {
+ -1, 1,
+ -1, -1,
+ 1, -1,
+ -1, 1,
+ 1, 1,
+ 1, -1,
+ };
+ GLfloat in_tex_coord[] = {
+ 0, 0,
+ 0, 1,
+ 1, 1,
+ 0, 0,
+ 1, 0,
+ 1, 1,
+ };
+ GLint l_position;
+ GLint l_tex_coord;
+
+ glUseProgram(gls->texture_blit_prog);
+ l_position = glGetAttribLocation(gls->texture_blit_prog, "in_position");
+ l_tex_coord = glGetAttribLocation(gls->texture_blit_prog, "in_tex_coord");
+ glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
+ glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_tex_coord);
+ glEnableVertexAttribArray(l_position);
+ glEnableVertexAttribArray(l_tex_coord);
+ glDrawArrays(GL_TRIANGLE_STRIP, l_position, 6);
+}
+
+/* ---------------------------------------------------------------------- */
+
+static GLuint gl_create_compile_shader(GLenum type, const GLchar *src)
+{
+ GLuint shader;
+ GLint status, length;
+ char *errmsg;
+
+ shader = 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 = malloc(length);
+ glGetShaderInfoLog(shader, length, &length, errmsg);
+ fprintf(stderr, "%s: compile %s error\n%s\n", __func__,
+ (type == 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 = 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 = 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 = gl_create_compile_shader(GL_VERTEX_SHADER, vert_src);
+ frag_shader = 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);
+}
+
+/* ---------------------------------------------------------------------- */
+
+ConsoleGLState *console_gl_init_context(void)
+{
+ ConsoleGLState *gls = g_new0(ConsoleGLState, 1);
+
+ gls->texture_blit_prog = 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 = GL_BGRA_EXT;
+ 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);
+ glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
+ surface_stride(surface) / surface_bytes_per_pixel(surface));
+ 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 = (void *)surface_data(surface);
+
+ assert(gls);
+
+ glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
+ surface_stride(surface) / surface_bytes_per_pixel(surface));
+ 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 = 0;
+}
+
+void surface_gl_setup_viewport(ConsoleGLState *gls,
+ DisplaySurface *surface,
+ int ww, int wh)
+{
+ int gw, gh, stripe;
+ float sw, sh;
+
+ assert(gls);
+
+ 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);
+ }
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions Gerd Hoffmann
@ 2015-01-19 16:05 ` Max Reitz
2015-01-20 11:00 ` Gerd Hoffmann
0 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2015-01-19 16:05 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Anthony Liguori
On 2015-01-19 at 08:36, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> 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="$libs_softmmu $fdt_libs"
> ##########################################
> # opengl probe (for sdl2, milkymist-tmu2)
> if test "$opengl" != "no" ; then
> - opengl_pkgs="gl"
> + opengl_pkgs="gl glesv2"
I don't know anything about GLES except for it being (as I've been told)
mostly similar to the normal OpenGL. I hope that'll suffice…
> if $pkg_config $opengl_pkgs x11; then
> opengl_cflags="$($pkg_config --libs $opengl_pkgs) $x11_cflags"
> opengl_libs="$($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"
>
> +#ifdef CONFIG_OPENGL
> +# include <GLES2/gl2.h>
> +# include <GLES2/gl2ext.h>
> +#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,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);
>
> +/* 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);
>
> 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..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 <kraxel@redhat.com>
> + *
> + * 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"
> +
> +struct ConsoleGLState {
> + GLint texture_blit_prog;
> +};
> +
> +/* ---------------------------------------------------------------------- */
> +
> +static GLchar texture_blit_vert_src[] =
> + "\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
shader, but this is mostly my premature optimization instinct kicking in
instead of a real performance difference considering how few vertices
there are in this case.
> + "out vec2 ex_tex_coord;\n"
> + "\n"
> + "void main(void) {\n"
> + " gl_Position = 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 = in_tex_coord;\n"
> + "}\n"
> + "\n";
> +
> +static GLchar texture_blit_frag_src[] =
> + "\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 = texture(image, ex_tex_coord);\n"
> + "}\n"
> + "\n";
> +
> +static void gl_run_texture_blit(ConsoleGLState *gls)
> +{
> + GLfloat in_position[] = {
> + -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.
For GL_TRIANGLE_STRIP, you first define one whole triangle (by
specifying each of the three vertices) and after that, each vertex
results in a new triangle drawn (whose two other vertices are the two
vertices preceding the last one).
Therefore, for a quad, you only need four vertices in GL_TRIANGLE_STRIP
mode. You will find that the following works just fine:
GLfloat in_position[] = {
-1, -1,
1, -1,
-1, 1,
1, 1,
};
(1)
> + };
> + GLfloat in_tex_coord[] = {
> + 0, 0,
> + 0, 1,
> + 1, 1,
> + 0, 0,
> + 1, 0,
> + 1, 1,
> + };
(1) and
GLfloat in_tex_coord[] = {
0, 1,
1, 1,
0, 0,
1, 0,
};
here.
> + GLint l_position;
> + GLint l_tex_coord;
> +
> + glUseProgram(gls->texture_blit_prog);
> + l_position = glGetAttribLocation(gls->texture_blit_prog, "in_position");
> + l_tex_coord = glGetAttribLocation(gls->texture_blit_prog, "in_tex_coord");
> + glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
> + glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_tex_coord);
I think it is disallowed to refer to non-OpenGL buffers here in the core
profile. The 4.4 core specification states (section 10.3, vertex
arrays): "Vertex data are placed into arrays that are stored in the
server’s address space"; the 4.4 compatibility specification states:
"Vertex data may also be placed into arrays that are stored in the
client’s address space (described here) or in the server’s address space".
("client" refers to the main memory, "server" refers to the video
memory, as far as I know)
As I said before, though, I am completely fine with going for the
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 = 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 = malloc(length);
> + glGetShaderInfoLog(shader, length, &length, errmsg);
> + fprintf(stderr, "%s: compile %s error\n%s\n", __func__,
> + (type == 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 = 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 = 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 = gl_create_compile_shader(GL_VERTEX_SHADER, vert_src);
> + frag_shader = 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
linked into the program (because the references will be lost when this
function returns).
> +}
> +
> +/* ---------------------------------------------------------------------- */
> +
> +ConsoleGLState *console_gl_init_context(void)
> +{
> + ConsoleGLState *gls = g_new0(ConsoleGLState, 1);
> +
> + gls->texture_blit_prog = 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 = GL_BGRA_EXT;
If you want to avoid the _EXT, you could use GL_RGBA here and
texture().bgra in the fragment shader. However, this would require a
different shader for the 32 bit and the 16 bit formats (because the 16
bit format has the right order, apparently).
I'm voting for keeping GL_BGRA_EXT until someone complains.
> + surface->gltype = GL_UNSIGNED_BYTE;
> + break;
> + case PIXMAN_r5g6b5:
> + surface->glformat = GL_RGB;
> + surface->gltype = GL_UNSIGNED_SHORT_5_6_5;
I haven't been able to really test 16 bit mode (forcing 16 bit mode in
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(surface));
> + 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 = (void *)surface_data(surface);
> +
> + assert(gls);
> +
> + glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> + surface_stride(surface) / surface_bytes_per_pixel(surface));
Maybe we should assert that surface_stride(surface) %
surface_bytes_per_pixel(surface) == 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 = 0;
> +}
> +
> +void surface_gl_setup_viewport(ConsoleGLState *gls,
> + DisplaySurface *surface,
> + int ww, int wh)
> +{
> + int gw, gh, stripe;
> + float sw, sh;
> +
> + assert(gls);
> +
> + 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);
> + }
> +}
With the vertex data changed to make use of GL_TRIANGLE_STRIP and with
or without the stride assertion and with or without deleting the shaders
after the program has been linked:
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
2015-01-19 16:05 ` Max Reitz
@ 2015-01-20 11:00 ` Gerd Hoffmann
2015-01-20 13:54 ` Max Reitz
0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-20 11:00 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Anthony Liguori
Hi,
> > +static GLchar texture_blit_vert_src[] =
> > + "\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
> shader, but this is mostly my premature optimization instinct kicking in
> instead of a real performance difference considering how few vertices
> there are in this case.
Still makes sense. Done.
> > + "out vec2 ex_tex_coord;\n"
> > + "\n"
> > + "void main(void) {\n"
> > + " gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n"
>
> vec4(in_position, 0.0, 1.0) *cough*
Done.
> This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
> For GL_TRIANGLE_STRIP, you first define one whole triangle (by
> specifying each of the three vertices) and after that, each vertex
> results in a new triangle drawn (whose two other vertices are the two
> vertices preceding the last one).
Thanks for the nice description.
So the trick to get it done with only four vertexes is to put the
correct points (which are going to be shared by both triangles) into the
middle. I've played around with it a bit without success (and before my
new opengl book arrived), and this 6-point version worked ...
> > + glUseProgram(gls->texture_blit_prog);
> > + l_position = glGetAttribLocation(gls->texture_blit_prog, "in_position");
> > + l_tex_coord = glGetAttribLocation(gls->texture_blit_prog, "in_tex_coord");
> > + glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
> > + glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_tex_coord);
>
> I think it is disallowed to refer to non-OpenGL buffers here in the core
> profile. The 4.4 core specification states (section 10.3, vertex
> arrays): "Vertex data are placed into arrays that are stored in the
> server’s address space"; the 4.4 compatibility specification states:
> "Vertex data may also be placed into arrays that are stored in the
> client’s address space (described here) or in the server’s address space".
Got this from gles sample code, so that should be ok ;)
I've also seen code explicitly storing vertex data in a buffer, which I
assume is more efficient, but I decided to not care for now, especially
given the small number of vertexes we are processing here.
> > + return gl_create_link_program(vert_shader, frag_shader);
>
> Minor thing: You are free to delete the shaders after they have been
> linked into the program (because the references will be lost when this
> function returns).
Done.
> > + switch (surface->format) {
> > + case PIXMAN_BE_b8g8r8x8:
> > + case PIXMAN_BE_b8g8r8a8:
> > + surface->glformat = GL_BGRA_EXT;
>
> If you want to avoid the _EXT, you could use GL_RGBA here and
> texture().bgra in the fragment shader. However, this would require a
> different shader for the 32 bit and the 16 bit formats (because the 16
> bit format has the right order, apparently).
At least it worked in testing.
> I haven't been able to really test 16 bit mode (forcing 16 bit mode in
> hw/display/vga.c doesn't count...), so I'm trusting you this works.
-kernel /boot/vmlinux-$whatever -append vga=0x314
Makes the kernel run vesafb with 800x600 @ 16bpp, and thanks to the
little friendly penguin in the upper left corner (CONFIG_LOGO=y) you can
easily spot colors being messed up.
> > + glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> > + surface_stride(surface) / surface_bytes_per_pixel(surface));
>
> Maybe we should assert that surface_stride(surface) %
> surface_bytes_per_pixel(surface) == 0 here.
Done.
thanks,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
2015-01-20 11:00 ` Gerd Hoffmann
@ 2015-01-20 13:54 ` Max Reitz
2015-01-20 14:44 ` Gerd Hoffmann
0 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2015-01-20 13:54 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Anthony Liguori
On 2015-01-20 at 06:00, Gerd Hoffmann wrote:
> Hi,
>
>>> +static GLchar texture_blit_vert_src[] =
>>> + "\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
>> shader, but this is mostly my premature optimization instinct kicking in
>> instead of a real performance difference considering how few vertices
>> there are in this case.
> Still makes sense. Done.
>
>>> + "out vec2 ex_tex_coord;\n"
>>> + "\n"
>>> + "void main(void) {\n"
>>> + " gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n"
>> vec4(in_position, 0.0, 1.0) *cough*
> Done.
>
>> This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
>> For GL_TRIANGLE_STRIP, you first define one whole triangle (by
>> specifying each of the three vertices) and after that, each vertex
>> results in a new triangle drawn (whose two other vertices are the two
>> vertices preceding the last one).
> Thanks for the nice description.
>
> So the trick to get it done with only four vertexes is to put the
> correct points (which are going to be shared by both triangles) into the
> middle. I've played around with it a bit without success (and before my
> new opengl book arrived), and this 6-point version worked ...
Hm, I did write a working solution in my reply, didn't I? It was: { -1,
-1, 1, -1, -1, 1, 1, 1 } for in_position[] and { 0, 1, 1, 1, 0,
0, 1, 0 } for in_tex_coord[]. At least it worked for me.
You haven't (explicitly) enabled face culling, but remember that while
normally counter-clockwise faces are backfaces and thus culled, the
direction is inverted for every triangle; so the first triangle needs to
be CCW, the second CW, the third CCW again, and so on. Maybe that's why
it didn't work for you...
>>> + glUseProgram(gls->texture_blit_prog);
>>> + l_position = glGetAttribLocation(gls->texture_blit_prog, "in_position");
>>> + l_tex_coord = glGetAttribLocation(gls->texture_blit_prog, "in_tex_coord");
>>> + glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
>>> + glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_tex_coord);
>> I think it is disallowed to refer to non-OpenGL buffers here in the core
>> profile. The 4.4 core specification states (section 10.3, vertex
>> arrays): "Vertex data are placed into arrays that are stored in the
>> server’s address space"; the 4.4 compatibility specification states:
>> "Vertex data may also be placed into arrays that are stored in the
>> client’s address space (described here) or in the server’s address space".
> Got this from gles sample code, so that should be ok ;)
>
> I've also seen code explicitly storing vertex data in a buffer, which I
> assume is more efficient, but I decided to not care for now, especially
> given the small number of vertexes we are processing here.
>
>>> + return gl_create_link_program(vert_shader, frag_shader);
>> Minor thing: You are free to delete the shaders after they have been
>> linked into the program (because the references will be lost when this
>> function returns).
> Done.
>
>>> + switch (surface->format) {
>>> + case PIXMAN_BE_b8g8r8x8:
>>> + case PIXMAN_BE_b8g8r8a8:
>>> + surface->glformat = GL_BGRA_EXT;
>> If you want to avoid the _EXT, you could use GL_RGBA here and
>> texture().bgra in the fragment shader. However, this would require a
>> different shader for the 32 bit and the 16 bit formats (because the 16
>> bit format has the right order, apparently).
> At least it worked in testing.
>
>> I haven't been able to really test 16 bit mode (forcing 16 bit mode in
>> hw/display/vga.c doesn't count...), so I'm trusting you this works.
> -kernel /boot/vmlinux-$whatever -append vga=0x314
>
> Makes the kernel run vesafb with 800x600 @ 16bpp, and thanks to the
> little friendly penguin in the upper left corner (CONFIG_LOGO=y) you can
> easily spot colors being messed up.
Thanks!
Max
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
2015-01-20 13:54 ` Max Reitz
@ 2015-01-20 14:44 ` Gerd Hoffmann
2015-01-20 14:52 ` Max Reitz
0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-20 14:44 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Anthony Liguori
Hi,
> >> This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
> >> For GL_TRIANGLE_STRIP, you first define one whole triangle (by
> >> specifying each of the three vertices) and after that, each vertex
> >> results in a new triangle drawn (whose two other vertices are the two
> >> vertices preceding the last one).
> > Thanks for the nice description.
> >
> > So the trick to get it done with only four vertexes is to put the
> > correct points (which are going to be shared by both triangles) into the
> > middle. I've played around with it a bit without success (and before my
> > new opengl book arrived), and this 6-point version worked ...
>
> Hm, I did write a working solution in my reply, didn't I?
Yep, was just trying to explain how I ended up with the 6 vertex
version, not having fully figured how triangle strips are working.
All fine now, I've changed it to use 4 vertexes version instead, and I
_also_ understood why it is working as intended.
FYI: current code is at
https://www.kraxel.org/cgit/qemu/log/?h=rebase/ui-sdl-next
[ incremental fixes not squashed in yet ]
thanks,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
2015-01-20 14:44 ` Gerd Hoffmann
@ 2015-01-20 14:52 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2015-01-20 14:52 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Anthony Liguori
On 2015-01-20 at 09:44, Gerd Hoffmann wrote:
> Hi,
>
>>>> This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
>>>> For GL_TRIANGLE_STRIP, you first define one whole triangle (by
>>>> specifying each of the three vertices) and after that, each vertex
>>>> results in a new triangle drawn (whose two other vertices are the two
>>>> vertices preceding the last one).
>>> Thanks for the nice description.
>>>
>>> So the trick to get it done with only four vertexes is to put the
>>> correct points (which are going to be shared by both triangles) into the
>>> middle. I've played around with it a bit without success (and before my
>>> new opengl book arrived), and this 6-point version worked ...
>> Hm, I did write a working solution in my reply, didn't I?
> Yep, was just trying to explain how I ended up with the 6 vertex
> version, not having fully figured how triangle strips are working.
I see.
> All fine now, I've changed it to use 4 vertexes version instead, and I
> _also_ understood why it is working as intended.
Great. :-)
> FYI: current code is at
> https://www.kraxel.org/cgit/qemu/log/?h=rebase/ui-sdl-next
Thanks!
Max
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] console-gl: externalize shader programs
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
` (3 preceding siblings ...)
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 16:15 ` Max Reitz
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 6/7] sdl2: move SDL_* includes to sdl2.h Gerd Hoffmann
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl Gerd Hoffmann
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori, Max Reitz
---
Makefile | 17 +++++++++++++++++
scripts/shaderinclude.pl | 16 ++++++++++++++++
ui/console-gl.c | 28 ++--------------------------
ui/shader/texture-blit.frag | 10 ++++++++++
ui/shader/texture-blit.vert | 11 +++++++++++
5 files changed, 56 insertions(+), 26 deletions(-)
create mode 100644 scripts/shaderinclude.pl
create mode 100644 ui/shader/texture-blit.frag
create mode 100644 ui/shader/texture-blit.vert
diff --git a/Makefile b/Makefile
index 6817c6f..6d77782 100644
--- a/Makefile
+++ b/Makefile
@@ -292,6 +292,7 @@ clean:
rm -f fsdev/*.pod
rm -rf .libs */.libs
rm -f qemu-img-cmds.h
+ rm -f ui/shader/*-vert.h ui/shader/*-frag.h
@# May not be present in GENERATED_HEADERS
rm -f trace/generated-tracers-dtrace.dtrace*
rm -f trace/generated-tracers-dtrace.h*
@@ -437,6 +438,22 @@ cscope:
find "$(SRC_PATH)" -name "*.[chsS]" -print | sed 's,^\./,,' > ./cscope.files
cscope -b
+# opengl shader programs
+ui/shader/%-vert.h: $(SRC_PATH)/ui/shader/%.vert $(SRC_PATH)/scripts/shaderinclude.pl
+ @mkdir -p $(dir $@)
+ $(call quiet-command,\
+ perl $(SRC_PATH)/scripts/shaderinclude.pl $< > $@,\
+ " VERT $@")
+
+ui/shader/%-frag.h: $(SRC_PATH)/ui/shader/%.frag $(SRC_PATH)/scripts/shaderinclude.pl
+ @mkdir -p $(dir $@)
+ $(call quiet-command,\
+ perl $(SRC_PATH)/scripts/shaderinclude.pl $< > $@,\
+ " FRAG $@")
+
+ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \
+ ui/shader/texture-blit-vert.h ui/shader/texture-blit-frag.h
+
# documentation
MAKEINFO=makeinfo
MAKEINFOFLAGS=--no-headers --no-split --number-sections
diff --git a/scripts/shaderinclude.pl b/scripts/shaderinclude.pl
new file mode 100644
index 0000000..81b5146
--- /dev/null
+++ b/scripts/shaderinclude.pl
@@ -0,0 +1,16 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+
+my $file = shift;
+open FILE, "<", $file or die "open $file: $!";
+my $name = $file;
+$name =~ s|.*/||;
+$name =~ s/[-.]/_/g;
+print "static GLchar ${name}_src[] =\n";
+while (<FILE>) {
+ chomp;
+ printf " \"%s\\n\"\n", $_;
+}
+print " \"\\n\";\n";
+close FILE;
diff --git a/ui/console-gl.c b/ui/console-gl.c
index 589c682..2c9412d 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -33,32 +33,8 @@ struct ConsoleGLState {
/* ---------------------------------------------------------------------- */
-static GLchar texture_blit_vert_src[] =
- "\n"
- "#version 300 es\n"
- "\n"
- "in vec2 in_position;\n"
- "in vec2 in_tex_coord;\n"
- "out vec2 ex_tex_coord;\n"
- "\n"
- "void main(void) {\n"
- " gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n"
- " ex_tex_coord = in_tex_coord;\n"
- "}\n"
- "\n";
-
-static GLchar texture_blit_frag_src[] =
- "\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 = texture(image, ex_tex_coord);\n"
- "}\n"
- "\n";
+#include "shader/texture-blit-vert.h"
+#include "shader/texture-blit-frag.h"
static void gl_run_texture_blit(ConsoleGLState *gls)
{
diff --git a/ui/shader/texture-blit.frag b/ui/shader/texture-blit.frag
new file mode 100644
index 0000000..148b1aa
--- /dev/null
+++ b/ui/shader/texture-blit.frag
@@ -0,0 +1,10 @@
+
+#version 300 es
+
+uniform sampler2D image;
+in highp vec2 ex_tex_coord;
+out highp vec4 out_frag_color;
+
+void main(void) {
+ out_frag_color = texture(image, ex_tex_coord);
+}
diff --git a/ui/shader/texture-blit.vert b/ui/shader/texture-blit.vert
new file mode 100644
index 0000000..4ffb5d1
--- /dev/null
+++ b/ui/shader/texture-blit.vert
@@ -0,0 +1,11 @@
+
+#version 300 es
+
+in vec2 in_position;
+in vec2 in_tex_coord;
+out vec2 ex_tex_coord;
+
+void main(void) {
+ gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);
+ ex_tex_coord = in_tex_coord;
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/7] console-gl: externalize shader programs
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 5/7] console-gl: externalize shader programs Gerd Hoffmann
@ 2015-01-19 16:15 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2015-01-19 16:15 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Anthony Liguori
On 2015-01-19 at 08:36, Gerd Hoffmann wrote:
> ---
S-o-b is missing.
> Makefile | 17 +++++++++++++++++
> scripts/shaderinclude.pl | 16 ++++++++++++++++
> ui/console-gl.c | 28 ++--------------------------
> ui/shader/texture-blit.frag | 10 ++++++++++
> ui/shader/texture-blit.vert | 11 +++++++++++
> 5 files changed, 56 insertions(+), 26 deletions(-)
> create mode 100644 scripts/shaderinclude.pl
> create mode 100644 ui/shader/texture-blit.frag
> create mode 100644 ui/shader/texture-blit.vert
>
> diff --git a/Makefile b/Makefile
> index 6817c6f..6d77782 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -292,6 +292,7 @@ clean:
> rm -f fsdev/*.pod
> rm -rf .libs */.libs
> rm -f qemu-img-cmds.h
> + rm -f ui/shader/*-vert.h ui/shader/*-frag.h
> @# May not be present in GENERATED_HEADERS
> rm -f trace/generated-tracers-dtrace.dtrace*
> rm -f trace/generated-tracers-dtrace.h*
> @@ -437,6 +438,22 @@ cscope:
> find "$(SRC_PATH)" -name "*.[chsS]" -print | sed 's,^\./,,' > ./cscope.files
> cscope -b
>
> +# opengl shader programs
> +ui/shader/%-vert.h: $(SRC_PATH)/ui/shader/%.vert $(SRC_PATH)/scripts/shaderinclude.pl
> + @mkdir -p $(dir $@)
> + $(call quiet-command,\
> + perl $(SRC_PATH)/scripts/shaderinclude.pl $< > $@,\
> + " VERT $@")
> +
> +ui/shader/%-frag.h: $(SRC_PATH)/ui/shader/%.frag $(SRC_PATH)/scripts/shaderinclude.pl
> + @mkdir -p $(dir $@)
> + $(call quiet-command,\
> + perl $(SRC_PATH)/scripts/shaderinclude.pl $< > $@,\
> + " FRAG $@")
> +
> +ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \
> + ui/shader/texture-blit-vert.h ui/shader/texture-blit-frag.h
> +
> # documentation
> MAKEINFO=makeinfo
> MAKEINFOFLAGS=--no-headers --no-split --number-sections
> diff --git a/scripts/shaderinclude.pl b/scripts/shaderinclude.pl
> new file mode 100644
> index 0000000..81b5146
> --- /dev/null
> +++ b/scripts/shaderinclude.pl
> @@ -0,0 +1,16 @@
> +#!/usr/bin/perl
> +use strict;
> +use warnings;
> +
> +my $file = shift;
> +open FILE, "<", $file or die "open $file: $!";
> +my $name = $file;
> +$name =~ s|.*/||;
> +$name =~ s/[-.]/_/g;
> +print "static GLchar ${name}_src[] =\n";
> +while (<FILE>) {
> + chomp;
> + printf " \"%s\\n\"\n", $_;
> +}
> +print " \"\\n\";\n";
> +close FILE;
> diff --git a/ui/console-gl.c b/ui/console-gl.c
> index 589c682..2c9412d 100644
> --- a/ui/console-gl.c
> +++ b/ui/console-gl.c
> @@ -33,32 +33,8 @@ struct ConsoleGLState {
>
> /* ---------------------------------------------------------------------- */
>
> -static GLchar texture_blit_vert_src[] =
> - "\n"
> - "#version 300 es\n"
> - "\n"
> - "in vec2 in_position;\n"
> - "in vec2 in_tex_coord;\n"
> - "out vec2 ex_tex_coord;\n"
> - "\n"
> - "void main(void) {\n"
> - " gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n"
> - " ex_tex_coord = in_tex_coord;\n"
> - "}\n"
> - "\n";
> -
> -static GLchar texture_blit_frag_src[] =
> - "\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 = texture(image, ex_tex_coord);\n"
> - "}\n"
> - "\n";
> +#include "shader/texture-blit-vert.h"
> +#include "shader/texture-blit-frag.h"
>
> static void gl_run_texture_blit(ConsoleGLState *gls)
> {
> diff --git a/ui/shader/texture-blit.frag b/ui/shader/texture-blit.frag
> new file mode 100644
> index 0000000..148b1aa
> --- /dev/null
> +++ b/ui/shader/texture-blit.frag
> @@ -0,0 +1,10 @@
> +
> +#version 300 es
> +
> +uniform sampler2D image;
> +in highp vec2 ex_tex_coord;
> +out highp vec4 out_frag_color;
Apart from me not knowing what highp really does because I don't know
GLES, I can imagine what it's probably supposed to do (high-precision);
do we really need it for these values?
(I know, I should've said that in the previous patch already…)
> +
> +void main(void) {
> + out_frag_color = texture(image, ex_tex_coord);
> +}
> diff --git a/ui/shader/texture-blit.vert b/ui/shader/texture-blit.vert
> new file mode 100644
> index 0000000..4ffb5d1
> --- /dev/null
> +++ b/ui/shader/texture-blit.vert
> @@ -0,0 +1,11 @@
> +
> +#version 300 es
> +
> +in vec2 in_position;
> +in vec2 in_tex_coord;
> +out vec2 ex_tex_coord;
> +
> +void main(void) {
> + gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);
> + ex_tex_coord = in_tex_coord;
> +}
gl_Position = vec4(in_position, 0.0, 1.0);
ex_tex_coord = vec2(1.0 + in_position.x, 1.0 - in_position.y) * 0.5;
*duck*
Anyway:
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 6/7] sdl2: move SDL_* includes to sdl2.h
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
` (4 preceding siblings ...)
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 5/7] console-gl: externalize shader programs Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 16:16 ` Max Reitz
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl Gerd Hoffmann
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori, Max Reitz
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/ui/sdl2.h | 6 ++++++
ui/sdl2-2d.c | 6 ------
ui/sdl2-input.c | 6 ------
ui/sdl2.c | 6 ------
4 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index f56c596..148308c 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -1,6 +1,12 @@
#ifndef SDL2_H
#define SDL2_H
+/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
+#undef WIN32_LEAN_AND_MEAN
+
+#include <SDL.h>
+#include <SDL_syswm.h>
+
struct sdl2_console {
DisplayChangeListener dcl;
DisplaySurface *surface;
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 9264817..7a82d50 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -23,12 +23,6 @@
*/
/* 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 "qemu-common.h"
#include "ui/console.h"
#include "ui/input.h"
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index a1973fc..ac5dc94 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -23,12 +23,6 @@
*/
/* 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 "qemu-common.h"
#include "ui/console.h"
#include "ui/input.h"
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 1ae2781..943d1da 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -23,12 +23,6 @@
*/
/* 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 "qemu-common.h"
#include "ui/console.h"
#include "ui/input.h"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl.
2015-01-19 13:36 [Qemu-devel] [PATCH v2 0/7] sdl2: add opengl rendering support Gerd Hoffmann
` (5 preceding siblings ...)
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 6/7] sdl2: move SDL_* includes to sdl2.h Gerd Hoffmann
@ 2015-01-19 13:36 ` Gerd Hoffmann
2015-01-19 16:22 ` Max Reitz
6 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-19 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Gerd Hoffmann, Anthony Liguori, Max Reitz
Add new sdl2-gl.c file, with display
rendering functions using opengl.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/ui/console.h | 1 +
include/ui/sdl2.h | 11 +++++
ui/Makefile.objs | 3 ++
ui/sdl.c | 11 +++++
ui/sdl2-2d.c | 7 ++++
ui/sdl2-gl.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++
ui/sdl2.c | 67 ++++++++++++++++++++++++++----
vl.c | 11 +++++
8 files changed, 216 insertions(+), 7 deletions(-)
create mode 100644 ui/sdl2-gl.c
diff --git a/include/ui/console.h b/include/ui/console.h
index 9157b64..1540d67 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -352,6 +352,7 @@ void surface_gl_setup_viewport(ConsoleGLState *gls,
#endif
/* sdl.c */
+void sdl_display_early_init(int opengl);
void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
/* cocoa.m */
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 148308c..3f89d30 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -17,6 +17,10 @@ struct sdl2_console {
int last_vm_running; /* per console for caption reasons */
int x, y;
int hidden;
+ int opengl;
+ int updates;
+ SDL_GLContext winctx;
+ ConsoleGLState *gls;
};
void sdl2_window_create(struct sdl2_console *scon);
@@ -35,4 +39,11 @@ void sdl2_2d_switch(DisplayChangeListener *dcl,
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);
+
#endif /* SDL2_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 3173778..a88f730 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -21,6 +21,9 @@ 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
+endif
endif
sdl.mo-cflags := $(SDL_CFLAGS)
diff --git a/ui/sdl.c b/ui/sdl.c
index 3e9d810..1fe002d 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -873,6 +873,17 @@ static const DisplayChangeListenerOps dcl_ops = {
.dpy_cursor_define = sdl_mouse_define,
};
+void sdl_display_early_init(int opengl)
+{
+ if (opengl == 1 /* on */) {
+ fprintf(stderr,
+ "SDL1 display code has no opengl support.\n"
+ "Please recompile qemu with SDL2, using\n"
+ "./configure --enable-sdl --with-sdlabi=2.0\n");
+ exit(1);
+ }
+}
+
void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
{
int flags;
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 7a82d50..f6cb438 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -36,6 +36,8 @@ void sdl2_2d_update(DisplayChangeListener *dcl,
DisplaySurface *surf = qemu_console_surface(dcl->con);
SDL_Rect rect;
+ assert(!scon->opengl);
+
if (!surf) {
return;
}
@@ -61,6 +63,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) {
@@ -101,12 +105,15 @@ void sdl2_2d_refresh(DisplayChangeListener *dcl)
{
struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+ assert(!scon->opengl);
graphic_hw_update(dcl->con);
sdl2_poll_events(scon);
}
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..b604c06
--- /dev/null
+++ b/ui/sdl2-gl.c
@@ -0,0 +1,112 @@
+/*
+ * QEMU SDL display driver -- opengl support
+ *
+ * Copyright (c) 2014 Red Hat
+ *
+ * Authors:
+ * Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * 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"
+#include "ui/input.h"
+#include "ui/sdl2.h"
+#include "sysemu/sysemu.h"
+
+static void sdl2_gl_render_surface(struct sdl2_console *scon)
+{
+ int ww, wh;
+
+ SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+
+ SDL_GetWindowSize(scon->real_window, &ww, &wh);
+ surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
+
+ surface_gl_render_texture(scon->gls, scon->surface);
+ SDL_GL_SwapWindow(scon->real_window);
+}
+
+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);
+
+ SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+ surface_gl_update_texture(scon->gls, scon->surface, x, y, w, h);
+ 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);
+
+ SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+ surface_gl_destroy_texture(scon->gls, scon->surface);
+
+ scon->surface = new_surface;
+
+ if (!new_surface) {
+ console_gl_fini_context(scon->gls);
+ scon->gls = NULL;
+ sdl2_window_destroy(scon);
+ return;
+ }
+
+ if (!scon->real_window) {
+ sdl2_window_create(scon);
+ scon->gls = console_gl_init_context();
+ } else if (old_surface &&
+ ((surface_width(old_surface) != surface_width(new_surface)) ||
+ (surface_height(old_surface) != surface_height(new_surface)))) {
+ sdl2_window_resize(scon);
+ }
+
+ surface_gl_create_texture(scon->gls, scon->surface);
+}
+
+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 943d1da..3be9de7 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -86,6 +86,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);
}
@@ -112,6 +115,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];
@@ -310,7 +324,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)
@@ -358,8 +372,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
@@ -378,7 +394,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
@@ -514,10 +530,10 @@ static void handle_windowevent(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:
@@ -670,6 +686,37 @@ 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_early_init(int opengl)
+{
+ switch (opengl) {
+ case -1: /* default */
+ case 0: /* off */
+ break;
+ case 1: /* on */
+#ifdef CONFIG_OPENGL
+ display_opengl = 1;
+#else
+ fprintf(stderr, "SDL2 has been compiled without opengl support\n");
+ exit(1);
+#endif
+ break;
+ default:
+ g_assert_not_reached();
+ break;
+ }
+}
+
void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
{
int flags;
@@ -715,10 +762,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 = display_opengl;
+ sdl2_console[i].dcl.ops = display_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. */
diff --git a/vl.c b/vl.c
index 0bf2acb..3ce1389 100644
--- a/vl.c
+++ b/vl.c
@@ -1947,6 +1947,7 @@ static DisplayType select_display(const char *p)
{
const char *opts;
DisplayType display = DT_DEFAULT;
+ int opengl = -1;
if (strstart(p, "sdl", &opts)) {
#ifdef CONFIG_SDL
@@ -1990,6 +1991,15 @@ static DisplayType select_display(const char *p)
} else {
goto invalid_sdl_args;
}
+ } else if (strstart(opts, ",gl=", &nextopt)) {
+ opts = nextopt;
+ if (strstart(opts, "on", &nextopt)) {
+ opengl = 1;
+ } else if (strstart(opts, "off", &nextopt)) {
+ opengl = 0;
+ } else {
+ goto invalid_sdl_args;
+ }
} else {
invalid_sdl_args:
fprintf(stderr, "Invalid SDL option string: %s\n", p);
@@ -1997,6 +2007,7 @@ static DisplayType select_display(const char *p)
}
opts = nextopt;
}
+ sdl_display_early_init(opengl);
#else
fprintf(stderr, "SDL support is disabled\n");
exit(1);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl.
2015-01-19 13:36 ` [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl Gerd Hoffmann
@ 2015-01-19 16:22 ` Max Reitz
2015-01-20 11:13 ` Gerd Hoffmann
0 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2015-01-19 16:22 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Paolo Bonzini, Anthony Liguori
On 2015-01-19 at 08:36, 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/console.h | 1 +
> include/ui/sdl2.h | 11 +++++
> ui/Makefile.objs | 3 ++
> ui/sdl.c | 11 +++++
> ui/sdl2-2d.c | 7 ++++
> ui/sdl2-gl.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++
> ui/sdl2.c | 67 ++++++++++++++++++++++++++----
> vl.c | 11 +++++
> 8 files changed, 216 insertions(+), 7 deletions(-)
> create mode 100644 ui/sdl2-gl.c
[snip]
> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> new file mode 100644
> index 0000000..b604c06
> --- /dev/null
> +++ b/ui/sdl2-gl.c
> @@ -0,0 +1,112 @@
> +/*
> + * QEMU SDL display driver -- opengl support
> + *
> + * Copyright (c) 2014 Red Hat
> + *
> + * Authors:
> + * Gerd Hoffmann <kraxel@redhat.com>
> + *
> + * 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"
> +#include "ui/input.h"
> +#include "ui/sdl2.h"
> +#include "sysemu/sysemu.h"
> +
> +static void sdl2_gl_render_surface(struct sdl2_console *scon)
> +{
> + int ww, wh;
> +
> + SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> +
> + SDL_GetWindowSize(scon->real_window, &ww, &wh);
> + surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
> +
> + surface_gl_render_texture(scon->gls, scon->surface);
> + SDL_GL_SwapWindow(scon->real_window);
> +}
> +
> +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);
> +
> + SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> + surface_gl_update_texture(scon->gls, scon->surface, x, y, w, h);
> + 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);
> +
> + SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> + surface_gl_destroy_texture(scon->gls, scon->surface);
Same question as for v1: Can a surface be in use by multiple DCLs?
> +
> + scon->surface = new_surface;
> +
> + if (!new_surface) {
> + console_gl_fini_context(scon->gls);
> + scon->gls = NULL;
> + sdl2_window_destroy(scon);
> + return;
> + }
> +
> + if (!scon->real_window) {
> + sdl2_window_create(scon);
> + scon->gls = console_gl_init_context();
> + } else if (old_surface &&
> + ((surface_width(old_surface) != surface_width(new_surface)) ||
> + (surface_height(old_surface) != surface_height(new_surface)))) {
And as in v1: If the window is scaled, this will reset the scaling to
100 %, which is fine. However, if the new surface has the same
dimensions as the old surface, the window will not be scaled. That would
seem strange to me (why is the scaling reset for some surface changes
but not for others?).
Max
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl.
2015-01-19 16:22 ` Max Reitz
@ 2015-01-20 11:13 ` Gerd Hoffmann
0 siblings, 0 replies; 20+ messages in thread
From: Gerd Hoffmann @ 2015-01-20 11:13 UTC (permalink / raw)
To: Max Reitz; +Cc: Paolo Bonzini, qemu-devel, Anthony Liguori
Hi,
> > +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);
> > +
> > + SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> > + surface_gl_destroy_texture(scon->gls, scon->surface);
>
> Same question as for v1: Can a surface be in use by multiple DCLs?
Oops, I remember that comment for the last series, forgot to answer ...
Yes. Old surface is released after notifying all DCLs about the new one
(see dpy_gfx_replace_surface() in console.c), so the old is still valid
at this point. That way refcounting should not be needed in most cases.
If a DCL needs the old surface image stay around after it returns from
the switch callback (say due to threads still holding pointers to it) it
can grab a reference on the pixman image backing the surface:
pixman_image_ref(surface->image). The surfaces itself are not reference
counted.
> > + } else if (old_surface &&
> > + ((surface_width(old_surface) != surface_width(new_surface)) ||
> > + (surface_height(old_surface) != surface_height(new_surface)))) {
>
> And as in v1: If the window is scaled, this will reset the scaling to
> 100 %, which is fine. However, if the new surface has the same
> dimensions as the old surface, the window will not be scaled. That would
> seem strange to me (why is the scaling reset for some surface changes
> but not for others?).
Yea, there are some corner cases. But they are not specific to opengl,
they happen in sdl2-2d mode too, so that is something for another patch
series ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread