Bug 111140

Reproduced.
From the pattern, it looks like it could be compression/modifier issue.
I wonder if mutter disables it to take the screenshot and then somewhere the state gets messed up.

Created attachment 144798 [details]
Nested shell repro

Reproduced in nested mode. That rules out any display stuff.

hi, I also able to reproduce this issue. Unfortunately that's not regression so couldn't find "workable" mesa version.

additional info found - works fine with iris driver (system 19.1.1 mesa, manjaro)

On my side I can see that one of the image is switched from using modifier to not using the anymore (likely when the image is resolved to get the data).
But chromium likely renders to that buffer still in compressed mode, which is why I think this leads to the issue.

I'm guessing the texture/miptree needs to be destroyed, but that's not something the driver can necessarily do on its own, because his view of the texture is consistent.

Denis : do you know if iris is using modifiers in that case?

I wonder if the other drivers don't see the issue because they're not using them.

Right mutter/cogl ends up calling glGetTexImage() which transfer one of the image of the chromium set of images into a non compressed texture.

I need to read more GL spec to figure out what should actually happen on the driver side.
One way mutter could workaround this would be to use the get_image_via_offscreen() path in meta_shaped_texture_get_image() so that the state of the image is not affected.

>Denis : do you know if iris is using modifiers in that case?

Not sure how to answer correctly on this question, but you lead me to check and compare these modifiers in iris and i965 => /src/mesa/drivers/dri/i965/intel_screen.c

Commenting this fixes the issue for i965 
>{ .modifier = I915_FORMAT_MOD_Y_TILED_CCS , .since_gen = 9 }

Maybe this may help you.

So looking at the way mutter bind the dmabuf shared with chromium to a 2D texture, it does so through cogl_egl_texture_2d_new_from_image().
That function seems to rely on GL_OES_EGL_image_external extension : https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import.txt

The extension has this bit in the spec :


    There is no support for most of the functions that manipulate
    other texture targets (e.g. you cannot use gl*Tex*Image*() functions with
    TEXTURE_EXTERNAL_OES).

That makes me think what mutter is doing is illegal.

(In reply to Lionel Landwerlin from comment #8)
> So looking at the way mutter bind the dmabuf shared with chromium to a 2D
> texture, it does so through cogl_egl_texture_2d_new_from_image().
> That function seems to rely on GL_OES_EGL_image_external extension :
> https://www.khronos.org/registry/EGL/extensions/EXT/
> EGL_EXT_image_dma_buf_import.txt
> 
> The extension has this bit in the spec :
> 
> 
>     There is no support for most of the functions that manipulate
>     other texture targets (e.g. you cannot use gl*Tex*Image*() functions with
>     TEXTURE_EXTERNAL_OES).
> 
> That makes me think what mutter is doing is illegal.

Does gl*Tex*Image() here imply glGetTexImage() too then you mean? Does reading from it mean "manipulate"?

Either way, I created https://gitlab.gnome.org/GNOME/mutter/merge_requests/687 that should avoid reading directly from it. It didn't reproduce with or without it, so if anyone that can reproduce it can give it a test I'd appreciate it.

FTR, this is the same as https://gitlab.freedesktop.org/xorg/xserver/issues/545

(In reply to Jonas Ådahl from comment #9)
> Either way, I created
> https://gitlab.gnome.org/GNOME/mutter/merge_requests/687 that should avoid
> reading directly from it. It didn't reproduce with or without it, so if
> anyone that can reproduce it can give it a test I'd appreciate it.

I tested and confirm https://gitlab.gnome.org/GNOME/mutter/merge_requests/687 fixes both https://gitlab.freedesktop.org/xorg/xserver/issues/545 *and* the screenshot issue for me.

(In reply to Lionel Landwerlin from comment #5)
> Denis : do you know if iris is using modifiers in that case?

iris does not use the Y+CCS modifier yet.  It uses X/Y/linear modifiers.  @jljusten is working on adding Y+CCS support.  So it probably works there simply because we're not using any modifiers with compression.

(In reply to Kenneth Graunke from comment #12)
> (In reply to Lionel Landwerlin from comment #5)
> > Denis : do you know if iris is using modifiers in that case?
> 
> iris does not use the Y+CCS modifier yet.  It uses X/Y/linear modifiers. 
> @jljusten is working on adding Y+CCS support.  So it probably works there
> simply because we're not using any modifiers with compression.

Thanks, I was going to test this tomorrow :)

(In reply to Jonas Ådahl from comment #9)
> (In reply to Lionel Landwerlin from comment #8)
> > So looking at the way mutter bind the dmabuf shared with chromium to a 2D
> > texture, it does so through cogl_egl_texture_2d_new_from_image().
> > That function seems to rely on GL_OES_EGL_image_external extension :
> > https://www.khronos.org/registry/EGL/extensions/EXT/
> > EGL_EXT_image_dma_buf_import.txt
> > 
> > The extension has this bit in the spec :
> > 
> > 
> >     There is no support for most of the functions that manipulate
> >     other texture targets (e.g. you cannot use gl*Tex*Image*() functions with
> >     TEXTURE_EXTERNAL_OES).

Duh! I pointed to the wrong spec...
Here is the good one : https://www.khronos.org/registry/OpenGL/extensions/OES/OES_EGL_image_external.txt

> > 
> > That makes me think what mutter is doing is illegal.
> 
> Does gl*Tex*Image() here imply glGetTexImage() too then you mean? Does
> reading from it mean "manipulate"?

My reading of manipulate goes for both read/write operations.
Essentially anything that could alter the metadata associated with the texture (size, format and those hidden compression settings).

I agree that the spec is a bit vague on this point and I'm not 100% sure what was the intent.
I could image we could allow glGet*Image* in the driver but that would essentially be replicating what you would otherwise do in the application (e.g. blit to an offscreen buffer and read that one back).

The problem is that the driver does a bunch of tracking on what you do with the texture and puts it in the most sensible disposition for what it's used.
Once that goes out of sync with what the other processes have, we're in trouble.

> 
> Either way, I created
> https://gitlab.gnome.org/GNOME/mutter/merge_requests/687 that should avoid
> reading directly from it. It didn't reproduce with or without it, so if
> anyone that can reproduce it can give it a test I'd appreciate it.

That fixes the problem on my side too. Thanks!

(In reply to Lionel Landwerlin from comment #14)

...

> I could image we could allow glGet*Image* in the driver but that would
> essentially be replicating what you would otherwise do in the application
> (e.g. blit to an offscreen buffer and read that one back).

FWIW, OES_EGL_image_external (used with EGLStreams) has similar limitations. Other than that, from a Wayland buffer point of view, there are other things that mandate drawing via an offscreen to get usable output (buffer transforms, viewports etc), so it's not like there shouldn't be paths in the application that handles these kind of situations either way.

> 
> The problem is that the driver does a bunch of tracking on what you do with
> the texture and puts it in the most sensible disposition for what it's used.
> Once that goes out of sync with what the other processes have, we're in
> trouble.

I don't think it's worth the trouble to let the driver handle if it would just complicate things.

> 
> > 
> > Either way, I created
> > https://gitlab.gnome.org/GNOME/mutter/merge_requests/687 that should avoid
> > reading directly from it. It didn't reproduce with or without it, so if
> > anyone that can reproduce it can give it a test I'd appreciate it.
> 
> That fixes the problem on my side too. Thanks!

Thanks for testing! Thanks to Olivier too :)

Fix landed in mutter, scheduled for release in version 3.33.4.

I would like to keep this open.
I've been tried to make this work with i965.
It's really not clear to me what we can and cannot do with texture bound to external images.

I'll be pretty much the same thing than what mutter did for fixing this bug.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/1819.