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

Adds "Clip Region" clipping to ConsoleDriver #2606

Closed
wants to merge 26 commits into from

Conversation

tig
Copy link
Collaborator

@tig tig commented May 7, 2023

As part of the new View architecture, we will need a more sophisticated clipping mechanism than just a single rectangle.

This PR adds such a thing, retaining the previous ConsoleDriver.Clip functionality.

The clip region is exposed via ClipRegion which is a List<Rect>. IsInClipRegion() (private) tests whether a coordinate is within the region or not.

ConsoleDriver.IsValidClip has been renamed to IsValidLocation (it was mis-named before because it also validates the coordinates are within the screen bounds).

Also:

  • cleans up ConsoleDriver code

Other than ensuring I didn't break anything and adding unit tests, I've not exercised this functionality for realz yet. That will come soon, hence this is still a draft PR.

@tig tig marked this pull request as ready for review May 8, 2023 06:01
@tig tig requested a review from migueldeicaza as a code owner May 8, 2023 06:01
@tig tig marked this pull request as draft May 8, 2023 18:19
@tig
Copy link
Collaborator Author

tig commented May 8, 2023

Still a draft.

I've discovered a bunch of things in all the drivers that need to be tweaked to make this work. No way am I going to do it all 4 times. Instead, I'm going to work on #666 (which I started in the code in this PR) as a separate PR first. It's just crazy how much duplicate/similar code there are in all 4 drivers. Much can be moved into ConsoleDriver in a way that makes unit testing it all much better.

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

1 participant