[Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Gerd Hoffmann]

  Hi,

This series add support for screen rendering using opengl.  This only
blits classic DisplaySurfaces to the screen using opengl, it does not
(yet) enable gfx emulation use opengl for rendering.

It depends on the "sdl2: fixes, cleanups and opengl preparation"
series just posted.

The big question mark (and the reason this is RfC) is how we are going
to enable/disable opengl at runtime.  The options I see are:

 (1) make that a ui option, i.e. '-display sdl,opengl={on,off}'.
 (2) make that a global option, i.e. -opengl={on,off}

I'd tend to go for (2).

First, because we have a initialization order issue once gfx emulation
starts using opengl:  ui code is initialized after device code (and this
can't be changed easily because devices need register displays before we
init the ui).  Therefore with (1) the device emulation doesn't know at
init time time whenever opengl is available or not.

Second for the long term there will be 3d support in a number of UIs:
I expect sdl2, gtk, egl (using render nodes, for headless) and spice.
Having a global switch for them all looks easier.

Comments?
Other suggestions?

cheers,
  Gerd

Gerd Hoffmann (3):
  configure: opengl overhaul
  sdl2: add support for display rendering using opengl.
  sdl2: move SDL_* includes to sdl2.h

 configure                        |  39 +++++------
 default-configs/lm32-softmmu.mak |   2 +-
 hw/display/Makefile.objs         |   2 +-
 hw/lm32/milkymist-hw.h           |   4 +-
 include/ui/sdl2.h                |  17 +++++
 ui/Makefile.objs                 |   4 ++
 ui/sdl2-2d.c                     |  12 ++--
 ui/sdl2-gl.c                     | 136 +++++++++++++++++++++++++++++++++++++++
 ui/sdl2-input.c                  |   6 --
 ui/sdl2.c                        |  56 ++++++++++++----
 10 files changed, 227 insertions(+), 51 deletions(-)
 create mode 100644 ui/sdl2-gl.c

-- 
1.8.3.1

[Qemu-devel] [RfC PATCH 1/3] configure: opengl overhaul

[Gerd Hoffmann]

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 ++--
 4 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/configure b/configure
index 47048f0..175dea2 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, used by 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;
-- 
1.8.3.1

[Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Gerd Hoffmann]

Add new sdl2-gl.c file, with display
rendering functions using opengl.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/sdl2.h |  10 ++++
 ui/Makefile.objs  |   4 ++
 ui/sdl2-2d.c      |   6 +++
 ui/sdl2-gl.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ui/sdl2.c         |  50 ++++++++++++++++---
 5 files changed, 206 insertions(+), 7 deletions(-)
 create mode 100644 ui/sdl2-gl.c

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 9e9a92d..ba90a91 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -8,6 +8,9 @@ struct sdl2_console {
     int last_vm_running; /* per console for caption reasons */
     int x, y;
     int hidden;
+    int opengl;
+    int updates;
+    SDL_GLContext winctx;
 };
 
 void sdl2_window_create(struct sdl2_console *scon);
