Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi, On Wed, May 22, 2019 at 4:01 AM Ian Romanick wrote: > > On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit > > > > Jason, can we reconsider Andrii's patch? It still applies cleanly > > (https://patchwork.freedesktop.org/patch/237490/) > > Looking at the patch and the "Simple reproducer" in the bug, I think > this just papers over the issue. It seems like the problem is somewhere > down inside the driver's handling of glXBindTexImageEXT. My best guess > is that the texture is GL_TEXTURE_2D but the miptree backing it is > GL_TEXTURE_RECTANGLE. It seems that the glXBindTexImageEXT handling > should mark the miptree as GL_TEXTURE_2D when binding the image to a > texture that is GL_TEXTURE_2D. Or is that not possible for some > non-obvious reason? Sorry to be a pain, but I still get bug reports in xfce for this, for everyone using a Mesa build with debug enabled (or a pre-release for that matter) on Intel, the `assert()` are enabled and this bug occurs with the xfce compositor. Maybe Andrii's patch is just hiding the issue, but that's already the case without the `assert()` enabled (i.e. all stable releases of Mesa), so I guess it's not such a big deal anyway. Can we agree on a fix on this? Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi all, On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit wrote: > > Ugh... not so good. According to Oliver on the bug, this just make the > > assert go away and doesn't actually fix anything. Likely this is needed > > but not sufficient. > > So as far as I understand Oliver found the bad commit in xorg glamor: > https://bugs.freedesktop.org/show_bug.cgi?id=107287 > > So at the moment we should fix just this "assertion" issue for Intel because > "rendering" issue came from xorg/glamor and there is no "rendering" issue in > Intel part. > Please correct me if I incorrect. Reviving an old thread/patch here. Andrey, I reckon your patch here is still much needed as it fixes the assert() issue: intel_mipmap_tree.c:1301: intel_miptree_match_image: Assertion `image->TexObject->Target == mt->target' failed. Which is still occurring even with current master. My patch was to fix the rendering issue (landed a while ago before 18.1 iirc), but yours was never merged and is still needed, I can reproduce the assert() at will with the reproducer from https://bugs.freedesktop.org/show_bug.cgi?id=107117 Jason, can we reconsider Andrii's patch? It still applies cleanly (https://patchwork.freedesktop.org/patch/237490/) Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: Resize EGL surface on update buffer for swrast
Hi
On Thu, Oct 25, 2018 at 5:51 PM Juan A. Suarez Romero
wrote:
>
> On Thu, 2018-10-25 at 14:48 +0200, Olivier Fourdan wrote:
> > After commit a9fb331ea ("wayland/egl: update surface size on window
> > resize"), the surface size is updated as soon as the resize is done, and
> > `update_buffers()` would resize only if the surface size differs from
> > the attached size.
> >
> > However, in the case of swrast, there is no resize callback and the
> > attached size is updated in `dri2_wl_swrast_commit_backbuffer()` prior
> > to the `swrast_update_buffers()` so the attached size is always up to
> > date when it reaches `swrast_update_buffers()` and the surface is never
> > resized.
> >
> > This can be observed with "totem" using the GDK backend on Wayland (the
> > default) when running on software rendering:
> >
> > $ LIBGL_ALWAYS_SOFTWARE=true CLUTTER_BACKEND=gdk totem
> >
> > Resizing the window would leave the EGL surface size unchanged.
> >
> > To avoid the issue, partially revert the part of commit a9fb331ea for
> > `swrast_update_buffers()` and resize on the win size and not the
> > attached size.
> >
> > Fixes: a9fb331ea - wayland/egl: update surface size on window resize
> > Signed-off-by: Olivier Fourdan
> > CC: Daniel Stone
> > CC: Juan A. Suarez Romero
> > CC: mesa-sta...@lists.freedesktop.org
> > ---
>
> I've been checking why this happened. Turns out that in the original patch, we
> update the size in the resize_callback(), so we need to do the changes in
> update_buffers() accordingly.
>
> But turns out that with swrast we are not invoking resize_callback(), so we
> need
> to keep the old code in swrast_update_buffers().
Yeap, that's what I tried to explain in the commit message.
> Thanks for finding this.
>
> Reviewed-by: Juan A. Suarez
Thanks Juan!
Could someone with commit access push that for me? Would be great to
have it fixed in the stable and forthcoming 18.3 branches as well.
>
> > Resending because I got the "mesa-dev" address wrong! And a reply to the
> > previous email won't fly with patchwork... Sorry!
> >
> > src/egl/drivers/dri2/platform_wayland.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_wayland.c
> > b/src/egl/drivers/dri2/platform_wayland.c
> > index 03a3e0993b..69a51e64fd 100644
> > --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -1652,8 +1652,8 @@ swrast_update_buffers(struct dri2_egl_surface
> > *dri2_surf)
> > if (dri2_surf->back)
> >return 0;
> >
> > - if (dri2_surf->base.Width != dri2_surf->wl_win->attached_width ||
> > - dri2_surf->base.Height != dri2_surf->wl_win->attached_height) {
> > + if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> > + dri2_surf->base.Height != dri2_surf->wl_win->height) {
> >
> >dri2_wl_release_buffers(dri2_surf);
> >
>
Cheers,
Olivier
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] wayland/egl: Resize EGL surface on update buffer for swrast
After commit a9fb331ea ("wayland/egl: update surface size on window
resize"), the surface size is updated as soon as the resize is done, and
`update_buffers()` would resize only if the surface size differs from
the attached size.
However, in the case of swrast, there is no resize callback and the
attached size is updated in `dri2_wl_swrast_commit_backbuffer()` prior
to the `swrast_update_buffers()` so the attached size is always up to
date when it reaches `swrast_update_buffers()` and the surface is never
resized.
This can be observed with "totem" using the GDK backend on Wayland (the
default) when running on software rendering:
$ LIBGL_ALWAYS_SOFTWARE=true CLUTTER_BACKEND=gdk totem
Resizing the window would leave the EGL surface size unchanged.
To avoid the issue, partially revert the part of commit a9fb331ea for
`swrast_update_buffers()` and resize on the win size and not the
attached size.
Fixes: a9fb331ea - wayland/egl: update surface size on window resize
Signed-off-by: Olivier Fourdan
CC: Daniel Stone
CC: Juan A. Suarez Romero
CC: mesa-sta...@lists.freedesktop.org
---
Resending because I got the "mesa-dev" address wrong! And a reply to the
previous email won't fly with patchwork... Sorry!
src/egl/drivers/dri2/platform_wayland.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 03a3e0993b..69a51e64fd 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -1652,8 +1652,8 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
if (dri2_surf->back)
return 0;
- if (dri2_surf->base.Width != dri2_surf->wl_win->attached_width ||
- dri2_surf->base.Height != dri2_surf->wl_win->attached_height) {
+ if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
+ dri2_surf->base.Height != dri2_surf->wl_win->height) {
dri2_wl_release_buffers(dri2_surf);
--
2.19.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: Resize EGL surface on update buffer for swrast
Oops got the mesa-dev address wrong...
On Thu, Oct 25, 2018 at 2:40 PM Olivier Fourdan wrote:
>
> After commit a9fb331ea ("wayland/egl: update surface size on window
> resize"), the surface size is updated as soon as the resize is done, and
> `update_buffers()` would resize only if the surface size differs from
> the attached size.
>
> However, in the case of swrast, there is no resize callback and the
> attached size is updated in `dri2_wl_swrast_commit_backbuffer()` prior
> to the `swrast_update_buffers()` so the attached size is always up to
> date when it reaches `swrast_update_buffers()` and the surface is never
> resized.
>
> This can be observed with "totem" using the GDK backend on Wayland (the
> default) when running on software rendering:
>
> $ LIBGL_ALWAYS_SOFTWARE=true CLUTTER_BACKEND=gdk totem
>
> Resizing the window would leave the EGL surface size unchanged.
>
> To avoid the issue, partially revert the part of commit a9fb331ea for
> `swrast_update_buffers()` and resize on the win size and not the
> attached size.
>
> Fixes: a9fb331ea - wayland/egl: update surface size on window resize
> Signed-off-by: Olivier Fourdan
> CC: Daniel Stone
> CC: Juan A. Suarez Romero
> CC: mesa-sta...@lists.freedesktop.org
> ---
> src/egl/drivers/dri2/platform_wayland.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index 03a3e0993b..69a51e64fd 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -1652,8 +1652,8 @@ swrast_update_buffers(struct dri2_egl_surface
> *dri2_surf)
> if (dri2_surf->back)
>return 0;
>
> - if (dri2_surf->base.Width != dri2_surf->wl_win->attached_width ||
> - dri2_surf->base.Height != dri2_surf->wl_win->attached_height) {
> + if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> + dri2_surf->base.Height != dri2_surf->wl_win->height) {
>
>dri2_wl_release_buffers(dri2_surf);
>
> --
> 2.19.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 1/2] wayland/egl: initialize window surface size to window size
On Thu, Oct 25, 2018 at 12:05 PM Olivier Fourdan wrote:
>
> Hi,
>
> On Tue, Aug 7, 2018 at 5:50 PM Juan A. Suarez Romero
> wrote:
> >
> > When creating a windows surface with eglCreateWindowSurface(), the
> > width and height returned by eglQuerySurface(EGL_{WIDTH,HEIGHT}) is
> > invalid until buffers are updated (like calling glClear()).
> >
> > But according to EGL 1.5 spec, section 3.5.6 ("Surface Attributes"):
> >
> > "Querying EGL_WIDTH and EGL_HEIGHT returns respectively the width and
> >height, in pixels, of the surface. For a window or pixmap surface,
> >these values are initially equal to the width and height of the
> >native window or pixmap with respect to which the surface was
> >created"
> >
> > This fixes dEQP-EGL.functional.color_clears.* CTS tests
> >
> > v2:
> > - Do not modify attached_{width,height} (Daniel)
> > - Do not update size on resizing window (Brendan)
> >
> > CC: Daniel Stone
> > CC: Brendan King
> > CC: mesa-sta...@lists.freedesktop.org
> > Tested-by: Eric Engestrom
> > ---
> > src/egl/drivers/dri2/platform_wayland.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/src/egl/drivers/dri2/platform_wayland.c
> > b/src/egl/drivers/dri2/platform_wayland.c
> > index dca099500a8..a5d43094cf3 100644
> > --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -258,6 +258,9 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> > _EGLDisplay *disp,
> >goto cleanup_surf;
> > }
> >
> > + dri2_surf->base.Width = window->width;
> > + dri2_surf->base.Height = window->height;
> > +
> > visual_idx = dri2_wl_visual_idx_from_config(dri2_dpy, config);
> > assert(visual_idx != -1);
> >
> > --
> > 2.17.1
>
> Just a quick heads up, this patch is causing a regression with
> "swrast" which does not have DRI2flushExtension.
>
> Easiest way to demonstrate the issue is to use totem with "swrast" on
> mesa-18.2.x (e.g. Fedora 29) under Wayland:
>
> $ LIBGL_ALWAYS_SOFTWARE=true CLUTTER_BACKEND=gdk totem
>
> Play a video and resize the totem toplevel window, the size of the EGL
> surface remains unchanged...
>
> Reverting that patch fixes the issue, as using a DRI driver which
> supports DRI2flushExtension. Nevertheless, that's a regression.
Sorry, wrong patch, the culprit is:
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_wayland.c?id=a9fb331ea
(or
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_wayland.c?id=7af6be886
cherry-picked in 18.2)
And actually reverting just that part from the patch:
@@ -1635,8 +1646,8 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
if (dri2_surf->back)
return 0;
- if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
- dri2_surf->base.Height != dri2_surf->wl_win->height) {
+ if (dri2_surf->base.Width != dri2_surf->wl_win->attached_width ||
+ dri2_surf->base.Height != dri2_surf->wl_win->attached_height) {
dri2_wl_release_buffers(dri2_surf);
Fixes the issue.
Cheers,
Olivier
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 1/2] wayland/egl: initialize window surface size to window size
Hi,
On Tue, Aug 7, 2018 at 5:50 PM Juan A. Suarez Romero
wrote:
>
> When creating a windows surface with eglCreateWindowSurface(), the
> width and height returned by eglQuerySurface(EGL_{WIDTH,HEIGHT}) is
> invalid until buffers are updated (like calling glClear()).
>
> But according to EGL 1.5 spec, section 3.5.6 ("Surface Attributes"):
>
> "Querying EGL_WIDTH and EGL_HEIGHT returns respectively the width and
>height, in pixels, of the surface. For a window or pixmap surface,
>these values are initially equal to the width and height of the
>native window or pixmap with respect to which the surface was
>created"
>
> This fixes dEQP-EGL.functional.color_clears.* CTS tests
>
> v2:
> - Do not modify attached_{width,height} (Daniel)
> - Do not update size on resizing window (Brendan)
>
> CC: Daniel Stone
> CC: Brendan King
> CC: mesa-sta...@lists.freedesktop.org
> Tested-by: Eric Engestrom
> ---
> src/egl/drivers/dri2/platform_wayland.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index dca099500a8..a5d43094cf3 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -258,6 +258,9 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
>goto cleanup_surf;
> }
>
> + dri2_surf->base.Width = window->width;
> + dri2_surf->base.Height = window->height;
> +
> visual_idx = dri2_wl_visual_idx_from_config(dri2_dpy, config);
> assert(visual_idx != -1);
>
> --
> 2.17.1
Just a quick heads up, this patch is causing a regression with
"swrast" which does not have DRI2flushExtension.
Easiest way to demonstrate the issue is to use totem with "swrast" on
mesa-18.2.x (e.g. Fedora 29) under Wayland:
$ LIBGL_ALWAYS_SOFTWARE=true CLUTTER_BACKEND=gdk totem
Play a video and resize the totem toplevel window, the size of the EGL
surface remains unchanged...
Reverting that patch fixes the issue, as using a DRI driver which
supports DRI2flushExtension. Nevertheless, that's a regression.
Cheers,
Olivier
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()
Hi Marek, On Thu, Aug 9, 2018 at 7:27 PM Marek Olšák wrote: > > This will leak the renderbuffer, but that's not the biggest problem. > > In your bug report, you said that the renderbuffer was created by > intel_new_renderbuffer, but this change is for st/mesa. Something is My comment in the bugzilla is probably wrong actually... > horribly wrong here. The intel driver should not ever end up in > st/mesa, because st/mesa is a different driver. What is going on here? Actually, the drivers aren't the same, sorry, my mistake: https://bugs.freedesktop.org/show_bug.cgi?id=107508#c4 So the backtrace is valid wrt the context / driver. Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Don't wait for fence of old buffer when re-allocating it
Hi
On Tue, Sep 4, 2018 at 6:23 PM Michel Dänzer wrote:
>
> From: Michel Dänzer
>
> We only need to wait for the fence before drawing to a buffer, not
> before reading from it.
>
> This might avoid hangs when re-allocating the fake front buffer, similar
> to the previous change. But I haven't seen any hard evidence that this
> was actually happening in practice.
>
> Signed-off-by: Michel Dänzer
> ---
> src/loader/loader_dri3_helper.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 6c162553f33..c173a61ba06 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -1778,7 +1778,6 @@ dri3_get_buffer(__DRIdrawable *driDrawable,
>&& buffer) {
>
> /* Fill the new buffer with data from an old buffer */
> - dri3_fence_await(draw->conn, draw, buffer);
> if (!loader_dri3_blit_image(draw,
> new_buffer->image,
> buffer->image,
> --
> 2.19.0.rc1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Both patches are successfully:
Tested-by: Olivier Fourdan
Cheers,
Olivier
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()
Hi Dave, On Thu, Aug 9, 2018 at 8:57 PM Dave Airlie wrote: > Sounds like a missing make current somewhere. Humm, the reproducer does a `glXMakeCurrent(dpy, 0, 0)` prior to destroying the context with `glXDestroyContext()`, removing that call to `glXMakeCurrent(dpy, 0, 0)` avoids the crash but it's legal to release the current context without assigning a new one, prior to destroy the context I think (besides, a client should not crash the server). Unless I misunderstood what you meant? Cheers, Olivier Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()
Hi,
On Fri, Aug 3, 2018 at 7:03 PM Aaron Watry wrote:
>
> Tested-by: Aaron Watry
>
> Yay, I can finally use gnome/wayland with the Slack snap again without
> insta-crashing my session.
>
> --Aaron
>
> On Thu, Aug 2, 2018 at 7:29 AM, Olivier Fourdan wrote:
> > st_renderbuffer_delete() can segfault if we get a non-NULL context
> > pointer but if the st_context is NULL:
> >
> > Thread 1 "Xwayland" received signal SIGSEGV, Segmentation fault.
> > in st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
> > 241 pipe_surface_release(st->pipe, >surface_srgb);
> > (gdb) bt
> > #0 st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
> > #1 _mesa_reference_renderbuffer_ () at main/renderbuffer.c:212
> > #2 _mesa_reference_renderbuffer () at main/renderbuffer.h:72
> > #3 _mesa_free_framebuffer_data (0) at main/framebuffer.c:229
> > #4 _mesa_destroy_framebuffer () at main/framebuffer.c:207
> > #5 _mesa_reference_framebuffer_ () at main/framebuffer.c:265
> > #6 _mesa_reference_framebuffer () at main/framebuffer.h:63
> > #7 _mesa_free_context_data () at main/context.c:1326
> > #8 st_destroy_context () at state_tracker/st_context.c:653
> > #9 dri_destroy_context () at dri_context.c:239
> > #10 driDestroyContext () at dri_util.c:524
> > #11 __glXDRIcontextDestroy () at glxdriswrast.c:132
> > #12 __glXFreeContext () at glxext.c:190
> > #13 ContextGone () at glxext.c:82
> > #14 doFreeResource () at resource.c:880
> > #15 FreeResourceByType () at resource.c:941
> > #16 __glXDisp_DestroyContext () at glxcmds.c:437
> > #17 dispatch_DestroyContext () at vnd_dispatch_stubs.c:82
> > #18 Dispatch () at dispatch.c:478
> > #19 dix_main () at main.c:276
> > #20 __libc_start_main () from /lib64/libc.so.6
> > #21 _start () at glxcmds.c:125
> >
> > (gdb) p st
> > $1 = (struct st_context *) 0x0
> >
> > Check for a non-NULL st_context pointer as well to avoid the crash.
> >
> > Bugzilla: https://bugzilla.redhat.com/1611140
> > Signed-off-by: Olivier Fourdan
> > ---
> > Note: This fixes several bug reported downstream, like:
> > https://bugzilla.redhat.com/1611140
> > https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/1762971
> > https://bugs.launchpad.net/ubuntu/+source/mesa/+bug/1754693
> > etc.
> > I don't know what this client actually does, but whatever it is it should
> > not crash Xwayland because of Mesa...
> > I tested this fix against the given reproducer (run snap on
> > Wayland/Xwayland)
> > and it works.
> >
> > src/mesa/state_tracker/st_cb_fbo.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_cb_fbo.c
> > b/src/mesa/state_tracker/st_cb_fbo.c
> > index 73414fdfa1..856d213b73 100644
> > --- a/src/mesa/state_tracker/st_cb_fbo.c
> > +++ b/src/mesa/state_tracker/st_cb_fbo.c
> > @@ -238,8 +238,10 @@ st_renderbuffer_delete(struct gl_context *ctx, struct
> > gl_renderbuffer *rb)
> > struct st_renderbuffer *strb = st_renderbuffer(rb);
> > if (ctx) {
> >struct st_context *st = st_context(ctx);
> > - pipe_surface_release(st->pipe, >surface_srgb);
> > - pipe_surface_release(st->pipe, >surface_linear);
> > + if (st) {
> > + pipe_surface_release(st->pipe, >surface_srgb);
> > + pipe_surface_release(st->pipe, >surface_linear);
> > + }
> >strb->surface = NULL;
> > }
> > pipe_resource_reference(>texture, NULL);
To better understand why this crash occurs, I filed:
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107508
With a simple reproducer program. This is not affecting only Xwayland,
but also Xephyr with glamor backend as well.
Cheers,
Olivier
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glx: Demand success from CreateContext requests
Hey
On Fri, Aug 3, 2018 at 7:44 PM Adam Jackson wrote:
>
> GLXCreate{,New}Context, like most X resource creation requests, does not
> emit a reply and therefore is emitted into the X stream asynchronously.
> However, unlike most resource creation requests, the GLXContext we
> return is a handle to library state instead of an XID. So if context
> creation fails for any reason - say, the server doesn't support indirect
> contexts - then we will fail in strange places for strange reasons.
>
> We could make every GLX entrypoint robust against half-created contexts,
> or we could just verify that context creation worked. Reuse the
> __glXIsDirect code to do this, as a cheap way of verifying that the
> XID is real.
>
> glXCreateContextAttribsARB solves this by using the _checked version of
> the xcb command, so effectively this change makes the classic context
> creation paths as robust as CreateContextAttribs.
>
> Signed-off-by: Adam Jackson
> ---
> src/glx/glxcmds.c | 92 +++
> 1 file changed, 54 insertions(+), 38 deletions(-)
>
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 4db0228eaba..3789f55d038 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -272,6 +272,43 @@ glx_context_init(struct glx_context *gc,
> return True;
> }
>
> +/**
> + * Determine if a context uses direct rendering.
> + *
> + * \param dpyDisplay where the context was created.
> + * \param contextID ID of the context to be tested.
> + * \param error Out parameter, set to True on error if not NULL
> + *
> + * \returns \c True if the context is direct rendering or not.
> + */
> +static Bool
> +__glXIsDirect(Display * dpy, GLXContextID contextID, int *error)
Nitpicking, maybe a Bool would be more explicit here (even if it's the
same), it's set to “None” and later set to “True”.
> +{
> + CARD8 opcode;
> + xcb_connection_t *c;
> + xcb_generic_error_t *err;
> + xcb_glx_is_direct_reply_t *reply;
> + Bool is_direct;
> +
> + opcode = __glXSetupForCommand(dpy);
> + if (!opcode) {
> + return False;
> + }
> +
> + c = XGetXCBConnection(dpy);
> + reply = xcb_glx_is_direct_reply(c, xcb_glx_is_direct(c, contextID), );
> + is_direct = (reply != NULL && reply->is_direct) ? True : False;
> +
> + if (err != NULL) {
> + *error = True;
glXIsDirect() passes “NULL” as “error”, but it's set unconditionally here.
> + __glXSendErrorForXcb(dpy, err);
> + free(err);
> + }
> +
> + free(reply);
> +
> + return is_direct;
> +}
>
> /**
> * Create a new context.
> @@ -376,6 +413,21 @@ CreateContext(Display *dpy, int generic_id, struct
> glx_config *config,
> gc->share_xid = shareList ? shareList->xid : None;
> gc->imported = GL_FALSE;
>
> + /* Unlike most X resource creation requests, we're about to return a
> handle
> +* with client-side state, not just an XID. To simplify error handling
> +* elsewhere in libGL, force a round-trip here to ensure the CreateContext
> +* request above succeeded.
> +*/
> + {
> + int error = None;
> + int isDirect = __glXIsDirect(dpy, gc->xid, );
> +
> + if (error != None || isDirect != gc->isDirect) {
> + gc->vtable->destroy(gc);
> + gc = NULL;
> + }
> + }
> +
> return (GLXContext) gc;
> }
>
> @@ -612,42 +664,6 @@ glXCopyContext(Display * dpy, GLXContext source_user,
> }
>
>
> -/**
> - * Determine if a context uses direct rendering.
> - *
> - * \param dpyDisplay where the context was created.
> - * \param contextID ID of the context to be tested.
> - *
> - * \returns \c True if the context is direct rendering or not.
> - */
> -static Bool
> -__glXIsDirect(Display * dpy, GLXContextID contextID)
> -{
> - CARD8 opcode;
> - xcb_connection_t *c;
> - xcb_generic_error_t *err;
> - xcb_glx_is_direct_reply_t *reply;
> - Bool is_direct;
> -
> - opcode = __glXSetupForCommand(dpy);
> - if (!opcode) {
> - return False;
> - }
> -
> - c = XGetXCBConnection(dpy);
> - reply = xcb_glx_is_direct_reply(c, xcb_glx_is_direct(c, contextID), );
> - is_direct = (reply != NULL && reply->is_direct) ? True : False;
> -
> - if (err != NULL) {
> - __glXSendErrorForXcb(dpy, err);
> - free(err);
> - }
> -
> - free(reply);
> -
> - return is_direct;
> -}
> -
> /**
> * \todo
> * Shouldn't this function \b always return \c False when
> @@ -668,7 +684,7 @@ glXIsDirect(Display * dpy, GLXContext gc_user)
> #ifdef GLX_USE_APPLEGL /* TODO: indirect on darwin */
> return False;
> #else
> - return __glXIsDirect(dpy, gc->xid);
> + return __glXIsDirect(dpy, gc->xid, NULL);
> #endif
> }
>
> @@ -1428,7 +1444,7 @@ glXImportContextEXT(Display *dpy, GLXContextID
> contextID)
>return NULL;
> }
>
> - if (__glXIsDirect(dpy, contextID))
> + if (__glXIsDirect(dpy, contextID, NULL))
>return NULL;
>
> opcode = __glXSetupForCommand(dpy);
> --
> 2.17.0
OIther than the two points
[Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()
st_renderbuffer_delete() can segfault if we get a non-NULL context
pointer but if the st_context is NULL:
Thread 1 "Xwayland" received signal SIGSEGV, Segmentation fault.
in st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
241 pipe_surface_release(st->pipe, >surface_srgb);
(gdb) bt
#0 st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
#1 _mesa_reference_renderbuffer_ () at main/renderbuffer.c:212
#2 _mesa_reference_renderbuffer () at main/renderbuffer.h:72
#3 _mesa_free_framebuffer_data (0) at main/framebuffer.c:229
#4 _mesa_destroy_framebuffer () at main/framebuffer.c:207
#5 _mesa_reference_framebuffer_ () at main/framebuffer.c:265
#6 _mesa_reference_framebuffer () at main/framebuffer.h:63
#7 _mesa_free_context_data () at main/context.c:1326
#8 st_destroy_context () at state_tracker/st_context.c:653
#9 dri_destroy_context () at dri_context.c:239
#10 driDestroyContext () at dri_util.c:524
#11 __glXDRIcontextDestroy () at glxdriswrast.c:132
#12 __glXFreeContext () at glxext.c:190
#13 ContextGone () at glxext.c:82
#14 doFreeResource () at resource.c:880
#15 FreeResourceByType () at resource.c:941
#16 __glXDisp_DestroyContext () at glxcmds.c:437
#17 dispatch_DestroyContext () at vnd_dispatch_stubs.c:82
#18 Dispatch () at dispatch.c:478
#19 dix_main () at main.c:276
#20 __libc_start_main () from /lib64/libc.so.6
#21 _start () at glxcmds.c:125
(gdb) p st
$1 = (struct st_context *) 0x0
Check for a non-NULL st_context pointer as well to avoid the crash.
Bugzilla: https://bugzilla.redhat.com/1611140
Signed-off-by: Olivier Fourdan
---
Note: This fixes several bug reported downstream, like:
https://bugzilla.redhat.com/1611140
https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/1762971
https://bugs.launchpad.net/ubuntu/+source/mesa/+bug/1754693
etc.
I don't know what this client actually does, but whatever it is it should
not crash Xwayland because of Mesa...
I tested this fix against the given reproducer (run snap on Wayland/Xwayland)
and it works.
src/mesa/state_tracker/st_cb_fbo.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/state_tracker/st_cb_fbo.c
b/src/mesa/state_tracker/st_cb_fbo.c
index 73414fdfa1..856d213b73 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -238,8 +238,10 @@ st_renderbuffer_delete(struct gl_context *ctx, struct
gl_renderbuffer *rb)
struct st_renderbuffer *strb = st_renderbuffer(rb);
if (ctx) {
struct st_context *st = st_context(ctx);
- pipe_surface_release(st->pipe, >surface_srgb);
- pipe_surface_release(st->pipe, >surface_linear);
+ if (st) {
+ pipe_surface_release(st->pipe, >surface_srgb);
+ pipe_surface_release(st->pipe, >surface_linear);
+ }
strb->surface = NULL;
}
pipe_resource_reference(>texture, NULL);
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] dri3: For 1.2, use root window instead of pixmap drawable
Hi,
On Thu, 26 Jul 2018 at 19:53, Eric Anholt wrote:
>
> Olivier Fourdan writes:
>
> > get_supported_modifiers() and pixmap_from_buffers() requests both
> > expect a window as drawable, passing a pixmap will fail as the Xserver
> > will fail to match the given drawable to a window.
> >
> > That leads to dri3_alloc_render_buffer() to return NULL and breaks
> > rendering when using GLX_DOUBLEBUFFER on pixmaps.
> >
> > Query the root window of the pixmap on first init, and use the root
> > window instead of the pixmap drawable for get_supported_modifiers()
> > and pixmap_from_buffers().
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107117
> > Fixes: 069fdd5 ("egl/x11: Support DRI3 v1.1")
> > Signed-off-by: Olivier Fourdan
>
> Looks great!
>
> Reviewed-by: Eric Anholt
Thanks both Daniel and Eric for the reviews!
If I may, could someone push that fix for me, I don't think I have
commit rights in Mesa...
(It's a regression affecting Mesa 18.1 with Xserver 1.20, would be
great to have it fixed)
Cheers,
Olivier
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3] dri3: For 1.2, use root window instead of pixmap drawable
get_supported_modifiers() and pixmap_from_buffers() requests both
expect a window as drawable, passing a pixmap will fail as the Xserver
will fail to match the given drawable to a window.
That leads to dri3_alloc_render_buffer() to return NULL and breaks
rendering when using GLX_DOUBLEBUFFER on pixmaps.
Query the root window of the pixmap on first init, and use the root
window instead of the pixmap drawable for get_supported_modifiers()
and pixmap_from_buffers().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107117
Fixes: 069fdd5 ("egl/x11: Support DRI3 v1.1")
Signed-off-by: Olivier Fourdan
---
v2: Use the root window instead of reverting to pixmap_from_buffer()
as suggested by Eric.
v3: Much simpler patch, no need to issue an xcb_get_geometry() twice,
we already did it in dri3_update_drawable(), just save the root window
from there... Sorry for the noise!
src/loader/loader_dri3_helper.c | 12 +---
src/loader/loader_dri3_helper.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index f0ff2f07bd..b8eb87f5aa 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -1149,7 +1149,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable
*draw, unsigned int format,
uint32_t count = 0;
mod_cookie = xcb_dri3_get_supported_modifiers(draw->conn,
- draw->drawable,
+ draw->window,
depth, buffer->cpp * 8);
mod_reply = xcb_dri3_get_supported_modifiers_reply(draw->conn,
mod_cookie,
@@ -1281,7 +1281,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable
*draw, unsigned int format,
buffer->modifier != DRM_FORMAT_MOD_INVALID) {
xcb_dri3_pixmap_from_buffers(draw->conn,
pixmap,
- draw->drawable,
+ draw->window,
num_planes,
width, height,
buffer->strides[0], buffer->offsets[0],
@@ -1357,6 +1357,7 @@ dri3_update_drawable(__DRIdrawable *driDrawable,
xcb_generic_error_t *error;
xcb_present_query_capabilities_cookie_t present_capabilities_cookie;
xcb_present_query_capabilities_reply_t*present_capabilities_reply;
+ xcb_window_t root_win;
draw->first_init = false;
@@ -1394,11 +1395,11 @@ dri3_update_drawable(__DRIdrawable *driDrawable,
mtx_unlock(>mtx);
return false;
}
-
draw->width = geom_reply->width;
draw->height = geom_reply->height;
draw->depth = geom_reply->depth;
draw->vtable->set_drawable_size(draw, draw->width, draw->height);
+ root_win = geom_reply->root;
free(geom_reply);
@@ -1432,6 +1433,11 @@ dri3_update_drawable(__DRIdrawable *driDrawable,
xcb_unregister_for_special_event(draw->conn, draw->special_event);
draw->special_event = NULL;
}
+
+ if (draw->is_pixmap)
+ draw->window = root_win;
+ else
+ draw->window = draw->drawable;
}
dri3_flush_present_events(draw);
mtx_unlock(>mtx);
diff --git a/src/loader/loader_dri3_helper.h b/src/loader/loader_dri3_helper.h
index 7e3d82947b..51d000343d 100644
--- a/src/loader/loader_dri3_helper.h
+++ b/src/loader/loader_dri3_helper.h
@@ -114,6 +114,7 @@ struct loader_dri3_drawable {
xcb_connection_t *conn;
__DRIdrawable *dri_drawable;
xcb_drawable_t drawable;
+ xcb_window_t window;
int width;
int height;
int depth;
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] dri3: For 1.2, use root window instead of pixmap drawable
get_supported_modifiers() and pixmap_from_buffers() requests both
expect a window as drawable, passing a pixmap will fail as the Xserver
will fail to match the given drawable to a window.
That leads to dri3_alloc_render_buffer() to return NULL and breaks
rendering when using GLX_DOUBLEBUFFER on pixmaps.
Query the root window of the pixmap on first init, and use the root
window instead of the pixmap drawable for get_supported_modifiers()
and pixmap_from_buffers().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107117
Fixes: 069fdd5 ("egl/x11: Support DRI3 v1.1")
Signed-off-by: Olivier Fourdan
---
v2: Use the root window instead of reverting to pixmap_from_buffer()
as suggested by Eric.
src/loader/loader_dri3_helper.c | 31 +--
src/loader/loader_dri3_helper.h | 1 +
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index f0ff2f07bd..c7ad67cd04 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -1091,6 +1091,21 @@ has_supported_modifier(struct loader_dri3_drawable
*draw, unsigned int format,
free(supported_modifiers);
return found;
}
+
+static xcb_window_t
+get_root_window (struct loader_dri3_drawable *draw)
+{
+ xcb_get_geometry_cookie_t cookie;
+ xcb_get_geometry_reply_t *reply;
+ xcb_window_t window = draw->drawable;
+
+ cookie = xcb_get_geometry(draw->conn, draw->drawable);
+ if ((reply = xcb_get_geometry_reply(draw->conn, cookie, NULL)))
+ window = reply->root;
+ free(reply);
+
+ return window;
+}
#endif
/** loader_dri3_alloc_render_buffer
@@ -1149,7 +1164,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable
*draw, unsigned int format,
uint32_t count = 0;
mod_cookie = xcb_dri3_get_supported_modifiers(draw->conn,
- draw->drawable,
+ draw->window,
depth, buffer->cpp * 8);
mod_reply = xcb_dri3_get_supported_modifiers_reply(draw->conn,
mod_cookie,
@@ -1281,7 +1296,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable
*draw, unsigned int format,
buffer->modifier != DRM_FORMAT_MOD_INVALID) {
xcb_dri3_pixmap_from_buffers(draw->conn,
pixmap,
- draw->drawable,
+ draw->window,
num_planes,
width, height,
buffer->strides[0], buffer->offsets[0],
@@ -1432,7 +1447,19 @@ dri3_update_drawable(__DRIdrawable *driDrawable,
xcb_unregister_for_special_event(draw->conn, draw->special_event);
draw->special_event = NULL;
}
+
+#ifdef HAVE_DRI3_MODIFIERS
+ /* get_supported_modifiers() and pixmap_from_buffers() expect a window,
+ * not just a drawable, and will fail if we pass the pixmap drawable.
+ * So, in case of a pixmap, use the root window instead.
+ */
+ if (draw->is_pixmap)
+ draw->window = get_root_window(draw);
+ else
+#endif
+ draw->window = draw->drawable;
}
+
dri3_flush_present_events(draw);
mtx_unlock(>mtx);
return true;
diff --git a/src/loader/loader_dri3_helper.h b/src/loader/loader_dri3_helper.h
index 7e3d82947b..51d000343d 100644
--- a/src/loader/loader_dri3_helper.h
+++ b/src/loader/loader_dri3_helper.h
@@ -114,6 +114,7 @@ struct loader_dri3_drawable {
xcb_connection_t *conn;
__DRIdrawable *dri_drawable;
xcb_drawable_t drawable;
+ xcb_window_t window;
int width;
int height;
int depth;
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] dri3: Do not get supported modifiers on pixmaps
get_supported_modifiers() expects a window as drawable, passing a pixmap
will fail as the Xserver will fail to match the id to a window.
That leads to dri3_alloc_render_buffer() to return NULL and breaks
rendering when using GLX_DOUBLEBUFFER.
Check if dealing with pixmap in dri3_alloc_render_buffer() in which case
avoid using get_supported_modifiers() and fallback to the good old
pixmap_from_buffer() method as before.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107117
Fixes: 069fdd5 ("egl/x11: Support DRI3 v1.1")
Signed-off-by: Olivier Fourdan
---
src/loader/loader_dri3_helper.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index f0ff2f07bd..83b7c66a44 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -1139,6 +1139,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable
*draw, unsigned int format,
if (!draw->is_different_gpu) {
#ifdef HAVE_DRI3_MODIFIERS
if (draw->multiplanes_available &&
+ !draw->is_pixmap &&
draw->ext->image->base.version >= 15 &&
draw->ext->image->queryDmaBufModifiers &&
draw->ext->image->createImageWithModifiers) {
@@ -1278,6 +1279,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable
*draw, unsigned int format,
pixmap = xcb_generate_id(draw->conn);
#ifdef HAVE_DRI3_MODIFIERS
if (draw->multiplanes_available &&
+ !draw->is_pixmap &&
buffer->modifier != DRM_FORMAT_MOD_INVALID) {
xcb_dri3_pixmap_from_buffers(draw->conn,
pixmap,
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi, On Tue, 10 Jul 2018 at 18:56, Jason Ekstrand wrote: > > Ugh... not so good. According to Oliver on the bug, this just make the > assert go away and doesn't actually fix anything. Likely this is needed but > not sufficient. Well, maybe not even needed, at least in my case I don't hit that assert() with the current Mesa code (from either master and 18.1), I've just seen that happening with past commits while doing the bisection in git... So adding this now wouldn't help with bisection (again, in my case, not sure about others). All I meant pointing at this assert() failure is that it breaks the bisection in git as I am unable to tell if the rendering is correct or not as it makes the test program abort. Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
