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

Crosshairs #95

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Crosshairs #95

wants to merge 7 commits into from

Conversation

tmccombs
Copy link
Collaborator

This is based on #56, but has been rebased on master, and fixed some issues I found while testing (mostly around outputs that don't start at the origin).

@tmccombs tmccombs requested a review from emersion September 26, 2021 20:26
Copy link

@ammgws ammgws left a comment

Choose a reason for hiding this comment

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

This is much better now! Something I always wanted to fix.

@ammgws ammgws mentioned this pull request Sep 28, 2021
@ammgws
Copy link

ammgws commented Oct 8, 2021

When moving the crosshairs between monitors, sometimes this happens:

crosshairs.mp4

@tmccombs
Copy link
Collaborator Author

When moving the crosshairs between monitors, sometimes this happens:

That's unfortunate. I'm not entirely sure why that happens. Reverting 0eeba3a fixes, it. That does mean we mark the outputs as dirty more often than necessary. I'd have to investigate more to see if there is a better way to handle that.

@ammgws
Copy link

ammgws commented Oct 16, 2021

By the way do you have merge permissions or does this still require @emersion's approval first?

@tmccombs
Copy link
Collaborator Author

I have merge permissions, but I would like it to be reviewed by someone else before merging.

@tmccombs tmccombs linked an issue Oct 26, 2021 that may be closed by this pull request
@ammgws
Copy link

ammgws commented Jan 28, 2022

Reverting 0eeba3a fixes, it. That does mean we mark the outputs as dirty more often than necessary. I'd have to investigate more to see if there is a better way to handle that.

Any luck with this? Or perhaps this could still be shipped since it doesn't seem like a showstopper, especially since crosshairs are not enabled by default.

@tmccombs
Copy link
Collaborator Author

No, I haven't investigated it any more yet. (But the revert is on this branch).

@tmccombs
Copy link
Collaborator Author

tmccombs commented Feb 1, 2022

Ok, I think I've got better checks on when to mark outputs as dirty now. I was just missing a condition in an if statements one of the places where seat_set_outputs_dirty was called.

ammgws added a commit to ammgws/dotfiles that referenced this pull request Feb 24, 2022
@ammgws
Copy link

ammgws commented Mar 9, 2022

This has been working well for me, thanks!

@tmccombs
Copy link
Collaborator Author

@emersion any objection to merging this?

@ammgws
Copy link

ammgws commented Dec 16, 2023

@tmccombs any luck?

tmccombs and others added 7 commits December 28, 2023 23:38
And have a seperate header file too.
…oordinates

This simplifies rendering, since we don't have to subtract the origin of
the output from the selection coordinates for each operation.

This also fixes the coordinates for the crosshairs drawn on the screen.
Previously, the calls to draw the crosshairs used a mix of both
coordinate systems, which would work for a single output, but not for
many multi-output setups.
The selection color defaults to being transparent, which is not very
visible if used over a light background, at least with the default
background color. I think using the border color is more useful.
In man page and help output.
@tmccombs
Copy link
Collaborator Author

I've resolved the conflicts.

@alexmaras
Copy link

I've been using this locally for a while from a manual build and I've been super happy with it. Thanks for putting it together @tmccombs!

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.

Full screen crosshairs
3 participants