@@ -25,3 +28,10 @@ void sdl2_2d_switch(DisplayChangeListener *dcl,
                     DisplaySurface *new_surface);
 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);
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 13b5cfb..b86bdb1 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -21,6 +21,10 @@ 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
+libs_softmmu += $(OPENGL_LIBS)
+endif
 endif
 sdl.mo-cflags := $(SDL_CFLAGS)
 
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 9c60075..85f1be4 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -42,6 +42,8 @@ void sdl2_2d_update(DisplayChangeListener *dcl,
     DisplaySurface *surf = qemu_console_surface(dcl->con);
     SDL_Rect rect;
 
+    assert(!scon->opengl);
+
     if (!surf) {
         return;
     }
@@ -67,6 +69,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) {
@@ -114,6 +118,8 @@ void sdl2_2d_refresh(DisplayChangeListener *dcl)
 
 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..30018d4
--- /dev/null
+++ b/ui/sdl2-gl.c
@@ -0,0 +1,143 @@
+/*
+ * QEMU SDL display driver
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/* 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 <SDL_opengl.h>
+
+#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 gw, gh, ww, wh, stripe;
+    float sw, sh;
+    GLuint tex;
+
+    gw = surface_width(scon->surface);
+    gh = surface_height(scon->surface);
+    SDL_GetWindowSize(scon->real_window, &ww, &wh);
+    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+
+    sw = (float)ww/gw;
+    sh = (float)wh/gh;
+    if (sw < sh) {
+        stripe = wh - wh*sw/sh;
+        glViewport(0, stripe / 2, ww, wh - stripe);
+    } else {
+        stripe = ww - ww*sh/sw;
+        glViewport(stripe / 2, 0, ww - stripe, wh);
+    }
+
+    glMatrixMode(GL_PROJECTION);
+    glLoadIdentity();
+
+    glMatrixMode(GL_MODELVIEW);
+    glLoadIdentity();
+
+    glClearColor(0.0, 0.0, 0.0, 0);
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+
+    glGenTextures(1, &tex);
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
+                 0, GL_BGRA_EXT, GL_UNSIGNED_BYTE,
+                 surface_data(scon->surface));
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+
+    glEnable(GL_TEXTURE_2D);
+    glBegin(GL_QUADS);
+    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
+    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
+    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
+    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
+    glEnd();
+
+    SDL_GL_SwapWindow(scon->real_window);
+
+    glDisable(GL_TEXTURE_2D);
+    glDeleteTextures(1, &tex);
+}
+
+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);
+    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);
+
+    scon->surface = new_surface;
+
+    if (!new_surface) {
+        sdl2_window_destroy(scon);
+        return;
+    }
+
+    if (!scon->real_window) {
+        sdl2_window_create(scon);
+    } else if (old_surface &&
+               ((surface_width(old_surface)  != surface_width(new_surface)) ||
+                (surface_height(old_surface) != surface_height(new_surface)))) {
+        sdl2_window_resize(scon);
+    }
+}
+
+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 a1def81..1e14a2a 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -92,6 +92,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);
 }
 
@@ -118,6 +121,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];
@@ -316,7 +330,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)
@@ -364,8 +378,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
@@ -384,7 +400,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
@@ -518,10 +534,10 @@ static void handle_windowevent(struct sdl2_console *scon, 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:
@@ -664,8 +680,22 @@ 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_init(DisplayState *ds, int full_screen, int no_frame)
 {
+#ifdef CONFIG_OPENGL
+    int opengl = 1;
+#endif
     int flags;
     uint8_t data = 0;
     char *filename;
@@ -709,10 +739,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 = opengl;
+        sdl2_console[i].dcl.ops = 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. */
-- 
1.8.3.1

[Qemu-devel] [RfC PATCH 3/3] sdl2: move SDL_* includes to sdl2.h

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/sdl2.h | 7 +++++++
 ui/sdl2-2d.c      | 6 ------
 ui/sdl2-gl.c      | 7 -------
 ui/sdl2-input.c   | 6 ------
 ui/sdl2.c         | 6 ------
 5 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index ba90a91..0b63556 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -1,3 +1,10 @@
+/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
+#undef WIN32_LEAN_AND_MEAN
+
+#include <SDL.h>
+#include <SDL_syswm.h>
+#include <SDL_opengl.h>
+
 struct sdl2_console {
     DisplayChangeListener dcl;
     DisplaySurface *surface;
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 85f1be4..f6cb438 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-gl.c b/ui/sdl2-gl.c
index 30018d4..3046d2d 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -23,13 +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 <SDL_opengl.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 6702e8e..252f099 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 1e14a2a..24e4c9a 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

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Daniel P. Berrange]

On Thu, Dec 11, 2014 at 12:05:53PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> This series add support for screen rendering using opengl.  This only
> blits classic DisplaySurfaces to the screen using opengl, it does not
> (yet) enable gfx emulation use opengl for rendering.
> 
> It depends on the "sdl2: fixes, cleanups and opengl preparation"
> series just posted.
> 
> The big question mark (and the reason this is RfC) is how we are going
> to enable/disable opengl at runtime.  The options I see are:
> 
>  (1) make that a ui option, i.e. '-display sdl,opengl={on,off}'.
>  (2) make that a global option, i.e. -opengl={on,off}
> 
> I'd tend to go for (2).
> 
> First, because we have a initialization order issue once gfx emulation
> starts using opengl:  ui code is initialized after device code (and this
> can't be changed easily because devices need register displays before we
> init the ui).  Therefore with (1) the device emulation doesn't know at
> init time time whenever opengl is available or not.
> 
> Second for the long term there will be 3d support in a number of UIs:
> I expect sdl2, gtk, egl (using render nodes, for headless) and spice.
> Having a global switch for them all looks easier.

In libvirt we model the ability to turn on 3d support for guests against
the video device, rather than the backend device, since it is a guest
visible feature. So should we do this against the virtual virtio-vga
device and have the backends just tie their behaviour to that ?

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Paolo Bonzini]

