[Spice-devel] [PATCH v4 5/5] gtk: add spice-widget GL scanout support
Marc-André Lureau
marcandre.lureau at gmail.com
Sun Feb 14 23:15:25 UTC 2016
Hi
On Sat, Feb 13, 2016 at 12:00 PM, Victor Toso <lists at victortoso.com> wrote:
> Hi,
>
> On Fri, Feb 05, 2016 at 12:36:52AM +0100, Marc-André Lureau wrote:
>> From: Marc-André Lureau <mlureau at redhat.com>
>>
>> Hook to spice-glib events to show the GL scanout.
>>
>> The opengl context is created with egl, and is currently
>> x11-only (supporting wayland with bare-egl doesn't seem trivial).
>>
>> Using GtkGLArea is left for a future series, since SpiceDisplay widget
>> is a GtkDrawingArea and can't be replaced without breaking
>> ABI. Furthermore, GtkGLArea won't work on non-egl contexts, so this
>> approach is necessary on gtk+ < 3.16 or X11 (because gdk/x11 uses glx).
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
>> ---
>> src/Makefile.am | 9 +
>> src/spice-widget-egl.c | 576 ++++++++++++++++++++++++++++++++++++++++++++++++
>> src/spice-widget-priv.h | 30 +++
>> src/spice-widget.c | 151 +++++++++++--
>> 4 files changed, 746 insertions(+), 20 deletions(-)
>> create mode 100644 src/spice-widget-egl.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 37b89fe..68884e6 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -122,6 +122,7 @@ SPICE_GTK_LIBADD_COMMON = \
>> libspice-client-glib-2.0.la \
>> $(GTK_LIBS) \
>> $(CAIRO_LIBS) \
>> + $(EPOXY_LIBS) \
>> $(LIBM) \
>> $(NULL)
>>
>> @@ -160,17 +161,25 @@ SPICE_GTK_SOURCES_COMMON += \
>> endif
>>
>> if WITH_GTK
>> +if WITH_EPOXY
>> +SPICE_GTK_SOURCES_COMMON += \
>> + spice-widget-egl.c \
>> + $(NULL)
>> +endif
>> +
>> if HAVE_GTK_2
>> libspice_client_gtk_2_0_la_DEPEDENCIES = $(GTK_SYMBOLS_FILE)
>> libspice_client_gtk_2_0_la_LDFLAGS = $(SPICE_GTK_LDFLAGS_COMMON)
>> libspice_client_gtk_2_0_la_LIBADD = $(SPICE_GTK_LIBADD_COMMON)
>> libspice_client_gtk_2_0_la_SOURCES = $(SPICE_GTK_SOURCES_COMMON)
>> +libspice_client_gtk_2_0_la_CFLAGS = $(EPOXY_CFLAGS)
>> nodist_libspice_client_gtk_2_0_la_SOURCES = $(nodist_SPICE_GTK_SOURCES_COMMON)
>> else
>> libspice_client_gtk_3_0_la_DEPEDENCIES = $(GTK_SYMBOLS_FILE)
>> libspice_client_gtk_3_0_la_LDFLAGS = $(SPICE_GTK_LDFLAGS_COMMON)
>> libspice_client_gtk_3_0_la_LIBADD = $(SPICE_GTK_LIBADD_COMMON)
>> libspice_client_gtk_3_0_la_SOURCES = $(SPICE_GTK_SOURCES_COMMON)
>> +libspice_client_gtk_3_0_la_CFLAGS = $(EPOXY_CFLAGS)
>> nodist_libspice_client_gtk_3_0_la_SOURCES = $(nodist_SPICE_GTK_SOURCES_COMMON)
>> endif
>>
>> diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
>> new file mode 100644
>> index 0000000..b7fd3d9
>> --- /dev/null
>> +++ b/src/spice-widget-egl.c
>> @@ -0,0 +1,576 @@
>> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
>> +/*
>> + Copyright (C) 2014-2015 Red Hat, Inc.
>> +
>> + This library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + This library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> +*/
>> +#include "config.h"
>> +
>> +#include <math.h>
>> +
>> +#define EGL_EGLEXT_PROTOTYPES
>> +#define GL_GLEXT_PROTOTYPES
>> +
>> +#include "spice-widget.h"
>> +#include "spice-widget-priv.h"
>> +#include "spice-gtk-session-priv.h"
>> +#include <libdrm/drm_fourcc.h>
>> +
>> +#include <gdk/gdkx.h>
>> +
>> +static const char *spice_egl_vertex_src = \
>> +" \
>> + #version 130\n \
>> + \
>> + in vec4 position; \
>> + in vec2 texcoords; \
>> + out vec2 tcoords; \
>> + uniform mat4 mproj; \
>> + \
>> + void main() \
>> + { \
>> + tcoords = texcoords; \
>> + gl_Position = mproj * position; \
>> + } \
>> +";
>> +
>> +static const char *spice_egl_fragment_src = \
>> +" \
>> + #version 130\n \
>> + \
>> + in vec2 tcoords; \
>> + out vec4 fragmentColor; \
>> + uniform sampler2D samp; \
>> + \
>> + void main() \
>> + { \
>> + fragmentColor = texture2D(samp, tcoords); \
>> + } \
>> +";
>> +
>> +static void apply_ortho(guint mproj, float left, float right,
>> + float bottom, float top, float near, float far)
>> +
>> +{
>> + float a = 2.0f / (right - left);
>> + float b = 2.0f / (top - bottom);
>> + float c = -2.0f / (far - near);
>> +
>> + float tx = - (right + left) / (right - left);
>> + float ty = - (top + bottom) / (top - bottom);
>> + float tz = - (far + near) / (far - near);
>> +
>> + float ortho[16] = {
>> + a, 0, 0, 0,
>> + 0, b, 0, 0,
>> + 0, 0, c, 0,
>> + tx, ty, tz, 1
>> + };
>> +
>> + glUniformMatrix4fv(mproj, 1, GL_FALSE, &ortho[0]);
>> +}
>> +
>> +static gboolean spice_egl_init_shaders(SpiceDisplay *display, GError **err)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + GLuint fs = 0, vs = 0, buf;
>> + GLint status, tex_loc, prog;
>> + gboolean success = false;
>> + gchar log[1000] = { 0, };
>> + GLsizei len;
>> +
>> + glGetIntegerv(GL_CURRENT_PROGRAM, &prog);
>> +
>> + fs = glCreateShader(GL_FRAGMENT_SHADER);
>> + glShaderSource(fs, 1, (const char **)&spice_egl_fragment_src, NULL);
>> + glCompileShader(fs);
>> + glGetShaderiv(fs, GL_COMPILE_STATUS, &status);
>> + if (!status) {
>> + glGetShaderInfoLog(fs, sizeof(log), &len, log);
>> + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "failed to compile fragment shader: %s", log);
>> + goto end;
>> + }
>> +
>> + vs = glCreateShader(GL_VERTEX_SHADER);
>> + glShaderSource(vs, 1, (const char **)&spice_egl_vertex_src, NULL);
>> + glCompileShader(vs);
>> + glGetShaderiv(vs, GL_COMPILE_STATUS, &status);
>> + if (!status) {
>> + glGetShaderInfoLog(vs, sizeof(log), &len, log);
>> + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "failed to compile vertex shader: %s", log);
>> + goto end;
>> + }
>> +
>> + d->egl.prog = glCreateProgram();
>> + glAttachShader(d->egl.prog, fs);
>> + glAttachShader(d->egl.prog, vs);
>> + glLinkProgram(d->egl.prog);
>> + glGetProgramiv(d->egl.prog, GL_LINK_STATUS, &status);
>> + if (!status) {
>> + glGetProgramInfoLog(d->egl.prog, 1000, &len, log);
>> + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "error linking shaders: %s", log);
>> + goto end;
>> + }
>
> In case of error in glGetProgramiv don't you need to detach fs and vs
> before deleting it?
>
No, they will be flagged for deletion and actually deleted when the
program is deleted.
>> +
>> + glUseProgram(d->egl.prog);
>> + glDetachShader(d->egl.prog, fs);
>> + glDetachShader(d->egl.prog, vs);
>> +
>> + d->egl.attr_pos = glGetAttribLocation(d->egl.prog, "position");
>> + g_assert(d->egl.attr_pos != -1);
>> + d->egl.attr_tex = glGetAttribLocation(d->egl.prog, "texcoords");
>> + g_assert(d->egl.attr_tex != -1);
>> + tex_loc = glGetUniformLocation(d->egl.prog, "samp");
>> + g_assert(tex_loc != -1);
>> + d->egl.mproj = glGetUniformLocation(d->egl.prog, "mproj");
>> + g_assert(d->egl.mproj != -1);
>> +
>> + glUniform1i(tex_loc, 0);
>> +
>> + /* we only use one VAO, so we always keep it bound */
>> + glGenVertexArrays(1, &buf);
>> + glBindVertexArray(buf);
>> +
>> + glGenBuffers(1, &buf);
>> + glBindBuffer(GL_ARRAY_BUFFER, buf);
>> + glBufferData(GL_ARRAY_BUFFER,
>> + (sizeof(GLfloat) * 4 * 4) +
>> + (sizeof(GLfloat) * 4 * 2),
>
> Maybe a macro for the size of this two values could help code
> readability (here and in the draw_rect_from_arrays below)
>
ok
>> + NULL,
>> + GL_STATIC_DRAW);
>> + d->egl.vbuf_id = buf;
>> +
>> + glGenTextures(1, &d->egl.tex_id);
>> + glGenTextures(1, &d->egl.tex_pointer_id);
>> +
>> + success = true;
>
> gbooleans are upper case
well, we have been using c99 bool for a while, but ok ;)
>> +
>> +end:
>> + if (fs) {
>> + glDeleteShader(fs);
>> + }
>> + if (vs) {
>> + glDeleteShader(vs);
>> + }
>> +
>> + glUseProgram(prog);
>> + return success;
>> +}
>> +
>> +G_GNUC_INTERNAL
>> +gboolean spice_egl_init(SpiceDisplay *display, GError **err)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + static const EGLint conf_att[] = {
>> + EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
>> + EGL_RENDERABLE_TYPE, EGL_OPENGL_BIT,
>> + EGL_RED_SIZE, 8,
>> + EGL_GREEN_SIZE, 8,
>> + EGL_BLUE_SIZE, 8,
>> + EGL_ALPHA_SIZE, 0,
>> + EGL_NONE,
>> + };
>> + static const EGLint ctx_att[] = {
>> +#ifdef EGL_CONTEXT_MAJOR_VERSION
>> + EGL_CONTEXT_MAJOR_VERSION, 3,
>> +#else
>> + EGL_CONTEXT_CLIENT_VERSION, 3,
>> +#endif
>> + EGL_NONE
>> + };
>> + EGLBoolean b;
>> + EGLenum api;
>> + EGLint major, minor, n;
>> + EGLNativeDisplayType dpy = 0;
>> + GdkDisplay *gdk_dpy = gdk_display_get_default();
>> +
>> +#ifdef GDK_WINDOWING_X11
>> + if (GDK_IS_X11_DISPLAY(gdk_dpy)) {
>> + dpy = (EGLNativeDisplayType)gdk_x11_display_get_xdisplay(gdk_dpy);
>> + }
>> +#endif
>> +
>> + d->egl.display = eglGetDisplay(dpy);
>> + if (d->egl.display == EGL_NO_DISPLAY) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "failed to get EGL display");
>> + return false;
>
> ditto (gboolean)
>
>> + }
>> +
>> + if (!eglInitialize(d->egl.display, &major, &minor)) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "failed to init EGL display");
>> + return false;
>> + }
>> +
>> + SPICE_DEBUG("EGL major/minor: %d.%d\n", major, minor);
>> + SPICE_DEBUG("EGL version: %s\n",
>> + eglQueryString(d->egl.display, EGL_VERSION));
>> + SPICE_DEBUG("EGL vendor: %s\n",
>> + eglQueryString(d->egl.display, EGL_VENDOR));
>> + SPICE_DEBUG("EGL extensions: %s\n",
>> + eglQueryString(d->egl.display, EGL_EXTENSIONS));
>> +
>> + api = EGL_OPENGL_API;
>> + b = eglBindAPI(api);
>
> why do you need the variable api?
nothing ;)
>
>> + if (!b) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "cannot bind OpenGLES API");
>> + return false;
>> + }
>> +
>> + b = eglChooseConfig(d->egl.display, conf_att, &d->egl.conf,
>> + 1, &n);
>> +
>> + if (!b || n != 1) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "cannot find suitable EGL config");
>> + return false;
>> + }
>> +
>> + d->egl.ctx = eglCreateContext(d->egl.display,
>> + d->egl.conf,
>> + EGL_NO_CONTEXT,
>> + ctx_att);
>> + if (!d->egl.ctx) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "cannot create EGL context");
>> + return false;
>> + }
>> +
>> + eglMakeCurrent(d->egl.display, EGL_NO_SURFACE, EGL_NO_SURFACE,
>> + d->egl.ctx);
>> +
>> + return spice_egl_init_shaders(display, err);
>> +}
>> +
>> +static gboolean spice_widget_init_egl_win(SpiceDisplay *display, GdkWindow *win,
>> + GError **err)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + EGLNativeWindowType native = 0;
>> + EGLBoolean b;
>> +
>> + if (d->egl.surface)
>> + return true;
>> +
>> +#ifdef GDK_WINDOWING_X11
>> + if (GDK_IS_X11_WINDOW(win)) {
>> + native = (EGLNativeWindowType)gdk_x11_window_get_xid(win);
>> + }
>> +#endif
>> +
>> + if (!native) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "this platform isn't supported");
>> + return false;
>> + }
>> +
>> + d->egl.surface = eglCreateWindowSurface(d->egl.display,
>> + d->egl.conf,
>> + native, NULL);
>> +
>> + if (!d->egl.surface) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "failed to init egl surface");
>> + return false;
>> + }
>> +
>> + b = eglMakeCurrent(d->egl.display,
>> + d->egl.surface,
>> + d->egl.surface,
>> + d->egl.ctx);
>> + if (!b) {
>> + g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> + "failed to activate context");
>> + return false;
>> + }
>> +
>> + return true;
>
> ditto (gboolean)
>
>> +}
>> +
>> +G_GNUC_INTERNAL
>> +gboolean spice_egl_realize_display(SpiceDisplay *display, GdkWindow *win, GError **err)
>> +{
>> + SPICE_DEBUG("egl realize");
>> + if (!spice_widget_init_egl_win(display, win, err))
>> + return false;
>> +
>> + spice_egl_resize_display(display, gdk_window_get_width(win),
>> + gdk_window_get_height(win));
>> +
>> + return true;
>
> ditto (gboolean)
>
>> +}
>> +
>> +G_GNUC_INTERNAL
>> +void spice_egl_unrealize_display(SpiceDisplay *display)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> +
>> + SPICE_DEBUG("egl unrealize %p", d->egl.surface);
>> +
>> + if (d->egl.image != NULL)
>> + eglDestroyImageKHR(d->egl.display, d->egl.image);
>
> I think to set it to NULL after destroying it
ok
>
>> +
>> + if (d->egl.tex_id)
>> + glDeleteTextures(1, &d->egl.tex_id);
>> +
>> + if (d->egl.tex_pointer_id)
>> + glDeleteTextures(1, &d->egl.tex_pointer_id);
>> +
>> + if (d->egl.surface != EGL_NO_SURFACE) {
>> + eglDestroySurface(d->egl.display, d->egl.surface);
>> + d->egl.surface = EGL_NO_SURFACE;
>> + }
>> + if (d->egl.vbuf_id)
>> + glDeleteBuffers(1, &d->egl.vbuf_id);
>> +
>> + if (d->egl.prog)
>> + glDeleteProgram(d->egl.prog);
>> +
>> + if (d->egl.ctx)
>> + eglDestroyContext(d->egl.display, d->egl.ctx);
>> +
>> + eglMakeCurrent(d->egl.display, EGL_NO_SURFACE, EGL_NO_SURFACE,
>> + EGL_NO_CONTEXT);
>> +}
>> +
>> +G_GNUC_INTERNAL
>> +void spice_egl_resize_display(SpiceDisplay *display, int w, int h)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + int prog;
>> +
>> + glGetIntegerv(GL_CURRENT_PROGRAM, &prog);
>> +
>> + glUseProgram(d->egl.prog);
>> + apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1);
>> + glViewport(0, 0, w, h);
>> +
>> + if (d->egl.image)
>> + spice_egl_update_display(display);
>> +
>> + glUseProgram(prog);
>> +}
>> +
>> +static void
>> +draw_rect_from_arrays(SpiceDisplay *display, const void *verts, const void *tex)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> +
>> + glBindBuffer(GL_ARRAY_BUFFER, d->egl.vbuf_id);
>> +
>> + if (verts) {
>> + glEnableVertexAttribArray(d->egl.attr_pos);
>> + glBufferSubData(GL_ARRAY_BUFFER,
>> + 0,
>> + sizeof(GLfloat) * 4 * 4,
>> + verts);
>> + glVertexAttribPointer(d->egl.attr_pos, 4, GL_FLOAT,
>> + GL_FALSE, 0, 0);
>> + }
>> + if (tex) {
>> + glEnableVertexAttribArray(d->egl.attr_tex);
>> + glBufferSubData(GL_ARRAY_BUFFER,
>> + sizeof(GLfloat) * 4 * 4,
>> + sizeof(GLfloat) * 4 * 2,
>> + tex);
>> + glVertexAttribPointer(d->egl.attr_tex, 2, GL_FLOAT,
>> + GL_FALSE, 0,
>> + (void *)(sizeof(GLfloat) * 4 * 4));
>> + }
>> +
>> + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>> + if (verts)
>> + glDisableVertexAttribArray(d->egl.attr_pos);
>> + if (tex)
>> + glDisableVertexAttribArray(d->egl.attr_tex);
>> +
>> + glBindBuffer(GL_ARRAY_BUFFER, 0);
>> +}
>> +
>> +static GLvoid
>> +client_draw_rect_tex(SpiceDisplay *display,
>> + float x, float y, float w, float h,
>> + float tx, float ty, float tw, float th)
>> +{
>> + float verts[4][4];
>> + float tex[4][2];
>> +
>> + verts[0][0] = x;
>> + verts[0][1] = y;
>> + verts[0][2] = 0.0;
>> + verts[0][3] = 1.0;
>> + tex[0][0] = tx;
>> + tex[0][1] = ty;
>> + verts[1][0] = x + w;
>> + verts[1][1] = y;
>> + verts[1][2] = 0.0;
>> + verts[1][3] = 1.0;
>> + tex[1][0] = tx + tw;
>> + tex[1][1] = ty;
>> + verts[2][0] = x;
>> + verts[2][1] = y + h;
>> + verts[2][2] = 0.0;
>> + verts[2][3] = 1.0;
>> + tex[2][0] = tx;
>> + tex[2][1] = ty + th;
>> + verts[3][0] = x + w;
>> + verts[3][1] = y + h;
>> + verts[3][2] = 0.0;
>> + verts[3][3] = 1.0;
>> + tex[3][0] = tx + tw;
>> + tex[3][1] = ty + th;
>> +
>> + draw_rect_from_arrays(display, verts, tex);
>> +}
>> +
>> +G_GNUC_INTERNAL
>> +void spice_egl_cursor_set(SpiceDisplay *display)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + GdkPixbuf *image = d->mouse_pixbuf;
>> +
>> + if (image == NULL)
>> + return;
>> +
>> + int width = gdk_pixbuf_get_width(image);
>> + int height = gdk_pixbuf_get_height(image);
>> +
>> + glBindTexture(GL_TEXTURE_2D, d->egl.tex_pointer_id);
>> + glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>> + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA,
>> + width, height, 0,
>> + GL_RGBA, GL_UNSIGNED_BYTE,
>> + gdk_pixbuf_read_pixels(image));
>> + glBindTexture(GL_TEXTURE_2D, 0);
>> +}
>> +
>> +G_GNUC_INTERNAL
>> +void spice_egl_update_display(SpiceDisplay *display)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + double s;
>> + int x, y, w, h;
>> + gdouble tx, ty, tw, th;
>> + int prog;
>> +
>> + g_return_if_fail(d->egl.image != NULL);
>> + spice_display_get_scaling(display, &s, &x, &y, &w, &h);
>> +
>> + glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
>> + glClear(GL_COLOR_BUFFER_BIT);
>> +
>> + tx = ((float)d->area.x / (float)d->egl.scanout.width);
>> + ty = ((float)d->area.y / (float)d->egl.scanout.height);
>> + tw = ((float)d->area.width / (float)d->egl.scanout.width);
>> + th = ((float)d->area.height / (float)d->egl.scanout.height);
>> + ty += 1 - th;
>
> err, I did not get the 1 here. Why not just `ty -= th;` ?
> Could you clarify with a comment?
That looks wrong indeed, I think what is meant is ty = 1 - (th + ty).
So that ty points to the lower area of the gl texture (bottom-up
direction)
> y-axis is inverted but seems that you are moving it by 1 there and
> in the chunk bellow
for the chunk below, it inverts the texture coordiantes to be
downward, so that 0 is actually the top, 1. (y: 0.1 -> 0.9, h: 0.1 ->
-0.1)
This might need further tweaking as the code as not been much
exerciced yet, I'll do more tests around that.
>> + if (!d->egl.scanout.y0top) {
>> + ty = 1 - ty;
>> + th = -1 * th;
>> + }
>> + SPICE_DEBUG("update %f +%d+%d %dx%d +%f+%f %fx%f", s, x, y, w, h,
>> + tx, ty, tw, th);
>> +
>> + glBindTexture(GL_TEXTURE_2D, d->egl.tex_id);
>> + glDisable(GL_BLEND);
>> + glGetIntegerv(GL_CURRENT_PROGRAM, &prog);
>> + glUseProgram(d->egl.prog);
>> + client_draw_rect_tex(display, x, y, w, h,
>> + tx, ty, tw, th);
>> +
>
> Seems that series to eanble mouse as server-side will be handy here...
>
>> + if (d->mouse_mode == SPICE_MOUSE_MODE_SERVER &&
>> + d->mouse_guest_x != -1 && d->mouse_guest_y != -1 &&
>> + !d->show_cursor &&
>> + spice_gtk_session_get_pointer_grabbed(d->gtk_session) &&
>> + d->mouse_pixbuf) {
>
> Could you make explicit the check against NULL the d->mouse_pixbuf ?
sure
>
>> + GdkPixbuf *image = d->mouse_pixbuf;
>> + int width = gdk_pixbuf_get_width(image);
>> + int height = gdk_pixbuf_get_height(image);
>> +
>> + glBindTexture(GL_TEXTURE_2D, d->egl.tex_pointer_id);
>> + glEnable(GL_BLEND);
>> + glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
>> + client_draw_rect_tex(display,
>> + x + (d->mouse_guest_x - d->mouse_hotspot.x) * s,
>> + y + h - (d->mouse_guest_y - d->mouse_hotspot.y) * s,
>> + width, -height,
>> + 0, 0, 1, 1);
>> + }
>> +
>> + if (d->egl.surface != EGL_NO_SURFACE)
>> + eglSwapBuffers(d->egl.display, d->egl.surface);
>
> Why can you have EGL_NO_SURFACE here?
I think we could get some update-display events before the widget is
realized. I don't see why the check is not done earlier though. Moving
it earlier.
>
>> +
>> + glUseProgram(prog);
>> +}
>> +
>> +
>> +G_GNUC_INTERNAL
>> +gboolean spice_egl_update_scanout(SpiceDisplay *display,
>> + const SpiceGlScanout *scanout,
>> + GError **err)
>> +{
>> + SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>> + EGLint attrs[13];
>> + guint32 format;
>> +
>> + g_return_val_if_fail(scanout != NULL, false);
>> + format = scanout->format;
>> +
>> + if (d->egl.image != NULL) {
>> + eglDestroyImageKHR(d->egl.display, d->egl.image);
>> + d->egl.image = NULL;
>> + }
>> +
>> + if (scanout->fd == -1)
>> + return true;
>
> ditto (gboolean)
>
>> +
>> + attrs[0] = EGL_DMA_BUF_PLANE0_FD_EXT;
>> + attrs[1] = scanout->fd;
>> + attrs[2] = EGL_DMA_BUF_PLANE0_PITCH_EXT;
>> + attrs[3] = scanout->stride;
>> + attrs[4] = EGL_DMA_BUF_PLANE0_OFFSET_EXT;
>> + attrs[5] = 0;
>> + attrs[6] = EGL_WIDTH;
>> + attrs[7] = scanout->width;
>> + attrs[8] = EGL_HEIGHT;
>> + attrs[9] = scanout->height;
>> + attrs[10] = EGL_LINUX_DRM_FOURCC_EXT;
>> + attrs[11] = format;
>> + attrs[12] = EGL_NONE;
>
> Could you point me out the documentation related to this attributes if
> available?
They are described as part of this egl extension:
https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
>
>> + SPICE_DEBUG("fd:%d stride:%d y0:%d %dx%d format:0x%x (%c%c%c%c)",
>> + scanout->fd, scanout->stride, scanout->y0top,
>> + scanout->width, scanout->height, format,
>> + format & 0xff, (format >> 8) & 0xff, (format >> 16) & 0xff, format >> 24);
>> +
>> + d->egl.image = eglCreateImageKHR(d->egl.display,
>> + EGL_NO_CONTEXT,
>> + EGL_LINUX_DMA_BUF_EXT,
>> + (EGLClientBuffer)NULL,
>> + attrs);
>> +
>> + glBindTexture(GL_TEXTURE_2D, d->egl.tex_id);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>> + glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, (GLeglImageOES)d->egl.image);
>> + d->egl.scanout = *scanout;
>> +
>> + return true;
>> +}
>> diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
>> index 682e889..c708e25 100644
>> --- a/src/spice-widget-priv.h
>> +++ b/src/spice-widget-priv.h
>> @@ -30,6 +30,10 @@
>> #include <windows.h>
>> #endif
>>
>> +#ifdef USE_EPOXY
>> +#include <epoxy/egl.h>
>> +#endif
>> +
>> #include "spice-widget.h"
>> #include "spice-common.h"
>> #include "spice-gtk-session.h"
>> @@ -124,6 +128,22 @@ struct _SpiceDisplayPrivate {
>> int x11_accel_denominator;
>> int x11_threshold;
>> #endif
>> +#ifdef USE_EPOXY
>> + struct {
>> + gboolean enabled;
>> + EGLSurface surface;
>> + EGLDisplay display;
>> + EGLConfig conf;
>> + EGLContext ctx;
>> + gint mproj, attr_pos, attr_tex;
>> + guint vbuf_id;
>> + guint tex_id;
>> + guint tex_pointer_id;
>> + guint prog;
>> + EGLImageKHR image;
>> + SpiceGlScanout scanout;
>> + } egl;
>> +#endif
>> };
>>
>> int spicex_image_create (SpiceDisplay *display);
>> @@ -135,6 +155,16 @@ void spicex_expose_event (SpiceDisplay *display, GdkEventExp
>> #endif
>> gboolean spicex_is_scaled (SpiceDisplay *display);
>> void spice_display_get_scaling (SpiceDisplay *display, double *s, int *x, int *y, int *w, int *h);
>> +gboolean spice_egl_init (SpiceDisplay *display, GError **err);
>> +gboolean spice_egl_realize_display (SpiceDisplay *display, GdkWindow *win,
>> + GError **err);
>> +void spice_egl_unrealize_display (SpiceDisplay *display);
>> +void spice_egl_update_display (SpiceDisplay *display);
>> +void spice_egl_resize_display (SpiceDisplay *display, int w, int h);
>> +gboolean spice_egl_update_scanout (SpiceDisplay *display,
>> + const SpiceGlScanout *scanout,
>> + GError **err);
>> +void spice_egl_cursor_set (SpiceDisplay *display);
>>
>> G_END_DECLS
>>
>> diff --git a/src/spice-widget.c b/src/spice-widget.c
>> index a1a68a6..53d66df 100644
>> --- a/src/spice-widget.c
>> +++ b/src/spice-widget.c
>> @@ -552,6 +552,7 @@ static void spice_display_init(SpiceDisplay *display)
>> GtkWidget *widget = GTK_WIDGET(display);
>> SpiceDisplayPrivate *d;
>> GtkTargetEntry targets = { "text/uri-list", 0, 0 };
>> + G_GNUC_UNUSED GError *err = NULL;
>>
>> d = display->priv = SPICE_DISPLAY_GET_PRIVATE(display);
>>
>> @@ -583,6 +584,13 @@ static void spice_display_init(SpiceDisplay *display)
>>
>> d->mouse_cursor = get_blank_cursor();
>> d->have_mitshm = true;
>> +
>> +#ifdef USE_EPOXY
>> + if (!spice_egl_init(display, &err)) {
>> + g_critical("egl init failed: %s", err->message);
>> + g_clear_error(&err);
>> + }
>> +#endif
>> }
>>
>> static GObject *
>> @@ -1132,6 +1140,20 @@ static gboolean do_color_convert(SpiceDisplay *display, GdkRectangle *r)
>> return true;
>> }
>>
>> +static void set_egl_enabled(SpiceDisplay *display, bool enabled)
>> +{
>> +#ifdef USE_EPOXY
>> + SpiceDisplayPrivate *d = display->priv;
>> +
>> + if (d->egl.enabled != enabled) {
>> + d->egl.enabled = enabled;
>> + /* even though the function is marked as deprecated, it's the
>> + * only way I found to prevent glitches when the window is
>> + * resized. */
>> + gtk_widget_set_double_buffered(GTK_WIDGET(display), !enabled);
>> + }
>> +#endif
>> +}
>>
>> #if GTK_CHECK_VERSION (2, 91, 0)
>> static gboolean draw_event(GtkWidget *widget, cairo_t *cr)
>> @@ -1140,6 +1162,13 @@ static gboolean draw_event(GtkWidget *widget, cairo_t *cr)
>> SpiceDisplayPrivate *d = display->priv;
>> g_return_val_if_fail(d != NULL, false);
>>
>> +#ifdef USE_EPOXY
>> + if (d->egl.enabled) {
>> + spice_egl_update_display(display);
>> + return false;
>
> ditto (gboolean)
>
>> + }
>> +#endif
>> +
>> if (d->mark == 0 || d->data == NULL ||
>> d->area.width == 0 || d->area.height == 0)
>> return false;
>> @@ -1760,6 +1789,10 @@ static void size_allocate(GtkWidget *widget, GtkAllocation *conf, gpointer data)
>> d->ww = conf->width;
>> d->wh = conf->height;
>> recalc_geometry(widget);
>> +#ifdef USE_EPOXY
>> + if (d->egl.enabled)
>> + spice_egl_resize_display(display, conf->width, conf->height);
>> +#endif
>> }
>>
>> d->mx = conf->x;
>> @@ -1786,18 +1819,29 @@ static void realize(GtkWidget *widget)
>> {
>> SpiceDisplay *display = SPICE_DISPLAY(widget);
>> SpiceDisplayPrivate *d = display->priv;
>> + G_GNUC_UNUSED GError *err = NULL;
>>
>> GTK_WIDGET_CLASS(spice_display_parent_class)->realize(widget);
>>
>> d->keycode_map =
>> vnc_display_keymap_gdk2xtkbd_table(gtk_widget_get_window(widget),
>> &d->keycode_maplen);
>> +
>> +#ifdef USE_EPOXY
>> + if (!spice_egl_realize_display(display, gtk_widget_get_window(GTK_WIDGET(display)), &err)) {
>> + g_critical("egl realize failed: %s", err->message);
>> + g_clear_error(&err);
>> + }
>> +#endif
>> update_image(display);
>> }
>>
>> static void unrealize(GtkWidget *widget)
>> {
>> spicex_image_destroy(SPICE_DISPLAY(widget));
>> +#ifdef USE_EPOXY
>> + spice_egl_unrealize_display(SPICE_DISPLAY(widget));
>> +#endif
>>
>> GTK_WIDGET_CLASS(spice_display_parent_class)->unrealize(widget);
>> }
>> @@ -2206,6 +2250,8 @@ static void invalidate(SpiceChannel *channel,
>> .height = h
>> };
>>
>> + set_egl_enabled(display, false);
>> +
>> if (!gtk_widget_get_window(GTK_WIDGET(display)))
>> return;
>>
>> @@ -2269,6 +2315,9 @@ static void cursor_set(SpiceCursorChannel *channel,
>> } else
>> g_warn_if_reached();
>>
>> +#ifdef USE_EPOXY
>> + spice_egl_cursor_set(display);
>> +#endif
>> if (d->show_cursor) {
>> /* unhide */
>> gdk_cursor_unref(d->show_cursor);
>> @@ -2416,6 +2465,38 @@ static void cursor_reset(SpiceCursorChannel *channel, gpointer data)
>> gdk_window_set_cursor(window, NULL);
>> }
>>
>> +#ifdef USE_EPOXY
>> +static void gl_scanout(SpiceDisplay *display)
>> +{
>> + SpiceDisplayPrivate *d = display->priv;
>> + const SpiceGlScanout *scanout;
>> + GError *err = NULL;
>> +
>> + scanout = spice_display_get_gl_scanout(SPICE_DISPLAY_CHANNEL(d->display));
>> + g_return_if_fail(scanout != NULL);
>> +
>> + SPICE_DEBUG("%s: got scanout", __FUNCTION__);
>> + set_egl_enabled(display, true);
>> +
>> + if (!spice_egl_update_scanout(display, scanout, &err)) {
>> + g_critical("update scanout failed: %s", err->message);
>> + g_clear_error(&err);
>> + }
>> +}
>> +
>> +static void gl_draw(SpiceDisplay *display,
>> + guint32 x, guint32 y, guint32 w, guint32 h)
>> +{
>> + SpiceDisplayPrivate *d = display->priv;
>> +
>> + SPICE_DEBUG("%s", __FUNCTION__);
>> + set_egl_enabled(display, true);
>> +
>> + spice_egl_update_display(display);
>> + spice_display_gl_draw_done(SPICE_DISPLAY_CHANNEL(d->display));
>> +}
>> +#endif
>> +
>> static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
>> {
>> SpiceDisplay *display = data;
>> @@ -2451,6 +2532,13 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
>> primary.stride, primary.shmid, primary.data, display);
>> mark(display, primary.marked);
>> }
>> +#ifdef USE_EPOXY
>> + spice_g_signal_connect_object(channel, "notify::gl-scanout",
>> + G_CALLBACK(gl_scanout), display, G_CONNECT_SWAPPED);
>> + spice_g_signal_connect_object(channel, "gl-draw",
>> + G_CALLBACK(gl_draw), display, G_CONNECT_SWAPPED);
>> +#endif
>> +
>> spice_channel_connect(channel);
>> return;
>> }
>> @@ -2634,34 +2722,57 @@ GdkPixbuf *spice_display_get_pixbuf(SpiceDisplay *display)
>> {
>> SpiceDisplayPrivate *d;
>> GdkPixbuf *pixbuf;
>> - int x, y;
>> - guchar *src, *data, *dest;
>> + guchar *data;
>>
>> g_return_val_if_fail(SPICE_IS_DISPLAY(display), NULL);
>>
>> d = display->priv;
>>
>> g_return_val_if_fail(d != NULL, NULL);
>> - /* TODO: ensure d->data has been exposed? */
>> - g_return_val_if_fail(d->data != NULL, NULL);
>> -
>> - data = g_malloc0(d->area.width * d->area.height * 3);
>> - src = d->data;
>> - dest = data;
>> -
>> - src += d->area.y * d->stride + d->area.x * 4;
>> - for (y = 0; y < d->area.height; ++y) {
>> - for (x = 0; x < d->area.width; ++x) {
>> - dest[0] = src[x * 4 + 2];
>> - dest[1] = src[x * 4 + 1];
>> - dest[2] = src[x * 4 + 0];
>> - dest += 3;
>> + g_return_val_if_fail(d->display != NULL, NULL);
>> +
>> +#ifdef USE_EPOXY
>> + if (d->egl.enabled) {
>> + GdkPixbuf *tmp;
>> +
>> + data = g_malloc0(d->area.width * d->area.height * 4);
>> + glReadBuffer(GL_FRONT);
>> + glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
>> + glReadPixels(0, 0, d->area.width, d->area.height,
>> + GL_RGBA, GL_UNSIGNED_BYTE, data);
>> + tmp = gdk_pixbuf_new_from_data(data, GDK_COLORSPACE_RGB, true,
>> + 8, d->area.width, d->area.height,
>> + d->area.width * 4,
>> + (GdkPixbufDestroyNotify)g_free, NULL);
>> + pixbuf = gdk_pixbuf_flip(tmp, false);
>> + g_object_unref(tmp);
>> + } else
>> +#endif
>> + {
>> + guchar *src, *dest;
>> + int x, y;
>> +
>> + /* TODO: ensure d->data has been exposed? */
>> + g_return_val_if_fail(d->data != NULL, NULL);
>> + data = g_malloc0(d->area.width * d->area.height * 3);
>> + src = d->data;
>> + dest = data;
>> +
>> + src += d->area.y * d->stride + d->area.x * 4;
>> + for (y = 0; y < d->area.height; ++y) {
>> + for (x = 0; x < d->area.width; ++x) {
>> + dest[0] = src[x * 4 + 2];
>> + dest[1] = src[x * 4 + 1];
>> + dest[2] = src[x * 4 + 0];
>> + dest += 3;
>> + }
>> + src += d->stride;
>> }
>> - src += d->stride;
>> + pixbuf = gdk_pixbuf_new_from_data(data, GDK_COLORSPACE_RGB, false,
>> + 8, d->area.width, d->area.height,
>> + d->area.width * 3,
>> + (GdkPixbufDestroyNotify)g_free, NULL);
>> }
>>
>> - pixbuf = gdk_pixbuf_new_from_data(data, GDK_COLORSPACE_RGB, false,
>> - 8, d->area.width, d->area.height, d->area.width * 3,
>> - (GdkPixbufDestroyNotify)g_free, NULL);
>> return pixbuf;
>
> Looks good here
>
> So, this one had only minor comments on it as well. I'm not
> knowledgeable with opengl / egl so I can't help much there.
> Feel free to address the ones you agree on and push the series.
>
> Virgl integration is very much welcome, thank you for your work on
> this.
>
thanks for reviewing, I'll resend later once I checked the texture
coordinates of the update
.
> Acked-by: Victor Toso <victortoso at redhat.com>
>
>> }
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
More information about the Spice-devel
mailing list