Re: [PATCH v4 5/5] gtk: add spice-widget GL scanout support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]