On 11/12/2014 12:05, Gerd Hoffmann wrote:
>   Hi,
> 
> This series add support for screen rendering using opengl.  This only
> blits classic DisplaySurfaces to the screen using opengl, it does not
> (yet) enable gfx emulation use opengl for rendering.
> 
> It depends on the "sdl2: fixes, cleanups and opengl preparation"
> series just posted.
> 
> The big question mark (and the reason this is RfC) is how we are going
> to enable/disable opengl at runtime.  The options I see are:
> 
>  (1) make that a ui option, i.e. '-display sdl,opengl={on,off}'.
>  (2) make that a global option, i.e. -opengl={on,off}
> 
> I'd tend to go for (2).
> 
> First, because we have a initialization order issue once gfx emulation
> starts using opengl:  ui code is initialized after device code (and this
> can't be changed easily because devices need register displays before we
> init the ui).  Therefore with (1) the device emulation doesn't know at
> init time time whenever opengl is available or not.

Do you have a case in mind where the device emulation needs to know that?
As opposed to "always need opengl" or "never needs opengl".

If not, the semantics of (1) are easy to describe, though maybe not to
implement:

   device request        backend support     result
     no                    no                  no
     no                    yes                 yes
     no                    unspecified         no or yes, depends on backend
     yes                   no                  error
     yes                   yes                 yes
     yes                   unspecified         error or yes, depends on backend

> Second for the long term there will be 3d support in a number of UIs:
> I expect sdl2, gtk, egl (using render nodes, for headless) and spice.
> Having a global switch for them all looks easier.

But you'd still need to know the UI in order to have a decent default.  For
example should "-display vnc" should disable opengl by default (it's not
in your list above)?  "-display egl" probably should enable it.

Perhaps "-display" could be processed in two steps.

Paolo

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Gerd Hoffmann]

  Hi,

> > Second for the long term there will be 3d support in a number of UIs:
> > I expect sdl2, gtk, egl (using render nodes, for headless) and spice.
> > Having a global switch for them all looks easier.
> 
> In libvirt we model the ability to turn on 3d support for guests against
> the video device, rather than the backend device, since it is a guest
> visible feature. So should we do this against the virtual virtio-vga
> device and have the backends just tie their behaviour to that ?

There will be a switch for the video device anyway, probably something
along the lines of '-device virtio-vga,opengl={on,off,auto}'.

'off' behavior is obvious ;)
'on' must fail in case the ui has no 3d support, but the video doesn't
know that at initialization time.
'auto' should enable 3d in case ui supports it, but again we have the
problem here is that the device doesn't know at init time.

With a global switch we can tell the video device 'we want run
with/without 3d support' and have it initialize accordingly.  Likewise
ui initialization can check the flag and figure whenever it should take
the 3d or non-3d code paths.  Finally after ui init is done we can look
whenever we actually have 3d support when it was requested and error out
in case we havn't.

Also note that opengl in the ui is useful even in case the video device
doesn't support opengl: scaling the display with opengl is faster than
doing it in software, so letting virtio-vga flip the global 3d switch
isn't a good idea too.

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Gerd Hoffmann]

  Hi,

> Do you have a case in mind where the device emulation needs to know that?
> As opposed to "always need opengl" or "never needs opengl".

virtio-gpu has optional opengl support, and ideally the default behavior
is 'enable opengl support in case the UI supports it'.

> But you'd still need to know the UI in order to have a decent default.  For
> example should "-display vnc" should disable opengl by default (it's not
> in your list above)?  "-display egl" probably should enable it.

For SDL I'd like to have it initially disabled by default, then later
when it proved to be stable flip the default to enabled.

> Perhaps "-display" could be processed in two steps.

Hmm, that could work.  Have some ${ui}_early_init(), called when
parameters are parsed, to figure whenever opengl is available (and
possibly have different opengl-enable defaults per UI) but don't do full
initialization yet.

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Daniel P. Berrange]

On Thu, Dec 11, 2014 at 04:29:00PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > Second for the long term there will be 3d support in a number of UIs:
> > > I expect sdl2, gtk, egl (using render nodes, for headless) and spice.
> > > Having a global switch for them all looks easier.
> > 
> > In libvirt we model the ability to turn on 3d support for guests against
> > the video device, rather than the backend device, since it is a guest
> > visible feature. So should we do this against the virtual virtio-vga
> > device and have the backends just tie their behaviour to that ?
> 
> There will be a switch for the video device anyway, probably something
> along the lines of '-device virtio-vga,opengl={on,off,auto}'.
> 
> 'off' behavior is obvious ;)
> 'on' must fail in case the ui has no 3d support, but the video doesn't
> know that at initialization time.
> 'auto' should enable 3d in case ui supports it, but again we have the
> problem here is that the device doesn't know at init time.
> 
> With a global switch we can tell the video device 'we want run
> with/without 3d support' and have it initialize accordingly.  Likewise
> ui initialization can check the flag and figure whenever it should take
> the 3d or non-3d code paths.  Finally after ui init is done we can look
> whenever we actually have 3d support when it was requested and error out
> in case we havn't.

