[waffle] [PATCH 10/10] waffle: add 'null' platform
Emil Velikov
emil.l.velikov at gmail.com
Fri Apr 10 10:22:04 PDT 2015
Hello Frank,
Props for all the hard work, and thank you for cleaning up some of my
wegl mistakes.
I'm not against WAFFLE_PLATFORM_NULL although having the mesa backend
(and the same name) before this goes in is a must imho.
On 30/03/15 19:12, Frank Henigman wrote:
> The 'null' platform (WAFFLE_PLATFORM_NULL) works with EGL_PLATFORM_NULL,
> KHR_surfaceless_context and EXT_image_dma_buf_import to hide surfaceless
> operation behind a waffle window.
> In this platform waffle windows contain gbm buffers which GLES renders
> into via dmabuf -> EGL image -> GL framebuffer.
> The gbm buffers can be displayed via DRM/KMS.
>
Currently this is based on the that the idea is that the very same
device that is opened for display is the one that does the rendering.
This won't work on platforms such as the tegra K1, and the upcoming
vivante (most likely). If one opens the card node they can ask for the
render device name via drmGetRenderDeviceNameFromFd(). Using the latter
for rendering and the former for the privileged KMS stuff.
The patch has some long lines, unmatched init/fini (ctor/dtor) which I
don't think makes sense to nag about.
Fwiw, I think that splitting this into smaller patches would be very nice.
> --- a/LICENSE.txt
> +++ b/LICENSE.txt
> @@ -11,6 +12,10 @@ modification, are permitted provided that the following conditions are met:
> this list of conditions and the following disclaimer in the documentation
> and/or other materials provided with the distribution.
>
> +- Neither the name of Google Inc. nor the names of its contributors may be used
> + to endorse or promote products derived from this software without specific
> + prior written permission.
> +
Can we have this hunk with the new sources in the null platform ?
> --- a/Options.cmake
> +++ b/Options.cmake
> diff --git a/include/waffle/waffle.h b/include/waffle/waffle.h
> index df0218e..aae35ee 100644
> --- a/include/waffle/waffle.h
> +++ b/include/waffle/waffle.h
> @@ -119,6 +119,7 @@ enum waffle_enum {
> WAFFLE_PLATFORM_GBM = 0x0016,
> WAFFLE_PLATFORM_WGL = 0x0017,
> WAFFLE_PLATFORM_NACL = 0x0018,
> + WAFFLE_PLATFORM_NULL = 0x0019,
>
Please update the man pages - waffle_init and waffle_enum.
> // ------------------------------------------------------------------
> // For waffle_config_choose()
> @@ -173,6 +174,10 @@ enum waffle_enum {
> WAFFLE_WINDOW_WIDTH = 0x0310,
> WAFFLE_WINDOW_HEIGHT = 0x0311,
> WAFFLE_WINDOW_FULLSCREEN = 0x0312,
> +
> + WAFFLE_WINDOW_NULL_SHOW_METHOD = 0x0320,
> + WAFFLE_WINDOW_NULL_SHOW_METHOD_FLIP = 0x0321,
> + WAFFLE_WINDOW_NULL_SHOW_METHOD_COPY = 0x0322,
> };
>
Ditto (waffle_enum/waffle_window) plus wcore_enum_to_string().
> --- /dev/null
> +++ b/include/waffle/waffle_null.h
> +#define __GBM__ 1
> +
I never understood why one needs this. Does anyone have a link that I
can read on the topic ?
> +struct waffle_null_window {
> + struct waffle_null_display display;
> + int width;
> + int height;
> + //XXX expose native buffers here, like gbm_bo, EGLImage, or GL framebuffer?
I'm assuming that this will be covered before we hit the release ?
Alternatively we might have to add API guards around the struct (and the
waffle union) as it gets extended.
> --- /dev/null
> +++ b/src/waffle/null/wnull_context.c
> @@ -0,0 +1,87 @@
> +// Copyright 2015 Google Inc. All Rights Reserved.
> +//
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE.txt file or at
> +// https://developers.google.com/open-source/licenses/bsd
> +
Any reason for not adding the licence inline in here ?
> +struct wcore_context*
> +wnull_context_create(struct wcore_platform *wc_plat,
> + struct wcore_config *wc_config,
> + struct wcore_context *wc_share_ctx)
> +{
> + struct wnull_context *ctx = wcore_calloc(sizeof(*ctx));
Other places in waffle seems to keep these two separate. Which imho
makes the code cleaner.
Also please add an empty line between variable declarations and code.
> + bool ok = true;
> +#define LOOKUP(type, name, args) \
> + ctx->name = waffle_dl_sym(dl, #name); \
Waffle itself does not use its public API. Although the whole dispatch
(func pointers) might be better to live in wnull_platform.
> --- /dev/null
> +++ b/src/waffle/null/wnull_display.c
> @@ -0,0 +1,631 @@
> +struct wnull_display_buffer {
> + struct wgbm_platform *plat;
> + struct gbm_bo *bo;
> + uint32_t drm_fb;
> + int drm_fd;
> + int dmabuf_fd;
> + void (*finish)();
> +};
> +
> +struct drm_display {
> + struct gbm_device *gbm_device;
> + drmModeConnectorPtr conn;
> + drmModeModeInfoPtr mode;
> + drmModeCrtcPtr crtc;
> + int32_t width;
> + int32_t height;
> + bool setcrtc_done;
> + bool flip_pending;
> + struct wnull_display_buffer scanout[2];
> + struct wnull_display_buffer *current;
> +};
> +
> +
> +static void
> +wnull_display_buffer_teardown(struct wnull_display_buffer *buf)
> +{
> + if (!buf || !buf->bo)
> + return;
An extra empty line after each if block looks cleaner imho.
> +void
> +wnull_display_buffer_destroy(struct wnull_display_buffer *buf)
> +{
> + wnull_display_buffer_teardown(buf);
> + free(buf);
> + prt("destroyed display buffer %p\n",buf);
> +}
Not sure that splitting out wnull_display_buffer_{teardown,init} brings
anything here. The latter seems to be used to setup the dpy_buf, but I'm
suspecting that we can simplify things (see below).
> +bool
> +wnull_display_buffer_dmabuf(struct wnull_display_buffer *buf,
> + int *fd,
> + uint32_t *stride)
The function name feels a bit short/incomplete imho.
> +//XXX i915-specific
> +static bool
> +buffer_copy(struct wnull_display_buffer *dst,
> + struct wnull_display_buffer *src)
> +{
> + assert(dst->bo);
> + assert(src->bo);
> +
> + struct drm_i915_gem_get_tiling dst_tiling = {
> + .handle = dst->plat->gbm_bo_get_handle(dst->bo).u32,
> + };
> + struct drm_i915_gem_get_tiling src_tiling = {
> + .handle = src->plat->gbm_bo_get_handle(src->bo).u32,
> + };
> + int dst_fd = dst->plat->gbm_device_get_fd(dst->plat->gbm_bo_get_device(dst->bo));
> + int src_fd = src->plat->gbm_device_get_fd(src->plat->gbm_bo_get_device(src->bo));
> +
> + if (drmIoctl(dst_fd, DRM_IOCTL_I915_GEM_GET_TILING, &dst_tiling) ||
> + drmIoctl(src_fd, DRM_IOCTL_I915_GEM_GET_TILING, &src_tiling))
> + return false;
> +
> + if (dst_tiling.tiling_mode != src_tiling.tiling_mode)
> + return false;
> +
> + unsigned rows;
> + switch (dst_tiling.tiling_mode) {
> + case I915_TILING_NONE:
> + rows = 1;
> + break;
> + case I915_TILING_X:
> + rows = 8;
> + break;
> + default:
> + return false;
> + }
> +
> + unsigned dst_step = dst->plat->gbm_bo_get_stride(dst->bo) * rows;
> + unsigned src_step = src->plat->gbm_bo_get_stride(src->bo) * rows;
> + unsigned copy_size = MIN(src_step, dst_step);
> + // round up, not down, or we miss the last partly filled tile
> + unsigned num_copy = (MIN(src->plat->gbm_bo_get_height(src->bo),
> + dst->plat->gbm_bo_get_height(dst->bo)) + rows - 1)
> + / rows;
> +
> + void *tmp = malloc(copy_size);
> + if (!tmp)
> + return false;
> +
> + struct drm_i915_gem_pread pread = {
> + .handle = src->plat->gbm_bo_get_handle(src->bo).u32,
> + .size = copy_size,
> + .offset = 0,
> + .data_ptr = (uint64_t) (uintptr_t) tmp,
> + };
> +
> + struct drm_i915_gem_pwrite pwrite = {
> + .handle = dst->plat->gbm_bo_get_handle(dst->bo).u32,
> + .size = copy_size,
> + .offset = 0,
> + .data_ptr = (uint64_t) (uintptr_t) tmp,
> + };
> +
> + // blit on gpu must be faster than this, but seems complicated to do
> + bool ok = true;
> + for (int i = 0; ok && i < num_copy; ++i) {
> + ok = !(drmIoctl(src_fd, DRM_IOCTL_I915_GEM_PREAD, &pread) ||
> + drmIoctl(dst_fd, DRM_IOCTL_I915_GEM_PWRITE, &pwrite));
> + pread.offset += src_step;
> + pwrite.offset += dst_step;
> + }
> + free(tmp);
> + return ok;
> +}
> +
I'm not sure how excited Daniel Vettel and the other kernel folk will be
to see this. Have anyone tried reaching them - they might have another
approach to this.
> +bool
> +wnull_display_copy_buffer(struct wnull_display *dpy,
> + struct wnull_display_buffer *buf)
> +{
> + if ( !wnull_display_show_buffer(dpy, drm->current)) {
Please, no space between ( and !
> + prt("show failed %p\n", buf);
> + return false;
> + }
> +
> + ++drm->current;
> + if (drm->current == ARRAY_END(drm->scanout))
> + drm->current = drm->scanout;
C++ like construct.
> + return true;
> +}
> +
> +static drmModeModeInfoPtr
> +choose_mode(drmModeConnectorPtr conn)
> +{
> + drmModeModeInfoPtr mode = NULL;
> + assert(conn);
> + assert(conn->connection == DRM_MODE_CONNECTED);
> + // use first preferred mode if any, else end up with last mode in list
> + for (int i = 0; i < conn->count_modes; ++i) {
> + mode = conn->modes + i;
> + if (mode->type & DRM_MODE_TYPE_PREFERRED)
if (conn->modes[i] & DRM...
> + break;
> + }
> + return mode;
> +}
> +
> +static int
> +choose_crtc(int fd, unsigned count_crtcs, drmModeConnectorPtr conn)
> +{
> + drmModeEncoderPtr enc = 0;
Drop the initialiser ?
> + for (int i = 0; i < conn->count_encoders; ++i) {
> + drmModeFreeEncoder(enc);
We should not need this.
> + enc = drmModeGetEncoder(fd, conn->encoders[i]);
drmModeGetEncoder can return NULL.
> + unsigned b = enc->possible_crtcs;
> + drmModeFreeEncoder(enc);
> + for (int j = 0; b && j < count_crtcs; b >>= 1, ++j) {
> + if (b & 1)
> + return j;
> + }
> + }
> + return -1;
> +}
> +
> +static void
> +drm_display_destroy(struct drm_display *self, struct wgbm_platform *plat)
> +{
> + struct wnull_display_buffer *buf;
> + for (buf = self->scanout; buf < ARRAY_END(self->scanout); ++buf)
> + wnull_display_buffer_teardown(buf);
> +
> + if (self->gbm_device) {
> + int fd = plat->gbm_device_get_fd(self->gbm_device);
> + plat->gbm_device_destroy(self->gbm_device);
> + close(fd);
> + }
Move the gbm cleanup after drmModeFree* ?
> +
Missing drmModeFreeCrtc().
> + drmModeFreeConnector(self->conn);
> + free(self);
> +}
> +
> +static struct drm_display *
> +drm_display_create(int fd, struct wgbm_platform *plat)
> +{
> + struct drm_display *drm = wcore_calloc(sizeof(*drm));
> + if (!drm)
> + return NULL;
> +
> + dlopen("libglapi.so.0", RTLD_LAZY | RTLD_GLOBAL);
> + drm->gbm_device = plat->gbm_create_device(fd);
> + if (!drm->gbm_device) {
> + wcore_errorf(WAFFLE_ERROR_UNKNOWN, "gbm_create_device failed");
> + goto error;
> + }
> +
> + drm->conn = NULL;
> + drmModeResPtr mr = drmModeGetResources(fd);
> + if (!mr) {
> + wcore_errorf(WAFFLE_ERROR_UNKNOWN,
> + "no display on device (is it a render node?");
> + goto error;
> + }
> +
> + bool monitor_connected = false;
> + for (int i = 0; i < mr->count_connectors; ++i) {
> + drmModeFreeConnector(drm->conn);
> + drm->conn = drmModeGetConnector(fd, mr->connectors[i]);
> + if (!drm->conn || drm->conn->connection != DRM_MODE_CONNECTED)
> + continue;
> + monitor_connected = true;
> + drm->mode = choose_mode(drm->conn);
> + if (!drm->mode)
> + continue;
> + int n = choose_crtc(fd, mr->count_crtcs, drm->conn);
> + if (n < 0)
> + continue;
> + drm->crtc = drmModeGetCrtc(fd, mr->crtcs[n]);
> + if (drm->crtc) {
> + drm->width = drm->mode->hdisplay;
> + drm->height = drm->mode->vdisplay;
> + return drm;
> + }
> + }
> +
We seems to be leaking mr ?
> + if (!monitor_connected) {
> + prt("headless\n");
> + assert(!drm->crtc);
> + // arbitrary
> + drm->width = 1280;
> + drm->height = 1024;
Do we really want a non-zero width/height on a headless system ? If so a
comment would be good.
> + return drm;
> + }
> +
> +error:
> + drm_display_destroy(drm, plat);
> + return NULL;
> +}
> +
> +bool
> +wnull_display_destroy(struct wcore_display *wc_self)
> +{
> + struct wnull_display *self = wnull_display(wc_self);
> + if (!self)
> + return true;
> +
> + if (self->drm)
> + drm_display_destroy(self->drm,
> + wgbm_platform(wegl_platform(wc_self->platform)));
> +
> + bool ok = wegl_display_teardown(&self->wegl);
> +
Can we mirror wnull_display_connect - swap the order of
wegl_display_teardown and drm_display_destroy ?
> +struct wcore_display*
> +wnull_display_connect(struct wcore_platform *wc_plat,
> + const char *name)
> +{
> + struct wgbm_platform *plat = wgbm_platform(wegl_platform(wc_plat));
> +
> + struct wnull_display *self = wcore_calloc(sizeof(*self));
> + if (!self)
> + return NULL;
> +
> + int fd;
> + if (name != NULL)
> + fd = open(name, O_RDWR | O_CLOEXEC);
> + else
> + fd = wgbm_get_default_fd_for_pattern("card[0-9]*");
> +
If we save the fd in wnull_display it will save us a few
gbm_device_get_fd() calls.
> +struct ctx_win {
> + struct wnull_context *ctx;
> + struct wnull_window *win;
> +};
> +
> +// Keep track of which context is current and maintain a list of which
> +// context/window pairs have been current.
> +// This lets us answer two questions:
> +// 1) Is it the first time the given pair will be current together?
> +// 2) Which windows have been current with the current context?
> +//
Rather silly question - why do we need to know the answer to either one
? Perhaps we should mention it here. If the only reason is to avoid the
extra glViewport/glScissor (and we can live with it), we can simplify
things vastly.
> --- /dev/null
> +++ b/src/waffle/null/wnull_platform.c
> @@ -0,0 +1,103 @@
> +static const struct wcore_platform_vtbl wnull_platform_vtbl = {
> + .destroy = wgbm_platform_destroy,
> +
> + .make_current = wnull_make_current,
> + .get_proc_address = wegl_get_proc_address,
> + .dl_can_open = wgbm_dl_can_open,
> + .dl_sym = wgbm_dl_sym,
> +
Is one allowed to use desktop GL and/or GLES1 considering the
restriction at wnull_context_create ? If not, perhaps you shouldn't use
the wgbm functions here.
> + .display = {
> + .connect = wnull_display_connect,
> + .destroy = wnull_display_destroy,
> + .supports_context_api = wegl_display_supports_context_api,
Considering that you handle the context differently than wegl (and error
out for non GLES2) I think you should provide wnull specific function here.
> --- /dev/null
> +++ b/src/waffle/null/wnull_platform.h
> @@ -0,0 +1,10 @@
> +
> +#pragma once
> +
struct wcore_platform;
> +
> +struct wcore_platform*
> +wnull_platform_create(void);
> --- /dev/null
> +++ b/src/waffle/null/wnull_window.c
> @@ -0,0 +1,495 @@
> +enum window_part {
> + WINDOW_PART_COLOR = 0x01,
> + WINDOW_PART_DEPTH = 0x02,
> + WINDOW_PART_STENCIL = 0x04,
> +};
> +
> +struct window_buffer {
> + struct wnull_display_buffer *dpy_buf;
> + EGLImageKHR image;
> + GLuint fb;
> + GLuint color;
> + GLuint depth_stencil;
> +};
> +
> +struct wnull_window {
> + uint32_t width;
> + uint32_t height;
> + unsigned parts;
> + bool show;
> + intptr_t show_method;
> + uint32_t gbm_format;
> + uint32_t drm_format;
> + GLenum depth_stencil_format;
> + struct window_buffer buffer[3];
> + struct window_buffer *current;
> + struct wcore_window wcore;
Normally derived structs wrap around the wcore ones. The wcore member
should be the first one in the struct ?
> +};
> +
> +struct wnull_window*
> +wnull_window(struct wcore_window *wcore_self)
> +{
> + if (wcore_self)
> + return container_of(wcore_self, struct wnull_window, wcore);
> + else
> + return 0;
return NULL
> +}
> +
Perhaps wnull_window and the structs above could live in a header ?
> +static void
> +window_buffer_destroy_fb(struct window_buffer *self,
> + struct wnull_context *ctx)
> +{
> + prt("destroy fb %u\n",self->fb);
> + ctx->glDeleteRenderbuffers(1, &self->color);
> + ctx->glDeleteRenderbuffers(1, &self->depth_stencil);
> + ctx->glDeleteFramebuffers(1, &self->fb);
> + self->fb = 0;
> +}
Can we have a window_buffer_create_fbs() analogous to the dtor above ?
> +
> +// If there is a GL FB, 'ctx' is the context used to create it, otherwise
> +// 'ctx' is NULL.
> +static void
> +window_buffer_teardown(struct window_buffer *self,
> + struct wnull_display *dpy,
> + struct wnull_context *ctx)
> +{
> + struct wegl_platform *plat = wegl_platform(dpy->wegl.wcore.platform);
> +
> + if (self->fb) {
> + assert(ctx);
> + window_buffer_destroy_fb(self, ctx);
> + }
> + if (self->image != EGL_NO_IMAGE_KHR) {
> + plat->eglDestroyImageKHR(dpy->wegl.egl, self->image);
> + self->image = EGL_NO_IMAGE_KHR;
> + }
> + wnull_display_buffer_destroy(self->dpy_buf);
> + self->dpy_buf = 0;
> +}
> +
> +#define GLCHECK { \
Bikeshed: How about CHECK_GL_ERROR() or GLCHECK_GETERROR()?
> + GLenum e = ctx->glGetError(); \
> + if (e != GL_NO_ERROR) { \
> + prt(stderr, "gl error %x @ line %d\n", (int)e, __LINE__); \
> + wcore_errorf(WAFFLE_ERROR_UNKNOWN, "gl error %x @ line %d\n", (int)e, __LINE__); \
> + goto gl_error; \
> + } \
> +}
> +
> +static bool
> +wnull_window_prepare_current_buffer(struct wnull_window *self,
> + struct wnull_display *dpy)
You don't need the struct wnull_display here. It is available via
wnull_window(self->wcore->display).
> +static bool
> +wnull_window_prepare_current_buffer(struct wnull_window *self,
> + struct wnull_display *dpy)
> +{
> + self->current->dpy_buf = wnull_display_buffer_create(dpy,
Why do we "store" a display_buffer in the window buffer ? Wouldn't it be
cleaner if we create/store it in struct drm_display and just have a
pointer to in ?
> + EGLint attr[] = {
> + EGL_WIDTH, self->width,
> + EGL_HEIGHT, self->height,
> + EGL_LINUX_DRM_FOURCC_EXT, self->drm_format,
> + EGL_DMA_BUF_PLANE0_FD_EXT, dmabuf_fd,
> + EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
> + EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
> + EGL_NONE,
> + };
static const ?
> +buf_error:
> + window_buffer_teardown(self->current, dpy, ctx);
> +
> +gl_error:
> + if (save_fb >= 0)
> + ctx->glBindFramebuffer(GL_FRAMEBUFFER, save_fb);
> + if (save_rb >= 0)
> + ctx->glBindFramebuffer(GL_RENDERBUFFER, save_rb);
> + return false;
> +}
Selecting which error paths we use seems arbitrary :'-(
> +bool
> +wnull_window_destroy(struct wcore_window *wc_self)
> +{
> + if (!wc_self)
> + return true;
> +
> + struct wnull_window *self = wnull_window(wc_self);
> + prt("window destroy %p\n", self);
> + if (wc_self->display) {
Hmm... have you ever encountered a case where there is a window and the
wcore_display is null ?
> + struct wnull_display *dpy = wnull_display(wc_self->display);
> + struct window_buffer *buf;
> + for (buf = self->buffer; buf != ARRAY_END(self->buffer); ++buf) {
> + // If the buffer has a GL FB, that FB must exist in the current
> + // context, because when contexts cease to be current we destroy
> + // all their GL FBs.
> + window_buffer_teardown(buf,
> + dpy,
> + buf->fb ? dpy->current_context : NULL);
> + }
> +
> + // tell the display this window is gone
> + wnull_display_clean(dpy, NULL, self);
> + }
> +
> + free(self);
> + return true;
> +}
> +
> +static void
> +wnull_window_destroy_fbs(struct wnull_window *self,
> + struct wnull_context *ctx)
> +{
> + struct window_buffer *buf;
> + for (buf = self->buffer; buf != ARRAY_END(self->buffer); ++buf)
> + window_buffer_destroy_fb(buf, ctx);
This (and a few other places) reads very C++ like. Can we go for the
more widely used:
for (i = 0; i < MAX_BUFFER_SIZE; i++)
window_buffer_destroy_fb(self->buffer[i], ctx)
> +struct wcore_window*
> +wnull_window_create(struct wcore_platform *wc_plat,
> + struct wcore_config *wc_config,
> + int32_t width,
> + int32_t height,
> + const intptr_t attrib_list[])
> +{
> + if (wcore_window_init(&window->wcore, wc_config))
> + return &window->wcore;
> +
Move the wcore_window_init() at the top of the function and goto error;
on failure ?
> +error:
> + wnull_window_destroy(&window->wcore);
> + return NULL;
> +}
> +
> +bool
> +wnull_window_show(struct wcore_window *wc_self)
> +{
> + struct wnull_window *self = wnull_window(wc_self);
> + self->show = true;
I would assume that one wants to trigger repaint/render at this point ?
> + return true;
> +}
> +
> +bool
> +wnull_window_swap_buffers(struct wcore_window *wc_self)
> +{
> + if (ok) {
> + ++self->current;
> + if (self->current == ARRAY_END(self->buffer))
> + self->current = self->buffer;
Another C++sm.
> +bool
> +wnull_make_current(struct wcore_platform *wc_plat,
> + struct wcore_display *wc_dpy,
> + struct wcore_window *wc_window,
> + struct wcore_context *wc_ctx)
> +{
> + struct wegl_platform *plat = wegl_platform(wc_plat);
> + struct wnull_display *dpy = wnull_display(wc_dpy);
> + struct wnull_context *ctx = wnull_context(wc_ctx);
> + struct wnull_context *old_ctx = dpy->current_context;
> + struct wnull_window *win = wnull_window(wc_window);
> +
> + bool first; // first time the context/window pair will be current?
> + struct wnull_window **old_win = NULL; // list of windows in old context
> + if (!wnull_display_make_current(dpy, ctx, win, &first, &old_win))
> + return false;
> +
> + // When the current context is changed to a different one we must
> + // delete any framebuffers created in the first context as it may
> + // not be seen again.
> + if (old_ctx && old_ctx != ctx && old_win)
> + for (struct wnull_window **w = old_win; *w; ++w)
> + wnull_window_destroy_fbs(*w, old_ctx);
> + free(old_win);
> +
> + if (!plat->eglMakeCurrent(dpy->wegl.egl,
> + EGL_NO_SURFACE,
> + EGL_NO_SURFACE,
> + ctx ? ctx->wegl.egl : EGL_NO_CONTEXT)) {
> + wegl_emit_error(plat, "eglMakeCurrent");
> + return false;
> + }
> +
> + bool ok = true;
> + if (ctx && win) {
> + ok = wnull_window_prepare_current_buffer(win, dpy);
> + if (ok && first) {
> + prt("setting viewport\n");
> + ctx->glViewport(0, 0, win->width, win->height);
> + ctx->glScissor(0, 0, win->width, win->height);
Pardon for the silly question but how big is the Viewport/Scissor
overhead ? You seem to be going through crazy amount of hoops to prevent it.
All in all things look quite good imho, despite that I have picked at
some places. The latter of which is most likely due to my lack of
knowledge than things being wrong :-]
Cheers
Emil
More information about the waffle
mailing list