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

Move Fullscreen enum into platforms #3901

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Sep 8, 2024

This is a bit of duplication, but needed if we intend to split the backends into separate crates, since we need to convert between the root crate's VideoModeHandle and MonitorHandles, and the platform's types.

@madsmtm madsmtm added the S - api Design and usability label Sep 8, 2024
@madsmtm madsmtm force-pushed the madsmtm/refactor-fullscreen branch from 7f5f677 to eec1774 Compare September 8, 2024 10:48
Needed if we intend to split these into separate crates.
@madsmtm madsmtm force-pushed the madsmtm/refactor-fullscreen branch from eec1774 to 82b5aa1 Compare September 8, 2024 10:59
Comment on lines +1009 to +1010
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Fullscreen {
Copy link
Member

Choose a reason for hiding this comment

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

How does this work if the functions implementing the API for this (that take or return the Fullscreen type) are defined by a core trait?

In any case, on Android we can remove this entirely because it's "not implemented" and not entirely sane to implement.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Given that the general movement recently to completely remove platform_impl::Type things, I'm not sure what to do about this one...

@kchibisov
Copy link
Member

since we need to convert between the root crate's VideoModeHandle and MonitorHandles, and the platform's types.

We don't actually want to convert though, we want to use the exact same thing on the top type as well, like for Window right now, we don't have any conversion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S - api Design and usability
Development

Successfully merging this pull request may close these issues.

3 participants