Consider mutli-head for a minute. There could be a scenario where we
want to allow 3d usage in one head, but not in the other head. This
might in turn mean we would want to enable opengl in one display
backend but not the other, depending on which head was connected.
eg so only one tenant gets exclusive usage of the 3d features.
A single -global wouldn't allow that to be done.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Gerd Hoffmann]

  Hi,

> Consider mutli-head for a minute. There could be a scenario where we
> want to allow 3d usage in one head, but not in the other head. This
> might in turn mean we would want to enable opengl in one display
> backend but not the other, depending on which head was connected.
> eg so only one tenant gets exclusive usage of the 3d features.
> A single -global wouldn't allow that to be done.

Would be more multi-seat, but yes, obscure but valid use case.

See reply to paolo, when splitting UI initialization into two stages
this should be doable with a per-UI option.

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Max Reitz]

On 2014-12-11 at 12:05, 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/sdl2.h |  10 ++++
>   ui/Makefile.objs  |   4 ++
>   ui/sdl2-2d.c      |   6 +++
>   ui/sdl2-gl.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ui/sdl2.c         |  50 ++++++++++++++++---
>   5 files changed, 206 insertions(+), 7 deletions(-)
>   create mode 100644 ui/sdl2-gl.c
>
> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> new file mode 100644
> index 0000000..30018d4
> --- /dev/null
> +++ b/ui/sdl2-gl.c
> @@ -0,0 +1,143 @@
> +/*
> + * QEMU SDL display driver
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +/* 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 <SDL_opengl.h>
> +
> +#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 gw, gh, ww, wh, stripe;
> +    float sw, sh;
> +    GLuint tex;
> +
> +    gw = surface_width(scon->surface);
> +    gh = surface_height(scon->surface);
> +    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> +
> +    sw = (float)ww/gw;
> +    sh = (float)wh/gh;
> +    if (sw < sh) {
> +        stripe = wh - wh*sw/sh;
> +        glViewport(0, stripe / 2, ww, wh - stripe);
> +    } else {
> +        stripe = ww - ww*sh/sw;
> +        glViewport(stripe / 2, 0, ww - stripe, wh);
> +    }
> +
> +    glMatrixMode(GL_PROJECTION);
> +    glLoadIdentity();

It's been a surprisingly long time since I last saw the OpenGL builtin 
matrix stack. :-)

> +
> +    glMatrixMode(GL_MODELVIEW);
> +    glLoadIdentity();
> +
> +    glClearColor(0.0, 0.0, 0.0, 0);

The alpha value is a float, too. So I'd either write 0 for everything 
and let the compiler handle the implicit conversion or (better, in my 
opinion) use 0.f (or 0.0f). Which brings me to that I'd rather not use 
doubles when the function takes floats...

> +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

But you don't need glClearColor() at all, and you don't need this 
glClear() either. The depth test is disabled and the quad fills the 
whole screen, thus you can just leave the depth and color buffer as they 
are.

> +
> +    glGenTextures(1, &tex);
> +    glBindTexture(GL_TEXTURE_2D, tex);
> +    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
> +                 0, GL_BGRA_EXT,GL_UNSIGNED_BYTE,

I feared this extensions might not be widespread enough, but EXT_bgra is 
from 1997 so it should be fine. :-)

> +                 surface_data(scon->surface));
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> +
> +    glEnable(GL_TEXTURE_2D);

Shouldn't you call this before doing the first operation on that target? 
(that is, before the glBindTexture())

> +    glBegin(GL_QUADS);
> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
> +    glEnd();

I've been trained to hate direct mode, but it should be fine for just 
this quad.

First, you may consider using glVertex2f().

Second, as hinted above, I don't like giving ints where floats are 
expected. So I'd like this to be "glTexCoord2f(0.f, 1.f)" etc., or 
(maybe even better because it prevents a discussion about whether to use 
0.f or 0 :-)) just glTexCoord2i() and glVertex2i().

> +
> +    SDL_GL_SwapWindow(scon->real_window);
> +
> +    glDisable(GL_TEXTURE_2D);
> +    glDeleteTextures(1, &tex);
> +}

Also, it hurts to always enable textures, generate one, load it, disable 
textures and delete them just to render a single quad with a texture 
loaded from main memory... (not to mention glViewport(), the matrix 
operations and SDL_GL_MakeCurrent())

Would it be possible to just enable GL_TEXTURE_2D once, store the GL ID 
of the texture in the sdl2_console object and either use glTexImage2D() 
or, technically better, glTexSubImage2D() here?

Using glTexSubImage2D() would give us the advantage of being able to 
perform partial updates on the texture; but it seems to fit pretty bad 
into the existing code. To make it fit, I'd call glTexSubImage2D() 
directly in sdl2_gl_update() and just draw the quad here.

Also, I'd move glViewport() into some function which is called on resize 
(not sdl2_gl_redraw()), and the matrix operations into some 
initialization code. We probably cannot do a whole lot about 
SDL_GL_MakeCurrent(), though, which is too bad (depending on what it 
actually does, the SDL Wiki is not very clear about it...).

Max

Re: [Qemu-devel] [RfC PATCH 0/3] sdl2: add opengl rendering support

[Paolo Bonzini]

On 11/12/2014 16:40, Gerd Hoffmann wrote:
>   Hi,
> 
>> Do you have a case in mind where the device emulation needs to know that?
>> As opposed to "always need opengl" or "never needs opengl".
> 
> virtio-gpu has optional opengl support, and ideally the default behavior
> is 'enable opengl support in case the UI supports it'.

That would break migration depending on backend configuration, which is
not great.

Paolo

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Gerd Hoffmann]

  Hi,

> > +static void sdl2_gl_render_surface(struct sdl2_console *scon)
> > +{
> > +    int gw, gh, ww, wh, stripe;
> > +    float sw, sh;
> > +    GLuint tex;
> > +
> > +    gw = surface_width(scon->surface);
> > +    gh = surface_height(scon->surface);
> > +    SDL_GetWindowSize(scon->real_window, &ww, &wh);
> > +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> > +
> > +    sw = (float)ww/gw;
> > +    sh = (float)wh/gh;
> > +    if (sw < sh) {
> > +        stripe = wh - wh*sw/sh;
> > +        glViewport(0, stripe / 2, ww, wh - stripe);
> > +    } else {
> > +        stripe = ww - ww*sh/sw;
> > +        glViewport(stripe / 2, 0, ww - stripe, wh);
> > +    }
> > +
> > +    glMatrixMode(GL_PROJECTION);
> > +    glLoadIdentity();
> 
> It's been a surprisingly long time since I last saw the OpenGL builtin 
> matrix stack. :-)

--verbose please.

> > +
> > +    glMatrixMode(GL_MODELVIEW);
> > +    glLoadIdentity();
> > +
> > +    glClearColor(0.0, 0.0, 0.0, 0);
> 
> The alpha value is a float, too. So I'd either write 0 for everything 
> and let the compiler handle the implicit conversion or (better, in my 
> opinion) use 0.f (or 0.0f). Which brings me to that I'd rather not use 
> doubles when the function takes floats...

Ok.

> > +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> 
> But you don't need glClearColor() at all, and you don't need this 
> glClear() either. The depth test is disabled and the quad fills the 
> whole screen, thus you can just leave the depth and color buffer as they 
> are.

The quad fills the whole screen only in case host window and guest
screen have the same aspect ratio, otherwise there is padding top/bottom
or left/right so we don't change the guests screen aspect ratio.

> > +
> > +    glGenTextures(1, &tex);
> > +    glBindTexture(GL_TEXTURE_2D, tex);
> > +    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
> > +                 0, GL_BGRA_EXT,GL_UNSIGNED_BYTE,
> 
> I feared this extensions might not be widespread enough, but EXT_bgra is 
> from 1997 so it should be fine. :-)

Note to self:  This also needs to be extended to handle other surface
formats.

> > +                 surface_data(scon->surface));
> > +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> > +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> > +
> > +    glEnable(GL_TEXTURE_2D);
> 
> Shouldn't you call this before doing the first operation on that target? 
> (that is, before the glBindTexture())

Probably ...

> > +    glBegin(GL_QUADS);
> > +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
> > +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
> > +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
> > +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
> > +    glEnd();
> 
> I've been trained to hate direct mode, but it should be fine for just 
> this quad.

--verbose please.  Guess for longer sequences it would be much more
efficient to compile this into a shader program?

> First, you may consider using glVertex2f().
> 
> Second, as hinted above, I don't like giving ints where floats are 
> expected. So I'd like this to be "glTexCoord2f(0.f, 1.f)" etc., or 
> (maybe even better because it prevents a discussion about whether to use 
> 0.f or 0 :-)) just glTexCoord2i() and glVertex2i().

Using gl*i makes sense indeed.

> > +
> > +    SDL_GL_SwapWindow(scon->real_window);
> > +
> > +    glDisable(GL_TEXTURE_2D);
> > +    glDeleteTextures(1, &tex);
> > +}
> 
> Also, it hurts to always enable textures, generate one, load it, disable 
> textures and delete them just to render a single quad with a texture 
> loaded from main memory... (not to mention glViewport(), the matrix 
> operations and SDL_GL_MakeCurrent())
> 
> Would it be possible to just enable GL_TEXTURE_2D once, store the GL ID 
> of the texture in the sdl2_console object and either use glTexImage2D() 
> or, technically better, glTexSubImage2D() here?

Probably.  I don't want tie this into sdl too much though.  My
longer-term plan is to have some generic gl helper functions in the
console code and have all uis with gl support use these.

> Using glTexSubImage2D() would give us the advantage of being able to 
> perform partial updates on the texture; but it seems to fit pretty bad 
> into the existing code. To make it fit, I'd call glTexSubImage2D() 
> directly in sdl2_gl_update() and just draw the quad here.

Yes, that should work.

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Max Reitz]

On 2014-12-12 at 12:04, Gerd Hoffmann wrote:
>    Hi,
>
>>> +static void sdl2_gl_render_surface(struct sdl2_console *scon)
>>> +{
>>> +    int gw, gh, ww, wh, stripe;
>>> +    float sw, sh;
>>> +    GLuint tex;
>>> +
>>> +    gw = surface_width(scon->surface);
>>> +    gh = surface_height(scon->surface);
>>> +    SDL_GetWindowSize(scon->real_window, &ww, &wh);
>>> +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>>> +
>>> +    sw = (float)ww/gw;
>>> +    sh = (float)wh/gh;
>>> +    if (sw < sh) {
>>> +        stripe = wh - wh*sw/sh;
>>> +        glViewport(0, stripe / 2, ww, wh - stripe);
>>> +    } else {
>>> +        stripe = ww - ww*sh/sw;
>>> +        glViewport(stripe / 2, 0, ww - stripe, wh);
>>> +    }
>>> +
>>> +    glMatrixMode(GL_PROJECTION);
>>> +    glLoadIdentity();
>> It's been a surprisingly long time since I last saw the OpenGL builtin
>> matrix stack. :-)
> --verbose please.

Well, I'm mostly used to OpenGL 3/4 Core now where that builtin matrix 
stack does not exist anymore.

>>> +
>>> +    glMatrixMode(GL_MODELVIEW);
>>> +    glLoadIdentity();
>>> +
>>> +    glClearColor(0.0, 0.0, 0.0, 0);
>> The alpha value is a float, too. So I'd either write 0 for everything
>> and let the compiler handle the implicit conversion or (better, in my
>> opinion) use 0.f (or 0.0f). Which brings me to that I'd rather not use
>> doubles when the function takes floats...
> Ok.
>
>>> +    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
>> But you don't need glClearColor() at all, and you don't need this
>> glClear() either. The depth test is disabled and the quad fills the
>> whole screen, thus you can just leave the depth and color buffer as they
>> are.
> The quad fills the whole screen only in case host window and guest
> screen have the same aspect ratio, otherwise there is padding top/bottom
> or left/right so we don't change the guests screen aspect ratio.

Right; additionally, I thought glClear() will be limited by the 
viewport. It doesn't, it is indeed necessary, yes. (Well, you could drop 
the GL_DEPTH_BUFFER_BIT, but if you're already clearing the color buffer 
it shouldn't really matter)

>>> +
>>> +    glGenTextures(1, &tex);
>>> +    glBindTexture(GL_TEXTURE_2D, tex);
>>> +    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, gw, gh,
>>> +                 0, GL_BGRA_EXT,GL_UNSIGNED_BYTE,
>> I feared this extensions might not be widespread enough, but EXT_bgra is
>> from 1997 so it should be fine. :-)
> Note to self:  This also needs to be extended to handle other surface
> formats.
>
>>> +                 surface_data(scon->surface));
>>> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>>> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>>> +
>>> +    glEnable(GL_TEXTURE_2D);
>> Shouldn't you call this before doing the first operation on that target?
>> (that is, before the glBindTexture())
> Probably ...
>
>>> +    glBegin(GL_QUADS);
>>> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
>>> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
>>> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
>>> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
>>> +    glEnd();
>> I've been trained to hate direct mode, but it should be fine for just
>> this quad.
> --verbose please.  Guess for longer sequences it would be much more
> efficient to compile this into a shader program?

Well, again, I'm used to OpenGL 3/4 Core now which doesn't have the 
immediate mode any more. Now, there are vertex arrays which are just 
arrays of the vertex data which are transferred to the GPU and then stay 
there.

For OpenGL 2 at least, there is something which is called vertex arrays 
as well which works nearly the same way. The main difference is that 
here there are not only vertex arrays, but also texture coordinate 
arrays, normal arrays and color arrays (whereas in 3/4 Core there is 
only opaque vertex data, which OpenGL does not care about whether that's 
a position or a texture coordinate or whatever).

So, I'm not sure whether these exist for OpenGL 1 as well... There are 
display lists, but they were just a dead end. So, immediate mode is dead 
and buried today, but it shouldn't be too bad here and maybe for some 
reason there are people which want to use qemu with OpenGL acceleration 
on a pre OpenGL 2 machine.

>> First, you may consider using glVertex2f().
>>
>> Second, as hinted above, I don't like giving ints where floats are
>> expected. So I'd like this to be "glTexCoord2f(0.f, 1.f)" etc., or
>> (maybe even better because it prevents a discussion about whether to use
>> 0.f or 0 :-)) just glTexCoord2i() and glVertex2i().
> Using gl*i makes sense indeed.
>
>>> +
>>> +    SDL_GL_SwapWindow(scon->real_window);
>>> +
>>> +    glDisable(GL_TEXTURE_2D);
>>> +    glDeleteTextures(1, &tex);
>>> +}
>> Also, it hurts to always enable textures, generate one, load it, disable
>> textures and delete them just to render a single quad with a texture
>> loaded from main memory... (not to mention glViewport(), the matrix
>> operations and SDL_GL_MakeCurrent())
>>
>> Would it be possible to just enable GL_TEXTURE_2D once, store the GL ID
>> of the texture in the sdl2_console object and either use glTexImage2D()
>> or, technically better, glTexSubImage2D() here?
> Probably.  I don't want tie this into sdl too much though.  My
> longer-term plan is to have some generic gl helper functions in the
> console code and have all uis with gl support use these.

Well, then you could create an opaque gl_state object or something which 
must be passed by all UI implementations which want to use OpenGL.

Max

>> Using glTexSubImage2D() would give us the advantage of being able to
>> perform partial updates on the texture; but it seems to fit pretty bad
>> into the existing code. To make it fit, I'd call glTexSubImage2D()
>> directly in sdl2_gl_update() and just draw the quad here.
> Yes, that should work.
>
> cheers,
>    Gerd

Re: [Qemu-devel] [RfC PATCH 1/3] configure: opengl overhaul

[Michael Walle]

Am 2014-12-11 12:05, schrieb Gerd Hoffmann:
> 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.

I'm ok with that, although it is a revert of 
b1e5fff4afd0c47148b0d9f9341887ae2c3eb9af. Maybe the tmu2 will be 
converted to SDL, too.

> 
> 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 ++--
>  4 files changed, 21 insertions(+), 26 deletions(-)
> 
> diff --git a/configure b/configure
> index 47048f0..175dea2 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, used by 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;


-- 
michael

Re: [Qemu-devel] [RfC PATCH 1/3] configure: opengl overhaul

[Gerd Hoffmann]

On Mo, 2014-12-15 at 17:46 +0100, Michael Walle wrote:
> Am 2014-12-11 12:05, schrieb Gerd Hoffmann:
> > 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.
> 
> I'm ok with that, although it is a revert of 
> b1e5fff4afd0c47148b0d9f9341887ae2c3eb9af. Maybe the tmu2 will be 
> converted to SDL, too.

Not directly to SDL, there will be a interface layer inbetween so things
don't depend on a single UI.

But, yes, the plan is to have interfaces for device emulations to get a
opengl context for rendering.  That isn't yet in the patch series
though.

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Gerd Hoffmann]

  Hi,

> >>> +    glBegin(GL_QUADS);
> >>> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
> >>> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
> >>> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
> >>> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
> >>> +    glEnd();
> >> I've been trained to hate direct mode, but it should be fine for just
> >> this quad.
> > --verbose please.  Guess for longer sequences it would be much more
> > efficient to compile this into a shader program?
> 
> Well, again, I'm used to OpenGL 3/4 Core now which doesn't have the 
> immediate mode any more. [ ... ]

> [ ... ] and maybe for some 
> reason there are people which want to use qemu with OpenGL acceleration 
> on a pre OpenGL 2 machine.

For virtio-gpu we'll need OPENGL 3 anyway, so I don't feel like caring
too much about old opengl versions.  How would the opengl 3/4 version of
the above would look like?

> >> Using glTexSubImage2D() would give us the advantage of being able to
> >> perform partial updates on the texture; but it seems to fit pretty bad
> >> into the existing code. To make it fit, I'd call glTexSubImage2D()
> >> directly in sdl2_gl_update() and just draw the quad here.
> > Yes, that should work.

Done, also factoring this into helper functions so gtk can use this too
some day, new series sent out, please have a look.

thanks,
  Gerd

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Gerd Hoffmann]

  Hi,

> How would the opengl 3/4 version of
> the above would look like?

Ok, scratch that one.  Looks like this is a seriously non-trivial update
and my rusty opengl knowledge needs a major update for modern-style
opengl ...

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Paolo Bonzini]

On 15/01/2015 12:15, Gerd Hoffmann wrote:
>> How would the opengl 3/4 version of
>> > the above would look like?
> Ok, scratch that one.  Looks like this is a seriously non-trivial update
> and my rusty opengl knowledge needs a major update for modern-style
> opengl ...

Unless we want to support OpenGLES, which doesn't have glBegin/glEnd
(you need to use vertex buffers IIRC, but I last played with OpenGL in
2008...), the old style is still fine.  Even the latest OpenGL additions
in GTK+ use glBegin(GL_QUADS).

Paolo

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Peter Maydell]

On 15 January 2015 at 12:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Unless we want to support OpenGLES, which doesn't have glBegin/glEnd
> (you need to use vertex buffers IIRC, but I last played with OpenGL in
> 2008...), the old style is still fine.  Even the latest OpenGL additions
> in GTK+ use glBegin(GL_QUADS).

OTOH OpenGLES support would be nice, maybe :-)

-- PMM

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Gerd Hoffmann]

On Do, 2015-01-15 at 12:23 +0000, Peter Maydell wrote:
> On 15 January 2015 at 12:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Unless we want to support OpenGLES, which doesn't have glBegin/glEnd
> > (you need to use vertex buffers IIRC, but I last played with OpenGL in
> > 2008...), the old style is still fine.  Even the latest OpenGL additions
> > in GTK+ use glBegin(GL_QUADS).
> 
> OTOH OpenGLES support would be nice, maybe :-)

That's exactly my current plan: go for gles ...

cheers,
  Gerd

Re: [Qemu-devel] [RfC PATCH 2/3] sdl2: add support for display rendering using opengl.

[Max Reitz]

On 2015-01-12 at 07:46, Gerd Hoffmann wrote:
>    Hi,
>
>>>>> +    glBegin(GL_QUADS);
>>>>> +    glTexCoord2f(0, 1);  glVertex3f(-1, -1, 0);
>>>>> +    glTexCoord2f(0, 0);  glVertex3f(-1, 1, 0);
>>>>> +    glTexCoord2f(1, 0);  glVertex3f(1, 1, 0);
>>>>> +    glTexCoord2f(1, 1);  glVertex3f(1, -1, 0);
>>>>> +    glEnd();
>>>> I've been trained to hate direct mode, but it should be fine for just
>>>> this quad.
>>> --verbose please.  Guess for longer sequences it would be much more
>>> efficient to compile this into a shader program?
>> Well, again, I'm used to OpenGL 3/4 Core now which doesn't have the
>> immediate mode any more. [ ... ]
>> [ ... ] and maybe for some
>> reason there are people which want to use qemu with OpenGL acceleration
>> on a pre OpenGL 2 machine.
> For virtio-gpu we'll need OPENGL 3 anyway, so I don't feel like caring
> too much about old opengl versions.  How would the opengl 3/4 version of
> the above would look like?

Regarding OpenGL 3/4 Core, you'd need some shaders and a buffer for the 
vertex data.

So, regarding the vertex buffers, you'd have to create a buffer with 
glGenBuffers(), bind it to GL_ARRAY_BUFFER with glBindBuffer() and fill 
it with glBufferData(). Then, for every (vertex) attribute your vertex 
shader has, you call glVertexAttribPointer() (after 
glEnableVertexArray()) to specify the part of the buffer to be used for 
that attribute. You can receive the ID required for 
glVertexAttribPointer() by using glGetAttribLocation() on the linked 
shader program.

It's probably best to just go for the OpenGL 1 (or 3/4 Compatibility) 
version for now and I'll see to a patch to make it 3/4 Core later on. 
Then we can decide what to do and don't have to decide now.

Max

>>>> Using glTexSubImage2D() would give us the advantage of being able to
>>>> perform partial updates on the texture; but it seems to fit pretty bad
>>>> into the existing code. To make it fit, I'd call glTexSubImage2D()
>>>> directly in sdl2_gl_update() and just draw the quad here.
>>> Yes, that should work.
> Done, also factoring this into helper functions so gtk can use this too
> some day, new series sent out, please have a look.
>
> thanks,
>    Gerd
>
>