[Mesa-dev] [PATCH] Added pbuffer hooks for surfaceless platform
Gurchetan Singh
gurchetansingh at chromium.org
Mon May 16 22:25:58 UTC 2016
Hi Chad,
Yes, we are running dEQP on the surfaceless platform. The front-buffered
pbuffer approach mostly works, though with a double-buffered pbuffer or
back-buffered pbuffer + the pastebin changes we do pass an additional 70
GLES3 tests. It is probably best to follow what X11 does to avoid breaking
things. I'll have an updated patch shortly.
Best wishes,
Gurchetan
On Mon, May 16, 2016 at 12:36 PM, Chad Versace <chad.versace at intel.com>
wrote:
> +Kristian, who also has an interest EGL
>
> On Tue 10 May 2016, Gurchetan Singh wrote:
> > Hi Chad,
> >
> > Thanks for the review and good suggestions. I am interested to know your
> > opinion regarding an issue that comes up when implementing a
> back-buffered
> > pbuffer. The problem is we'll be creating a front renderbuffer, not a
> back
> > renderbuffer if we pass in single buffered visuals.
>
> I was afraid that this would cause a chain of issues.
>
> > For example, in intelCreateBuffer in the i965 driver, the back buffer is
> > only created when double-buffered visuals are passed in:
> >
> > >>_mesa_add_renderbuffer(fb, BUFFER_FRONT_LEFT, &rb->Base.Base);
> >
> > >>if (mesaVis->doubleBufferMode) {
> > >> rb = intel_create_renderbuffer(rgbFormat, num_samples);
> > >> _mesa_add_renderbuffer(fb, BUFFER_BACK_LEFT, &rb->Base.Base)
> > >>}
>
> Right, that's a problem.
>
> > Due to this, with a single-buffered configuration, we'll have to do the
> > following in surfaceless_image_get_buffers:
> >
> > >>buffers->image_mask |= __DRI_IMAGE_BUFFER_FRONT
> >
> > not this:
> >
> > >>buffers->image_mask |= __DRI_IMAGE_BUFFER_BACK
>
> I think the above snippet is the correct solution. The surfaceless
> platform should assign single-buffered configs to the pbuffers, and
> create a *front* buffer for the pbuffer but no back buffer.
>
> > If this a problem, we'll have to pass in a parameter in the visual
> > indicating we're trying to create an EGL pbuffer (i.e,
> > mesaVis->is_egl_pbuffer). Then we'll set the proper renderbuffer and
> > framebuffer options from that config in several places. Some these
> places
> > include:
> >
> > http://pastebin.com/28ddebWF
>
> I think you should avoid fixing all the things the pastebin patch. In
> attempting to fix the world, I believe there is a lot of risk that the
> "fixed" code will break existing users of pbuffers and, more
> importantly, pixmaps. The Mesa/i965 code around pixmaps and pbuffers
> have many hidden assumptions, I'm afraid of breaking those assumptions.
>
> > Is the fact we'll work with the default front render buffer a problem?
> If
> > so, what's the best way to fix the issue?
>
> Yes, it's a small problem. But I think it's an acceptable compromise,
> and the correct way forward.
>
> I investigated the EGL/X11 platform more deeply, to learn exactly how it
> handles pbuffers. After a good hour in gdb, I discovered that it handles
> pbuffers exactly as you suggested in this email: the X11 platform
> assigns single-buffered configs to pbuffers, and creates
> __DRI_IMAGE_BUFFER_FRONT but never __DRI_IMAGE_BUFFER_BACK.
>
> In experimenting with gdb, I played with the approach you suggested by
> modifying your patch. Below is my experimental diff against your patch.
> As far as I can tell, your patch + the diff works as expected.
> It succeeds in making Piglit's egl-create-pbuffer-surface test pass
> (though I had to also hack the Piglit test to remove the X11
> dependency).
>
> How are you testing this patch? Even more importantly, what prompted you
> to write this patch?!? Is this patch related to dEQP somehow?
>
> By the way, I'm currently on my sabbatical, so I'm not checking mesa-dev
> daily. I'll do my best to check my email bi-daily, though, until we get
> this patch resolved.
>
> ------ Hacky experimental diff
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h
> b/src/egl/drivers/dri2/egl_dri2.h
> index ecf9c76..5fe7697 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -294,7 +294,10 @@ struct dri2_egl_surface
>
> #if defined(HAVE_SURFACELESS_PLATFORM)
> __DRIimage *front;
> - __DRIimage *back;
> +
> + /* FIXME: The 'format' field breaks the build because it conflicts
> with
> + * the same field in the Wayland section.
> + * */
> unsigned int format;
> #endif
>
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> index 08d5448..745d481 100644
> --- a/src/egl/drivers/dri2/platform_surfaceless.c
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> @@ -65,20 +65,13 @@ surfaceless_free_images(struct dri2_egl_surface
> *dri2_surf)
> dri2_dpy->image->destroyImage(dri2_surf->front);
> dri2_surf->front = NULL;
> }
> - if(dri2_surf->back) {
> - dri2_dpy->image->destroyImage(dri2_surf->back);
> - dri2_surf->back = NULL;
> - }
> }
>
> static EGLBoolean
> surfaceless_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface
> *draw)
> {
> - struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
> - __DRIimage *temp = dri2_surf->back;
> -
> - dri2_surf->back = dri2_surf->front;
> - dri2_surf->front = temp;
> + /* The EGL spec mandates that eglSwapBuffers is a no-op for pbuffers.
> */
> + return EGL_TRUE;
> }
>
> static int
> @@ -96,6 +89,19 @@ surfaceless_image_get_buffers(__DRIdrawable
> *driDrawable,
> buffers->front = NULL;
> buffers->back = NULL;
>
> + /* The EGL 1.5 spec states that pbuffers are single-buffered.
> Specifically,
> + * the spec states that they have a back buffer but no front buffer, in
> + * contrast to pixmaps, which have a front buffer but no back buffer.
> + *
> + * Single-buffered surfaces with no front buffer confuse Mesa; so We
> deviate
> + * from the spec, following the precedent of Mesa's EGL X11 platform.
> The
> + * X11 platform correctly assigns pbuffers to single-buffered configs,
> but
> + * assigns the pbuffer a front buffer instead of a back buffer.
> + *
> + * Pbuffers in the X11 platform mostly work today, so let's just copy
> its
> + * behavior instead of trying to fix (and hence potentially breaking)
> the
> + * world.
> + */
> if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
> if (!dri2_surf->front)
> dri2_surf->front =
> @@ -104,14 +110,6 @@ surfaceless_image_get_buffers(__DRIdrawable
> *driDrawable,
> buffers->image_mask |= __DRI_IMAGE_BUFFER_FRONT;
> buffers->front = dri2_surf->front;
> }
> - if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
> - if (!dri2_surf->back)
> - dri2_surf->back =
> - surfaceless_alloc_image(dri2_dpy, dri2_surf);
> -
> - buffers->image_mask |= __DRI_IMAGE_BUFFER_BACK;
> - buffers->back = dri2_surf->back;
> - }
>
> return 1;
> }
> @@ -142,7 +140,7 @@ dri2_surfaceless_create_surface(_EGLDriver *drv,
> _EGLDisplay *disp, EGLint type,
> * configs were filtered out in surfaceless_add_configs_for_visuals).
> */
> srgb = (dri2_surf->base.GLColorspace == EGL_GL_COLORSPACE_SRGB_KHR);
> - config = dri2_conf->dri_double_config[srgb];
> + config = dri2_conf->dri_single_config[srgb];
>
> if (!config)
> goto cleanup_surface;
> @@ -211,19 +209,10 @@ surfaceless_add_configs_for_visuals(_EGLDriver *drv,
> _EGLDisplay *dpy)
> count = 0;
> for (i = 0; i < ARRAY_SIZE(visuals); i++) {
> for (j = 0; dri2_dpy->driver_configs[j]; j++) {
> - const EGLint surface_type = EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
> struct dri2_egl_config *dri2_conf;
> - unsigned int double_buffered = 0;
> -
> - dri2_dpy->core->getConfigAttrib(dri2_dpy->driver_configs[j],
> - __DRI_ATTRIB_DOUBLE_BUFFER, &double_buffered);
> -
> - /* support only double buffered configs */
> - if (!double_buffered)
> - continue;
>
> dri2_conf = dri2_add_config(dpy, dri2_dpy->driver_configs[j],
> - count + 1, surface_type, NULL, visuals[i]);
> + count + 1, EGL_PBUFFER_BIT, NULL, visuals[i]);
> if (dri2_conf)
> count++;
> }
> --
> 2.8.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160516/296d631c/attachment-0001.html>
More information about the mesa-dev
mailing list