-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Really fix floating window border resize problems #5299
Conversation
I can confirm this also fixes the issue I had. |
Let me know when this is ready for review. |
Thanks for the reminder. Ready for review now. |
// Determine new width/height, and accommodate for floating min/max values | ||
double width = e->ref_width + grow_width; | ||
double height = e->ref_height + grow_height; | ||
int min_width, max_width, min_height, max_height; | ||
floating_calculate_constraints(&min_width, &max_width, | ||
&min_height, &max_height); | ||
width = fmax(min_width, fmin(width, max_width)); | ||
height = fmax(min_height, fmin(height, max_height)); | ||
width = fmax(min_width + border_width, fmin(width, max_width)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably take border_width
into account for the max too? Something like max_width - border_width
? We should also be careful that the result is > 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this PR as-is. I've created a follow-up issue: #5337
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Same problem, different approach.
Fixes: #5250
I've done testing of all combinations of settings and runtime border changes with resize I could come up with. So far I'm unable to see it brake. More testing is welcome.
@David96 : please test this instead #5292, it should resolve your problems.