[PATCH v2] egl: Adds EGL_EXT_swap_buffers_with_damage implementation
Robert Bragg
robert at sixbynine.org
Sat Feb 11 13:17:53 PST 2012
On Sat, Feb 11, 2012 at 6:20 PM, Benjamin Franzke
<benjaminfranzke at googlemail.com> wrote:
> This looks good and useful to me in general.
> Would like to have that in egl_gallium as well,
> but I can also add that there later on.
>
> 2012/2/11 Robert Bragg <robert at sixbynine.org>:
>> This adds an implementation of EGL_EXT_swap_buffers_with_damage which
>> provides an alternative for eglSwapBuffers called
>> eglSwapBuffersWithDamageEXT that has identical semantics except that it
>> takes a list of damage rectangles that may be passed to a compositor
>> when running as a client in a compositing environment. The compositor
>> can optionally use this region to optimize how much of the screen it
>> redraws in response to the clients new front buffer.
>>
>> Currently this is only supported by the wayland EGL platform.
>>
>> Signed-off-by: Robert Bragg <robert at sixbynine.org>
>> ---
>> docs/EXT_swap_buffers_with_damage.txt | 138 +++++++++++++++++++++++++++++++
>> include/EGL/eglext.h | 8 ++
>> src/egl/drivers/dri2/platform_wayland.c | 25 +++++-
>> src/egl/main/eglapi.c | 28 ++++++
>> src/egl/main/eglapi.h | 8 ++
>> src/egl/main/egldisplay.h | 2 +
>> src/egl/main/eglmisc.c | 2 +
>> 7 files changed, 208 insertions(+), 3 deletions(-)
>> create mode 100644 docs/EXT_swap_buffers_with_damage.txt
>>
>> diff --git a/docs/EXT_swap_buffers_with_damage.txt b/docs/EXT_swap_buffers_with_damage.txt
>> new file mode 100644
>> index 0000000..605338f
>> --- /dev/null
>> +++ b/docs/EXT_swap_buffers_with_damage.txt
>> @@ -0,0 +1,138 @@
>> +EGL_EXT_swap_buffers_with_damage
>> +
>> + EXT_swap_buffers_with_damage
>> +
>> +Name Strings
>> +
>> + EGL_EXT_swap_buffers_with_damage
>> +
>> +Notice
>> +
>> + Copyright 2011 Intel Cooperation. All rights reserved.
>> +
>> +Contributors
>> +
>> + Robert Bragg
>> + Tapani Pälli
>> + Kristian Høgsberg
>> +
>> +Contacts
>> +
>> + Robert Bragg, Intel (robert.bragg 'at' intel.com)
>> +
>> +Status
>> +
>> + Draft
>> +
>> +Version
>> +
>> + 4 - Feb 11, 2012
>> +
>> +Number
>> +
>> + TBD
>> +
>> +Dependencies
>> +
>> + Requires EGL 1.4
>> +
>> + This extension is written against the wording of the EGL 1.4
>> + Specification.
>> +
>> +Overview
>> +
>> + This extension provides a means to issue a swap buffers request to
>> + display the contents of the current back buffer and also specify a
>> + list of damage rectangles that can be passed to a system
>> + compositor so it can minimize how it has to recompose.
>> +
>> + This should be used in situations where an application is only
>> + animating a small portion of a surface since it enables the
>> + compositor to avoid wasting time recomposing parts of the surface
>> + that haven't changed.
>> +
>> +New Procedures and Functions
>> +
>> + EGLBoolean eglSwapBuffersWithDamageEXT (
>> + EGLDisplay dpy,
>> + EGLSurface surface,
>> + EGLint *rects,
>> + EGLint n_rects);
>> +
>> +New Tokens
>> +
>> + None
>> +
>> +Additions to Section 3.9 of the EGL 1.4 Specification (Posting the
>> +Color Buffer)
>> +
>> + Add the following text to the end of subsection 3.9.1 titled
>> + "Posting to a Window"
>> +
>> + As an alternative to eglSwapBuffers use:
>> +
>> + EGLBoolean eglSwapBuffersWithDamageEXT (
>> + EGLDisplay dpy,
>> + EGLSurface surface,
>> + EGLint *rects,
>> + EGLint n_rects);
>> +
>> + to do the same thing as eglSwapBuffers but additionally report
>> + a list of rectangles that defines the region that has truly
>> + changed since the last frame. To be clear; the entire contents
>> + of the back buffer will still be swapped to the front so
>> + applications using this API must still ensure that the entire
>> + back buffer is consistent. The rectangles are only a hint for
>> + the system compositor so it can avoid recomposing parts of the
>> + surface that haven't really changed.
>> + <rects> points to a list of integers in groups of four that
>> + each describe a rectangle in screen coordinates in this
>> + layout: {x, y, width, height}. The rectangles are specified
>> + relative to the top-left of the surface and the x and y
>> + components of each rectangle specify the top-left position of
>> + that rectangle. <n_rects> determines how many groups of 4
>> + integers can be read from <rects>. It is not necessary to
>> + avoid overlaps of the specified rectangles.
>> + In addition to the error conditions checked for by the
>> + eglSwapBuffers api an EGL_BAD_PARAMETER error is generated if
>> + <rects> is NULL and <n_rects> is greater than 0.
>
> Is there a good usecase for rects==NULL and n_rects==0?
> I think a setence that states that in that case no damage reporting
> takes place would be good.
yeah I think we can just say we'll report EGL_BAD_PARAMETER for NULL
rects or if n_rects is less than or equal to 0; I can't think of a
good reason for issuing a swap buffers if nothing has actually
changed.
>
> Also what about n_rects < 0, that should give EGL_BAD_PARAMETER always.
right
>> +
>> +Dependencies on OpenGL ES
>> +
>> + None
>> +
>> +Dependencies on OpenVG
>> +
>> + None
>> +
>> +Issues
>> +
>> +1) Do applications have to make sure the rectangles don't overlap?
>> +
>> + RESOLVED: No, that would be inconvenient for applications and we
>> + see no difficulty for implementations to supporting overlapping
>> + rectangles.
>> +
>> +2) Would it be valid for an implementation to discard the list of
>> + rectangles internally and work just in terms of the
>> + eglSwapBuffers api?
>> +
>> + Yes the rectangles are only there for optimization purposes so
>> + although it wouldn't be beneficial to applications if it was
>> + convenient at times then it would be compliant for an
>> + implementation to discard the rectangles and just call
>> + eglSwapBuffers instead. The error conditions that should be
>> + checked for are compatible with the requirements for
>> + eglSwapBuffers.
>> +
>> +Revision History
>> +
>> + Version 1, 29/07/2011
>> + - First draft
>> + Version 2, 03/08/2011
>> + - Clarify that the rectangles passed may overlap
>> + Version 3, 01/09/2011
>> + - Fix a missing '*' in prototype to make rects a pointer
>> + Version 4, 11,02,2012
>> + - Clarify that implementing in terms of eglSwapBuffers would be
>> + compliant.
>> diff --git a/include/EGL/eglext.h b/include/EGL/eglext.h
>> index a40165e..75b28e4 100644
>> --- a/include/EGL/eglext.h
>> +++ b/include/EGL/eglext.h
>> @@ -340,6 +340,14 @@ typedef EGLBoolean (EGLAPIENTRYP PFNEGLDISPATCHEVENTSINTELPROC) (EGLDisplay dpy)
>> typedef EGLBoolean (EGLAPIENTRYP PFNEGLFORWARDEVENTINTELPROC) (EGLDisplay dpy, EGLNativeEventTypeINTEL event);
>> #endif /* EGL_INTEL_native_event_objects */
>>
>> +#ifndef EGL_EXT_swap_buffers_with_damage
>> +#define EGL_EXT_swap_buffers_with_damage 1
>> +#ifdef EGL_EGLEXT_PROTOTYPES
>> +EGLAPI EGLBoolean EGLAPIENTRY eglSwapBuffersWithDamageEXT(EGLDisplay dpy, EGLSurface surface, const EGLint *rects, EGLint n_rects);
>> +#endif
>> +typedef EGLBoolean (EGLAPIENTRYP PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC) (EGLDisplay dpy, EGLSurface surface, const EGLint *rects, EGLint n_rects);
>> +#endif /* EGL_EXT_swap_buffers_with_damage */
>> +
>> #include <EGL/eglmesaext.h>
>>
>> #ifdef __cplusplus
>> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
>> index b2981a0..93a7090 100644
>> --- a/src/egl/drivers/dri2/platform_wayland.c
>> +++ b/src/egl/drivers/dri2/platform_wayland.c
>> @@ -564,7 +564,11 @@ static const struct wl_callback_listener frame_listener = {
>> * Called via eglSwapBuffers(), drv->API.SwapBuffers().
>> */
>> static EGLBoolean
>> -dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>> +dri2_swap_buffers_with_damage(_EGLDriver *drv,
>> + _EGLDisplay *disp,
>> + _EGLSurface *draw,
>> + const EGLint *rects,
>> + EGLint n_rects)
>> {
>> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
>> @@ -582,6 +586,7 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>> wl_callback_add_listener(callback, &frame_listener, dri2_surf);
>>
>> if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>> + int i;
>> pointer_swap(
>> (const void **) &dri2_surf->dri_buffers[__DRI_BUFFER_FRONT_LEFT],
>> (const void **) &dri2_surf->dri_buffers[__DRI_BUFFER_BACK_LEFT]);
>> @@ -611,8 +616,11 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>> dri2_surf->dx = 0;
>> dri2_surf->dy = 0;
>>
>> - wl_surface_damage(dri2_surf->wl_win->surface, 0, 0,
>> - dri2_surf->base.Width, dri2_surf->base.Height);
>> + for (i = 0; i < n_rects; i++) {
>> + int *rect = &rects[i * 4];
>> + wl_surface_damage(dri2_surf->wl_win->surface, rect[0], rect[1],
>> + rect[2], rect[3]);
>> + }
>
> Hm.. I'd also like to have something for wl_buffer_damage (thats some
> lines before this).
> But we cant use the provided damage region directly,
> since the damage region is relative to the previous buffer.
> Maybe we could cache the damage region in egl,
> and always send the currently and the cached damage region for a buffer.
>
> Note: This is not needed in egl_dri2 since wl_drm buffer damage is a
> no-op anyway,
> but for wl_shm buffers which we use for software rendering in egl_gallium.
> So just see this more as informational for now.
yeah I avoided dealing with the buffer damage since it does only seem
to be relevant to wl_shm currently and there was also talk last week
of removing the buffer damage request.
>
>> }
>>
>> _EGLContext *ctx;
>> @@ -628,6 +636,14 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>> return EGL_TRUE;
>> }
>>
>> +static EGLBoolean
>> +dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>> +{
>> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
>> + EGLint damage[4] = { 0, 0, dri2_surf->base.Width, dri2_surf->base.Height };
>> + return dri2_swap_buffers_with_damage (drv, disp, draw, damage, 1);
>> +}
>
> We usually place a newline before returns (at the end of a function).
sure, I can tweak this.
>
>> +
>> /**
>> * Called via eglCreateImageKHR(), drv->API.CreateImageKHR().
>> */
>> @@ -803,6 +819,7 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
>> drv->API.CreatePixmapSurface = dri2_create_pixmap_surface;
>> drv->API.DestroySurface = dri2_destroy_surface;
>> drv->API.SwapBuffers = dri2_swap_buffers;
>> + drv->API.SwapBuffersWithDamageEXT = dri2_swap_buffers_with_damage;
>> drv->API.CreateImageKHR = dri2_wayland_create_image_khr;
>> drv->API.Terminate = dri2_terminate;
>>
>> @@ -878,6 +895,8 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
>> disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
>> dri2_dpy->authenticate = dri2_wayland_authenticate;
>>
>> + disp->Extensions.EXT_swap_buffers_with_damage = EGL_TRUE;
>> +
>> /* we're supporting EGL 1.4 */
>> disp->VersionMajor = 1;
>> disp->VersionMinor = 4;
>> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
>> index 082a557..f8bc670 100644
>> --- a/src/egl/main/eglapi.c
>> +++ b/src/egl/main/eglapi.c
>> @@ -754,6 +754,31 @@ eglSwapBuffers(EGLDisplay dpy, EGLSurface surface)
>>
>>
>> EGLBoolean EGLAPIENTRY
>> +eglSwapBuffersWithDamageEXT(EGLDisplay dpy, EGLSurface surface,
>> + const EGLint *rects, EGLint n_rects)
>> +{
>> + _EGLContext *ctx = _eglGetCurrentContext();
>> + _EGLDisplay *disp = _eglLockDisplay(dpy);
>> + _EGLSurface *surf = _eglLookupSurface(surface, disp);
>> + _EGLDriver *drv;
>> + EGLBoolean ret;
>> +
>> + _EGL_CHECK_SURFACE(disp, surf, EGL_FALSE, drv);
>> +
>> + /* surface must be bound to current context in EGL 1.4 */
>> + if (_eglGetContextHandle(ctx) == EGL_NO_CONTEXT ||
>> + surf != ctx->DrawSurface)
>> + RETURN_EGL_ERROR(disp, EGL_BAD_SURFACE, EGL_FALSE);
>> +
>> + if (rects == NULL && n_rects != 0)
>> + RETURN_EGL_ERROR(disp, EGL_BAD_PARAMETER, EGL_FALSE);
>> +
>> + ret = drv->API.SwapBuffersWithDamageEXT(drv, disp, surf, rects, n_rects);
>> +
>> + RETURN_EGL_EVAL(disp, ret);
>> +}
>> +
>> +EGLBoolean EGLAPIENTRY
>> eglCopyBuffers(EGLDisplay dpy, EGLSurface surface, EGLNativePixmapType target)
>> {
>> _EGLDisplay *disp = _eglLockDisplay(dpy);
>> @@ -990,6 +1015,9 @@ eglGetProcAddress(const char *procname)
>> { "eglUnbindWaylandDisplayWL", (_EGLProc) eglUnbindWaylandDisplayWL },
>> #endif
>> { "eglPostSubBufferNV", (_EGLProc) eglPostSubBufferNV },
>> +#ifdef EGL_EXT_swap_buffers_with_damage
>> + { "eglSwapBuffersWithDamageEXT", (_EGLProc) eglSwapBuffersWithDamageEXT },
>> +#endif
>> { NULL, NULL }
>> };
>> EGLint i;
>> diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
>> index 056da7b..bbcc337 100644
>> --- a/src/egl/main/eglapi.h
>> +++ b/src/egl/main/eglapi.h
>> @@ -133,6 +133,10 @@ typedef EGLBoolean (*DispatchEventsINTEL_t) (_EGLDriver *drv, _EGLDisplay *dpy);
>> typedef EGLBoolean (*ForwardEventINTEL_t) (_EGLDriver *drv, _EGLDisplay *dpy, EGLNativeEventTypeINTEL event);
>> #endif /* EGL_INTEL_native_event_objects */
>>
>> +#ifdef EGL_EXT_swap_buffers_with_damage
>> +typedef EGLBoolean (*SwapBuffersWithDamageEXT_t) (_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surface, const EGLint *rects, EGLint n_rects);
>> +#endif /* EGL_INTEL_native_event_objects */
>
> This looks like a copy&paste mistake, shouldnt be
> EGL_INTEL_native_event_objects.
ah oops, also I just realized this patch is based on other private
patches I have which will cause people trouble if they try applying
this to master, I'll rebase my next patch.
>> +
>> /**
>> * The API dispatcher jumps through these functions
>> */
>> @@ -213,6 +217,10 @@ struct _egl_api
>> ForwardEventINTEL_t ForwardEventINTEL;
>> #endif /* EGL_INTEL_native_event_objects */
>>
>> +#ifdef EGL_EXT_swap_buffers_with_damage
>> + SwapBuffersWithDamageEXT_t SwapBuffersWithDamageEXT;
>> +#endif /* EGL_EXT_swap_buffers_with_damage */
>> +
>> PostSubBufferNV_t PostSubBufferNV;
>> };
>>
>> diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
>> index 905c7a4..148a533 100644
>> --- a/src/egl/main/egldisplay.h
>> +++ b/src/egl/main/egldisplay.h
>> @@ -113,6 +113,8 @@ struct _egl_extensions
>> EGLBoolean ANDROID_image_native_buffer;
>>
>> EGLBoolean NV_post_sub_buffer;
>> +
>> + EGLBoolean EXT_swap_buffers_with_damage;
>> };
>>
>>
>> diff --git a/src/egl/main/eglmisc.c b/src/egl/main/eglmisc.c
>> index 9d534f0..65ee228 100644
>> --- a/src/egl/main/eglmisc.c
>> +++ b/src/egl/main/eglmisc.c
>> @@ -117,6 +117,8 @@ _eglUpdateExtensionsString(_EGLDisplay *dpy)
>> _EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
>>
>> _EGL_CHECK_EXTENSION(NV_post_sub_buffer);
>> +
>> + _EGL_CHECK_EXTENSION(EXT_swap_buffers_with_damage);
>> #undef _EGL_CHECK_EXTENSION
>> }
>>
>> --
>> 1.7.7.5
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
thanks for the feedback, I'll send an updated patch.
kind regards,
- Robert
More information about the wayland-devel
mailing list