On 5 July 2018 at 17:17, Eric Engestrom <eric.engest...@intel.com> wrote: > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: >> On 5 July 2018 at 10:53, Eric Engestrom <eric.engest...@intel.com> wrote: >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: >> >> This fixes crash due to NULL window when swap interval is set >> >> for pbuffer surface. >> >> >> >> Test: CtsDisplayTestCases pass >> >> >> >> Signed-off-by: samiuddi <sami.uddin.moham...@intel.com> >> >> --- >> >> >> >> Kindly ignore this patch >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html >> >> >> >> src/egl/drivers/dri2/platform_android.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/src/egl/drivers/dri2/platform_android.c >> >> b/src/egl/drivers/dri2/platform_android.c >> >> index ca8708a..b5b960a 100644 >> >> --- a/src/egl/drivers/dri2/platform_android.c >> >> +++ b/src/egl/drivers/dri2/platform_android.c >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy, >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); >> >> struct ANativeWindow *window = dri2_surf->window; >> >> >> >> - if (window->setSwapInterval(window, interval)) >> >> + if (window && window->setSwapInterval(window, interval)) >> >> return EGL_FALSE; >> > >> > Shouldn't we return false if we couldn't set the swap interval? >> > >> > I think this should be: >> > if (!window || window->setSwapInterval(window, interval)) >> > return EGL_FALSE; >> > >> Changing the patch as above will lead to eglSwapInterval consistently >> failing for pbuffer surfaces. > > I'm not that familiar with pbuffers, but does SwapInterval really make > sense for them? I thought you couldn't swap a pbuffer. > > If so, failing an invalid op seems like the right thing to do. > Otherwise, I guess sure, no-op returning success is fine. > Looking at eglSwapInterval manpage [1] (with my annotation): "eglSwapInterval — specifies the minimum number of video frame periods per buffer swap for the _window_ associated with the current context." While the spec does not mention window/pbuffer/pixmap at all - it extensively refers to eglSwapBuffers.
Wrt eglSwapBuffers manpage [2] and spec are consistent: "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no effect, and no error is generated." Perhaps it's wrong to set eglSwapInterval for !window surfaces, but its sibling (eglSwapBuffers) does not error out. Hence I doubt many users make a distinction between window and pbuffer surfaces for eglSwap*. Worth bringing to the WG meeting - it' planned for 1st August. -Emil [1] https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglSwapInterval.xhtml [2] https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglSwapBuffers.xhtml _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
