Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GlesRenderer::import_shm_buffer needs synchronization for shared EGL contexts in different threads #1615

Open
ids1024 opened this issue Dec 19, 2024 · 1 comment

Comments

@ids1024
Copy link
Member

ids1024 commented Dec 19, 2024

@Drakulix and I noticed some apparent issues here debugging pop-os/cosmic-comp#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_buffer with 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.

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.)

@ids1024
Copy link
Member Author

ids1024 commented Dec 19, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant