Exposes Common Window Resolutions as an Enum.

[Sapein]

Objective

Implements: #14153

Solution

Exposes Common Window Resolutions that are likely to be used through an Enum (Resolution) that can be turned into a UVec2 and an iterable.

Testing

I've tested this briefly locally. I wrote a small test just to double check that things worked as expected and it passed.

[github-actions[bot]]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[MiniaczQ]

Should we add a Custom variant that can have arbitrary w/h values? It would be skipped from iter, but would expose a decent interface

[Sapein]

Looks good to me, we can add the Custom variant if necessary in another PR

Apologies for that btw. I was working on it while you were reviewing and I didn't notice.

I'm fine with adding a Custom variant as well, since it would be rather nice.

Also there are a few questions regarding the resolutions, as an example exposing a 4k variant would also probably be useful, and 360p can technically be one of two resolutions. That is, 480 x 360 which is 4:3 and 680 x 360 which is 16:9. I went with the latter because that's what I tend to use, and it works better with other resolutions (mostly having to do with scaling better to 720p and higher).

[Sapein]

I'm not against CommonScreenResolution, but something about it doesn't feel quiet right to me tbh, although I'm not against it as it is a better name than Resolution especially given context.

Also should it be Resolution or Resolutions? I'm actually not sure what the conventions are here regarding that...

[alice-i-cecile]

I quite like this: this is a utility that almost every game will want. To make this better:

1. More doc comments, explaining why this is useful.
2. Breadcrumbs to this in at least one example: probably window_settings or game_menu. I would really love to wire this up to a settings menu but that might be too much work for one PR.
3. CommonScreenResolution is the name I'd use: Resolution is way too broad for a little utility.
4. A doc test demonstrating how to use it.
5. A nice Display implementation for use in menus that shows "1920 x 1080" and so on.

Long-term I'd like to see more variants and an CommonAspectRatio enum with a CommonScreenResolution::aspect_ratio method but that's easily split off.

[Sapein]

I quite like this: this is a utility that almost every game will want. To make this better:

1. More doc comments, explaining why this is useful.

2. Breadcrumbs to this in at least one example: probably window_settings or game_menu. I would really love to wire this up to a settings menu but that might be too much work for one PR.

3. CommonScreenResolution is the name I'd use: Resolution is way too broad for a little utility.

4. A doc test demonstrating how to use it.

5. A nice Display implementation for use in menus that shows "1920 x 1080" and so on.

Long-term I'd like to see more variants and an CommonAspectRatio enum with a CommonScreenResolution::aspect_ratio method but that's easily split off.

CommonScreenResolution it is then. I'll be working down this list as I can. Also, how would you imagine the CommonAspectRatio enum to work? I'm mostly asking because of the aforementioned two 360p resolutions.

FWIW I've tended to implement things similar to this across a few games now, because it's useful to not have to try and remember the exact details about the resolution I'm using.

I'm also more than willing to add in more variants as well. I just started off with what I tend to use (plus 2k).

Also, should I change the usage of the default construction of WindowResolution to use the enum? It's currently set to 1280x720 by default. (Also I just realized this could be used to change the default WindowResolutions through some sort of setting or something if desired in the future)

[alice-i-cecile]

I was thinking:

enum CommonAspectRatio {
   FourToThree,
   SixteenToNine
}

We should also have a From<CommonScreenResolution> for WindowResoltion, now that you mention it. I'm fine to leave the default impl alone for now though.

[Sapein]

Fair enough. I'll go ahead and implement From<CommonScreenResolution> for WindowResolution as well, and leave the Default for now, it was mostly just something I thought of as a bit more of a side thing.

That would also be rather useful, although I'm not sure how it would entirely work, but it would help with cases where a resolution can be one of two -- dependent upon the Aspect Ratio.

(Technically 360p could be split into "360p" and "nHD" (the former being 4:3 and the latter being 16:9) as those are the Standard Names, according to Wikipedia. However, the class for both is 360p and I don't think a lot of people would recognize nHD tbqh.)

[janhohenheim]

Please do use it in WindowResoltion::default; many users click on the implementation of these to find out how they're setup. As such, I consider that a kind of documentation. Having CommonScreenResolution there will point them the way on how to cleanly setup their own resolution :)

[Sapein]

A nice Display implementation for use in menus that shows "1920 x 1080" and so on.

Would we want it to display "w x h" or something like the name ("360p", "720p", etc)? I've created an implementation that does the former based on this comment, as I imagine that it would probably be more useful for in game menus. But thought I would ask just to make sure.

[alice-i-cecile]

The former is what I've seen in game menus so I think it's better to stick to that.

[Sapein]

Breadcrumbs to this in at least one example: probably window_settings or game_menu. I would really love to wire this up to a settings menu but that might be too much work for one PR.

Could you elaborate on what you mean by Breadcrumbs to this, I presume you want me to either use it in one of those examples, or mention it's an option in one of those examples. I'm mostly asking just for clarification so I can better understand what's wanted here.

[alice-i-cecile]

Yep, just use it in one of those examples so it's easier to discover (by following the metaphorical bread crumbs).

[Sapein]

Should I add in an 800x600 resolution as well? It seems like many examples use that resolution as well. So it might be useful to implement that resolution as well.

[janhohenheim]

@Sapein that's just SVGA, right? Pretty sure the examples just used some arbitrary resolution, so there's no harm to changing it to something like 720p or HD.

Edit: Maybe supporting this table would be good, but that can also be a follow-up PR if you want.

[Sapein]

@Sapein that's just SVGA, right? Pretty sure the examples just used some arbitrary resolution, so there's no harm to changing it to something like 720p or HD.

Edit: Maybe supporting this table would be good, but that can also be a follow-up PR if you want.

Yeah I think it's just SVGA. I can definitely change it, just wasn't sure if I should change it or not.

Also should I have a From<CommonScreenResolution> for Vec2 implementation? I'm mostly asking because window_resizing uses Vec2s for Window Sizes.

[alice-i-cecile]

@Sapein if you get back to this, take a look at the changes in #15091 :) Let us know if you'd like any advice on adapting this PR to work with it.

[Sapein]

@Sapein if you get back to this, take a look at the changes in #15091 :) Let us know if you'd like any advice on adapting this PR to work with it.

Hey, thanks for letting me know about that and I would really like some advice on adapting it to work with that as well, since I'm not entirely certain how it should tbh.

[alice-i-cecile]

Paging @miniex, you may have ideas here as well :)

[miniex]

I like structure that can be dynamically created and extended.

First, what about resolution cateogry?

ex:

pub enum ResolutionCategory {
    HD,
    FHD,
    QHD,
    Mobile,
    Ultrawide,
}

impl CommonScreenResolution {
    pub fn category(&self) -> ResolutionCategory {
        ...
    }
}

And, i like a good idea to allow them to be made in AspectRatio.

Then, that make it easier to add documents or solutions?

My english is bad, so I'm not sure if i understood it. ToT

[Sapein]

I like structure that can be dynamically created and extended.

First, what about resolution cateogry?

ex:

pub enum ResolutionCategory {
    HD,
    FHD,
    QHD,
    Mobile,
    Ultrawide,
}

impl CommonScreenResolution {
    pub fn category(&self) -> ResolutionCategory {
        ...
    }
}

And, i like a good idea to allow them to be made in AspectRatio.

Then, that make it easier to add documents or solutions?

My english is bad, so I'm not sure if i understood it. ToT

I'm not sure I understand how this is supposed to work, tbqh. Could you try and elaborate or provide some examples?

[IceSentry]

I don't think this should be an enum and I'm not convinced at all it even needs to exists. An ordered list could make sense for a setting menu, but that list should be dynamically generated based on what the hardware supports. It should include ultrawide resolutions for people using an ultrawide.

Monitors are not limited to just 16:9. Games or apps should pretty much never hardcode a resolution and they should adapt their game to work at any aspect ratio and resolution. Some games need a fixed aspect ratio for artistic reasons, while I personally disagree that it should ever be done, if a game has to do it, it should not be done at the window level. The window should still be able to scale to any screen size. If we had to make a list, and I really don't think we should, but we should use something like steam hardware survey. We shouldn't encourage people to test at a fixed 16:9 ratio, it just makes things more painful for any player that doesn't have a 16:9 screen.

[miniex]

My opinion is same as @IceSentry. As example, i made this code. I don't know this is good code.

use bevy_math::AspectRatio;

pub struct Resolution {
    width: u32,
    height: u32,
    category: ResolutionCategory,
}

pub enum ResolutionCategory {
    HD,
    FHD,
    QHD,
    Mobile,
    Ultrawide,
}

impl Resolution {
    pub fn new(width: u32, height: u32, category: ResolutionCategory)
    pub fn aspect_ratio(&self) -> AspectRatio
}

pub struct CommonScreenResolutions {
    resolutions: HashMap<String, Resolution>,
}

impl CommonScreenResolutions {
    // or default
    pub fn new() -> Self {
        let mut resolutions = HashMap::new();
        resolutions.insert("360p_4:3", Resolution::new(480, 360, ResolutionCategory::SD));
        resolutions.insert("360p_16:9", Resolution::new(640, 360, ResolutionCategory::SD));
        resolutions.insert("720p", Resolution::new(1280, 720, ResolutionCategory::HD));
        resolutions.insert("1080p", Resolution::new(1920, 1080, ResolutionCategory::FHD));
        Self { resolutions }
    }
    
    pub fn add(&mut self, name: String, resolution: Resolution)
    pub fn get(&self, name: &str) -> Option<&Resolution>
    pub fn find_by_dimensions(&self, width: u32, height: u32) -> Option<(&String, &Resolution)>
    pub fn find_by_aspect_ratio(&self, aspect_ratio: AspectRatio) -> Vec<(&String, &Resolution)>
}

[miniex]

• find_by_category

[Sapein]

I don't think this should be an enum and I'm not convinced at all it even needs to exists. An ordered list could make sense for a setting menu, but that list should be dynamically generated based on what the hardware supports. It should include ultrawide resolutions for people using an ultrawide.

Monitors are not limited to just 16:9. Games or apps should pretty much never hardcode a resolution and they should adapt their game to work at any aspect ratio and resolution. Some games need a fixed aspect ratio for artistic reasons, while I personally disagree that it should ever be done, if a game has to do it, it should not be done at the window level. The window should still be able to scale to any screen size. If we had to make a list, and I really don't think we should, but we should use something like steam hardware survey. We shouldn't encourage people to test at a fixed 16:9 ratio, it just makes things more painful for any player that doesn't have a 16:9 screen.

I strongly disagree, as this is a common thing that I've had to setup for all of my projects, and it's a pain point that Bevy has, in that this is rather unfriendly IMHO. It's also not uncommon for 2D games in particular to focus on specific resolutions that they support as well, especially if you're doing pixel art.

[alice-i-cecile]

Reopening, but marking controversial.

[benfrankel]

Games or apps should pretty much never hardcode a resolution and they should adapt their game to work at any aspect ratio and resolution.

FWIW, a lot of games in my Steam library either prevent you from resizing the window to a different aspect ratio, or very clearly were not tested on different aspect ratios. Those are real published games. Sure, it would be nice for a game to support any aspect ratio, but evidently it isn't necessary and many developers consider it not worth the effort and artistic compromise.

[IceSentry]

I strongly disagree, as this is a common thing that I've had to setup for all of my projects, and it's a pain point that Bevy has, in that this is rather unfriendly IMHO. It's also not uncommon for 2D games in particular to focus on specific resolutions that they support as well, especially if you're doing pixel art.

We should have an easy way to do letterboxing of course, but hiding this behind something that appears as a "common resolution" is not the way it should be done. Common resolution are a myth and we shoudn't encourage devs to believe there is such a thing as a common resolution. It should be very clear that it is something only used for letterboxing for artistic purposes.

FWIW, a lot of games in my Steam library either prevent you from resizing the window to a different aspect ratio, or very clearly were not tested on different aspect ratios. Those are real published games. Sure, it would be nice for a game to support any aspect ratio, but evidently it isn't necessary and many developers consider it not worth the effort and artistic compromise.

I'm extremely aware of that and it's one of the main reason why I don't want to push people towards a limited set of resolution at a fixed aspect ratio. Most games actually do handle this correctly which makes the ones that don't even more jarring. I really don't think this should be encouraged by making this the easy path and calling it a common resolution.

[cart]

but hiding this behind something that appears as a "common resolution" is not the way it should be done

I agree. Any "opinionated" view of available resolutions is a user-facing problem to solve (either directly by the app developer, or as a 3rd party ecosystem plugin).

I'm pretty adamantly against including this upstream, so in the interest of saving time I'm closing this out. @Sapein I encourage you to create a 3rd party plugin for this! (and register it in Bevy Assets)

[janhohenheim]

@IceSentry @cart interesting, I didn't know that having such a list hardcoded in the settings was considered bad practice. Thanks for pointing it out :) Do you have any resources on how to do this the right way in Bevy? How do I dynamically generate such a list based on hardware for the settings menu? Another thing that is not clear to me is how fixed aspect ratio games that do this for artistic purposes choose an aspect ratio. If common aspect ratios are a myth, do they just pick something arbitrary like 1300x700 because that happens to look good? Assuming best practices, of course.