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

Web: monitor API improvements #3847

Merged
merged 2 commits into from
Aug 4, 2024
Merged

Conversation

daxpedda
Copy link
Member

@daxpedda daxpedda commented Aug 4, 2024

  • Improved the documentation to point users into the right direction from all kinds of methods and types.
  • De-duplicated some code and added more comments.
  • Implement an ID system to correctly and efficiently implement Eq, Hash, Ord, PartialEq and PartialOrd.
  • Fixed screen locking support being cached thread local, ergo calling from a different thread would require to make the check again, not fulfilling its purpose as a fast-path at all.

Follow-up to #3801.

@daxpedda daxpedda force-pushed the web-monitor-2 branch 2 times, most recently from 56c1b2e to 2f0772b Compare August 4, 2024 13:12
Necessary to correctly and efficiently implement `Eq`, `Hash`, `Ord`, `PartialEq` and `PartialOrd`.
@daxpedda
Copy link
Member Author

daxpedda commented Aug 4, 2024

I did some thinking about the API complexity of this implementation.
There are basically two different MonitorHandle types, but they are the same Rust type. Users can query which type it is with is_detailed(), but that's all not exactly ideal.

I think what could be done about that in the future is to split up the orientation and screen locking stuff away from MonitorHandle entirely, that way there won't be a use for MonitorHandle on Web at all unless its the "detailed" type.
However, this requires a new API surface that we have yet to think about and implement, which requires some cross-platform coordination as well.

In any case, its some food for thought.

@daxpedda daxpedda merged commit 586255a into rust-windowing:master Aug 4, 2024
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants