Hi On Sat, Feb 13, 2016 at 12:00 PM, Victor Toso <lists@xxxxxxxxxxxxxx> wrote: > Hi, > > On Fri, Feb 05, 2016 at 12:36:52AM +0100, Marc-André Lureau wrote: >> From: Marc-André Lureau <mlureau@xxxxxxxxxx> >> >> 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@xxxxxxxxxx> >> --- >> 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@xxxxxxxxxx> > >> } >> -- >> 2.5.0 >> >> _______________________________________________ >> Spice-devel mailing list >> Spice-devel@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel