On Mon 15 May 2017, Emil Velikov wrote:
> From: Emil Velikov <emil.veli...@collabora.com>
>
> Analogous to previous commit.
>
> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
> ---
> If people prefer I can split the whitespace/indent changes in
> this/previous path from the rest. Don't feel too strongly either way.
I do think the patches would be easier to review if you did that. As
written, it's difficult to see what's behavioral change vs cosmetic
cleanups.
>
> src/egl/drivers/dri2/platform_x11.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_x11.c
> b/src/egl/drivers/dri2/platform_x11.c
> index 3bce0bb3f21..becf00547e6 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -817,8 +817,7 @@ dri2_copy_region(_EGLDriver *drv, _EGLDisplay *disp,
> if (draw->Type == EGL_PIXMAP_BIT || draw->Type == EGL_PBUFFER_BIT)
> return EGL_TRUE;
>
> - if (dri2_dpy->flush)
> - dri2_dpy->flush->flush(dri2_surf->dri_drawable);
> + dri2_dpy->flush->flush(dri2_surf->dri_drawable);
>
> if (dri2_surf->have_fake_front)
> render_attachment = XCB_DRI2_ATTACHMENT_BUFFER_FAKE_FRONT_LEFT;
> @@ -880,8 +879,7 @@ dri2_x11_swap_buffers_msc(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *draw,
> * happened. The driver should still be using the viewport hack to catch
> * window resizes.
> */
> - if (dri2_dpy->flush &&
> - dri2_dpy->flush->base.version >= 3 && dri2_dpy->flush->invalidate)
> + if (dri2_dpy->flush->base.version >= 3 && dri2_dpy->flush->invalidate)
> dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
>
> return swap_count;
> @@ -893,19 +891,17 @@ dri2_x11_swap_buffers(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *draw)
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
>
> - if (dri2_dpy->dri2) {
> - if (dri2_x11_swap_buffers_msc(drv, disp, draw, 0, 0, 0) != -1) {
> - return EGL_TRUE;
> - }
> + if (!dri2_dpy->flush) {
> + dri2_dpy->core->swapBuffers(dri2_surf->dri_drawable);
> + return EGL_TRUE;
Yes, thank you.
I'll say it again to anyone who's reading. The Go style guide insists on
this, for good reason.
https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow
Try to keep the normal code path at a minimal indentation, and indent
the error handling, dealing with it first. This improves the readability
of the code by permitting visually scanning the normal path quickly.
> + }
> +
> + if (dri2_x11_swap_buffers_msc(drv, disp, draw, 0, 0, 0) == -1) {
> /* Swap failed with a window drawable. */
> _eglError(EGL_BAD_NATIVE_WINDOW, __func__);
> return EGL_FALSE;
> - } else {
> - assert(dri2_dpy->swrast);
> -
> - dri2_dpy->core->swapBuffers(dri2_surf->dri_drawable);
> - return EGL_TRUE;
> }
> + return EGL_TRUE;
> }
There's a lot of hidden complexity in platform_x11.c, and I'm unsure if
this patch preserves proper extension checks. I defer to someone who
knows this code better.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
