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

Fix damage-ring bounds not being set when unplugging -> plugging in monitor #7530

Merged
merged 4 commits into from
Apr 6, 2023

Conversation

ErikReider
Copy link
Contributor

#7524 was a partial fix. Seems like this is still an issue when unplugging and plugging the monitor back in.

I'm not sure if it's a good fix due to the update_output_manager_config function being called from other places though.

ErikReider referenced this pull request Mar 31, 2023
Otherwise the initial bounds would be `INT_MAX` until `handle_mode` or `handle_commit` is called :)
@ErikReider
Copy link
Contributor Author

According to @quoing, this fixes #7528. Should I undo the changes in ac1ed63 and use this instead?

@emersion
Copy link
Member

emersion commented Apr 5, 2023

Does it fix the bug to add | WLR_OUTPUT_STATE_ENABLED to this line instead?

if (event->committed & (WLR_OUTPUT_STATE_MODE | WLR_OUTPUT_STATE_TRANSFORM)) {

@ErikReider
Copy link
Contributor Author

ErikReider commented Apr 5, 2023

Does it fix the bug to add | WLR_OUTPUT_STATE_ENABLED to this line instead?

It sadly doesn't

@emersion
Copy link
Member

emersion commented Apr 5, 2023

Well, I don't think update_output_manager_config() is a good place to do this. One would need to figure out which of the call sites fixes the issue.

@ErikReider
Copy link
Contributor Author

Well, I don't think update_output_manager_config() is a good place to do this. One would need to figure out which of the call sites fixes the issue.

I should have time to debug later tonight :)

@quoing
Copy link

quoing commented Apr 5, 2023

Could we rollback the original commit that actually broke this thing - or did it fix something else for majority of users? At this point I have to ignore any updates for sway-git or build sway manually (while rolling back the problematic commit).. I'm of course open to check fixes in separate devel branch in case something needs to be tested :)

@ErikReider
Copy link
Contributor Author

It's this call that's the caller.

update_output_manager_config(server);

Fixed in the latest commit

sway/desktop/output.c Outdated Show resolved Hide resolved
@ErikReider
Copy link
Contributor Author

ErikReider commented Apr 5, 2023

Could we rollback the original commit that actually broke this thing - or did it fix something else for majority of users?

I was experiencing integer overflows when trying to expand the damage when working on adding blur in SwayFX, so probably nothing that anyone else experienced :)

Does the latest commit still fix the issue @quoing?

sway/desktop/output.c Outdated Show resolved Hide resolved
Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@quoing
Copy link

quoing commented Apr 6, 2023

tested the commit and it fixes my issue #7528 ..

~ swaymsg -t get_version
sway version 1.9-dev-fa4aa79c (Apr  6 2023, branch 'makepkg')

@emersion emersion merged commit fa7b686 into swaywm:master Apr 6, 2023
@emersion
Copy link
Member

emersion commented Apr 6, 2023

Thanks!

@ErikReider ErikReider deleted the fix-damage-ring branch April 6, 2023 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants