Destroy layer shell surfaces instead of layer shell for freeze-feat #115
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
While it doesn't throw error anymore, the wayshot process hangs until manually killed, and the screenshot itself isn't copied into |
|
Reading the protocol, it seems that unmapping the layer shell surface by committing a null buffer to it is necessary before destruction. Have tested this on sway, river and Hyprland. Can you test this too @Gigas002 ? |
|
Works great now 👍 |
|
@CheerfulPianissimo Would you like me to review and merge this PR? |
Shinyzenith
reviewed
May 1, 2024
libwayshot/src/lib.rs
Outdated
| @@ -452,6 +452,8 @@ impl WayshotConnection { | |||
| } | |||
| }; | |||
|
|
|||
| let mut layer_shell_surfaces = Vec::new(); | |||
There was a problem hiding this comment.
We know the wl_output count right? Can we just make a static array to reduce allocations?
There was a problem hiding this comment.
That should be possible, I'll look into it.
There was a problem hiding this comment.
Actually, no, we don't know the wl_output count at compile time so I can't see how we could use a simple static array. What we can do is use Vec::with_capacity to init the vec with the number of output frames, is this what you meant?
|
Yes, freeze feat with clipboard is unusable on some compositors without this. Up to you if you want it merge it as it is or want to wait fot the static allocation change @Shinyzenith . |
|
What do you mean by static allocation change? Do you have any pr/issue/code sample i can look at? |
|
This chnage you requested @Shinyzenith #115 (comment) |
|
Ah my apologies, I am forgetful. Would be ideal to get it done first. Thank you for your help sooraj, much appreciated. |
Shinyzenith
reviewed
May 28, 2024
| debug!("Unmapping and destroying layer shell surfaces."); | ||
| for (surface, layer_shell_surface) in layer_shell_surfaces.iter() { | ||
| surface.attach(None, 0, 0); | ||
| surface.commit(); //unmap surface by committing a null buffer |
There was a problem hiding this comment.
Note: I am approving and merging the PR for now but I am unsure if this is correct
Shinyzenith
approved these changes
May 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempts to solve the freeze feature crashing sporadically as described in #109 (comment)
#109 (comment) :
@Gigas002 can you test this fix?