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

subsurface_widget fixes #189

Merged
merged 8 commits into from
Dec 3, 2024
Merged

subsurface_widget fixes #189

merged 8 commits into from
Dec 3, 2024

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Nov 7, 2024

Fixes for issues I see in cosmic-workspaces, when trying to update to latest iced rebase.

Still not working for subsurfaces of drag surfaces...

@ids1024
Copy link
Member Author

ids1024 commented Nov 22, 2024

I guess drag surfaces are currently rendered offscreen, and passed to window_clipboard in Icon::Buffer. To use subsurfaces (which may be important for cosmic-workspaces), we'll want to use Icon::Surface.

The create_surface call used when rendering drag surfaces is actually creating another wgpu surface for the parent window, and calls vkCreateSwapchainKHR. Technically the Vulkan spec requires an VK_ERROR_NATIVE_WINDOW_IN_USE_KHR error when that is done, but Mesa doesn't send that in every case it should (https://gitlab.freedesktop.org/mesa/mesa/-/issues/7467). Presumably presenting with this swapchain would cause issues with the original window, but we don't.

But really screenshot() shouldn't require a surface at all, and that's easy to fix (iced-rs#2672).

This type is an `Arc`, so there's no major benefit to using a reference,
or a `Cow` to make it generic.

Drag surfaces currently require a `'static` `Element`, so this is needed
their.
Otherwise the `draw` call in response to `AboutToWait` causes this list
to grow, and eventually hit errors in `sendmsg` due to too many buffered
file descriptors from creating subsurfaces.

Probably the number of places `UserInterface::draw` are called should be
cleaned up. And it shouldn't be called in `AboutToWiat` like this. Some
improvements could probably be made in upstream iced.
This was previously needed, but should be redundant now that an empty
input region is set for the subsurface. With that, the subsurface should
never get input events, so mapping them isn't needed.
This argument was completely ignored by the wgpu renderer, and used only
for the `clip_mask` by the `tiny_skia` renderer. I believe creating a
new clip mask is correct.

This way it's possible to render offscreen without needing a surface.
`.create_surface()` actually created another wgpu surface for the parent
surface. Which ends up calling `vkCreateSwapchainKHR`. Looks like that
technically should have errored with
`VK_ERROR_NATIVE_WINDOW_IN_USE_KHR`, but Mesa doesn't produce that error
in every case where it should:
https://gitlab.freedesktop.org/mesa/mesa/-/issues/7467.
This is potentially useful.

Other properties are specific to shm/dmabuf, and not meaningful in
general.
In an earlier version, this took a `WlBuffer`, so the width/height
arguments were needed. But since using the same `wl_buffer` multiple
times conflicted with `release` handling we instead accept
`SubsurfaceBuffer`, which stores the same height/width.
@ids1024 ids1024 marked this pull request as ready for review December 2, 2024 23:41
@ids1024 ids1024 requested a review from a team December 2, 2024 23:41
@ids1024 ids1024 merged commit 5ec2ff7 into master Dec 3, 2024
4 of 32 checks passed
@ids1024 ids1024 deleted the subsurface branch December 3, 2024 15:52
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

Successfully merging this pull request may close these issues.

2 participants