`GlesRenderer::import_shm_buffer` needs synchronization for shared EGL contexts in different threads
@Drakulix and I noticed some apparent issues here debugging https://github.com/pop-os/cosmic-comp/issues/1078. I'm not sure they fully explain what's going on there, but it does seem like something that needs to be fixed.
- If two threads call
GlesRenderer::import_shm_bufferwith shared EGL contexts (EGLContext::new_shared), it looks like both could try to draw to the same GLES texture. There should be some kind of synchronization to prevent parallel writes (and to avoid unncessary copy of the same damage regions from the shm buffer). - If one context renders to the GLES texture, and another context tries to read it, it looks like that also needs synchronization, and this doesn't happen implicitly.
- See section "C.2 Propagating Changes to Objects" in https://registry.khronos.org/OpenGL/specs/es/2.0/es_full_spec_2.0.pdf
- Looks like we need to wait on the fence (or use glFinish) before a
glBindTexturein a different context
- Looks like we need to wait on the fence (or use glFinish) before a
- See section "C.2 Propagating Changes to Objects" in https://registry.khronos.org/OpenGL/specs/es/2.0/es_full_spec_2.0.pdf
Not using shared EGL contexts, or importing a seperate GLES texture for each shared context, could fix this. But it would be good to avoid unnecessary duplication of copies from shm buffers to GPU textures if possible, even though it's not normally the most important thing.
(I guess if there are major changes to the logic for mirroring shm buffers to GPU buffers, we should also consider how best to manage the reverse, which is needed for ext-image-copy-capture-v1.)
To synchronize, we should presumably use eglCreateSync after the draw calls, then another renderer using a shared context can eglWaitSync on the sync point stored alongside the texture ide.
To avoid copying the same damage multiple times, maybe import_shm_buffer shouldn't be taking damage rectangles, but should handle the damage since the last commit internally. (So it doesn't get the same damage rectangles in two different threads without knowing it needs to copy the same thing.)
Then if there's any additional damage, it can wait on the existing sync point (if any), draw the addition damage regions, and then export a new one.
I guess something should also block rendering to a texture when it's still being read elsewhere? Though ultimately that shouldn't happen with correct clients, since they won't be modifying a buffer and committing new damage until the buffer is released, at which point nothing in the compositor should be using it. So maybe not needed as long as it's not undefined behavior (in the sense of being unsound).