Skip to content

Fix gnome window screenshot #812

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

Merged
merged 4 commits into from
Apr 3, 2024
Merged

Fix gnome window screenshot #812

merged 4 commits into from
Apr 3, 2024

Conversation

jtaala
Copy link
Collaborator

@jtaala jtaala commented Apr 3, 2024

This PR fixes #811.

@jtaala jtaala linked an issue Apr 3, 2024 that may be closed by this pull request
@jtaala jtaala added the merging soon Label for PRs that are planned to be merged soon (usually within the next week) label Apr 3, 2024
@jtaala jtaala requested a review from lost-melody April 3, 2024 09:14
Copy link
Collaborator

@lost-melody lost-melody left a comment

Choose a reason for hiding this comment

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

Approved, thank you.

One thing I'm confused is that generally for cmp(a, b) we return -1 on (!a && b) and 1 on (a && !b) while here it returns 1 on (!a && b). But I guess it does not matter.

@jtaala
Copy link
Collaborator Author

jtaala commented Apr 3, 2024

Approved, thank you.

One thing I'm confused is that generally for cmp(a, b) we return -1 on (!a && b) and 1 on (a && !b) while here it returns 1 on (!a && b). But I guess it does not matter.

It's just a comparator, meaning we're sorting the "windows" that don't have a metaWindow first - it doesn't really matter in this case, but we're aligning to the approach below it:

    if (ia === -1) {
        return -1;
    }
    if (ib === -1) {
        return 1;
    }

which is just ordering the a before b if there no window in that space. It's just for consistency really.

@jtaala jtaala merged commit 7db476d into develop Apr 3, 2024
@jtaala jtaala deleted the fix-gnome-window-screenshot branch April 3, 2024 09:47
@jtaala
Copy link
Collaborator Author

jtaala commented Apr 3, 2024

One thing I'm confused is that generally for cmp(a, b) we return -1 on (!a && b) and 1 on (a && !b) while here it returns 1 on (!a && b)

Ah, apologies, you're right, was looking at the code again - for consistency we should return -1 on !aw. Doesn't essentially matter in this case, but as you pointed out, can be confusing. I'll make that change for consistency and add it to the next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merging soon Label for PRs that are planned to be merged soon (usually within the next week)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Capturing window is broken in screenshot view caused by a js error
2 participants