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: don't group floating windows unless one is fully contained in the other #2527

Merged
merged 1 commit into from May 11, 2024

Conversation

sid-6581
Copy link
Contributor

@sid-6581 sid-6581 commented May 7, 2024

What kind of change does this PR introduce?

  • Fix

Did this PR introduce a breaking change?

  • No

This is an attempt to make it less likely that floating windows with consecutive z-indexes are rendered together if they shouldn't be rendered together, like the first screenshot in #2502.

Instead of checking if the window rects intersect, I check if one is contained in the other. This isn't foolproof, because we could still have windows with consecutive indexes that should not be rendered together (like the first screenshot in #2502, but in a bigger directory), so the whole window grouping thing needs a lot more thought, and should probably be able to be disabled entirely by an option.

Still, I can't think of any time it would make sense to render two windows with consecutive z-indexes together if one isn't fully contained in the other.

@sid-6581 sid-6581 changed the title fix: don't group floating windows unless they fully overlap fix: don't group floating windows unless one is fully contained in the other May 7, 2024
Copy link

github-actions bot commented May 7, 2024

Test Results

  6 files  ±0    6 suites  ±0   14s ⏱️ -4s
110 tests ±0  110 ✅ ±0  0 💤 ±0  0 ❌ ±0 
644 runs  ±0  644 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a7e24de. ± Comparison against base commit 5efb713.

Copy link
Member

@fredizzimo fredizzimo left a comment

Choose a reason for hiding this comment

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

I think this looks good, but it's better that @Kethku double checks, since she knows the details better.

@fredizzimo fredizzimo merged commit 47bc75a into neovide:main May 11, 2024
12 checks passed
@fredizzimo
Copy link
Member

Thanks, I decided to merge this, since it's strictly better than the previous behaviour, although not perfect.

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.

None yet

2 participants