The `WindowResolution` API is very confusing
- Dominant language
- Rust
- Stars
- 48.2k
- Forks
- 4.8k
- Avg merge
- 3d 16h
- Merged PRs (30d)
- 171
Description
## What problem does this solve or what need does it fill?
The `WindowResolution` struct:
```rust
pub struct WindowResolution {
/// Width of the window in physical pixels.
physical_width: u32,
/// Height of the window in physical pixels.
physical_height: u32,
/// Code-provided ratio of physical size to logical size.
///
/// Should be used instead of `scale_factor` when set.
scale_factor_override: Option,
/// OS-provided ratio of physical size to logical size.
///
/// Set automatically depending on the pixel density of the screen.
scale_factor: f64,
}
```
It has a method `new`:
```rust
pub fn new(logical_width: f32, logical_height: f32) -> Self {
Self {
physical_width: logical_width as u32,
physical_height: logical_height as u32,
..Default::default()
}
}
```
Which looks like nonsense as we are writing values in logical pixels into fields named `physical_width` and `physical_height`.
It's not a bug though. What happens is that on window creation the windowing backend retrieves the OS scale factor (assume the user hasn't set a scalefactor override) and writes it to the `scale_factor` field. Then it multiplies the logical `physical_width` and `physical_height` values by the scale factor to get the underlying resolution in physical pixels that the window should be created with.
It's very confusing and difficult to understand from the code, particularly because the window creation backend logic is in a separate module.
## What solution would you like?
My preferred API would be something like this:
```rust
pub struct Window {
/// Desired resolution for the window
pub target_resolution: TargetWindowResolution,
/// Resolution of the window, automatically set on creation
pub current_resolution: Option,
// .. rest of Window's fields
}
pub struct TargetWindowResolution {
pub size: LogicalSize,
pub scale_factor_override: Option,
}
pub struct WindowResolution {
pub size: PhysicalSize,
pub scale_factor: f32,
}
```
## What alternative(s) have you considered?
We could make `WindowResolution` into an enum instead, something like:
```rust
pub enum WindowResolution {
Target {
size: LogicalSize,
scale_factor_override: Option,
},
Current {
size: PhysicalSize,
scale_factor: f32,
scale_factor_override: Option,
}
}
```
## Additional context
Some more discussion and context at #11015
Contributor guide
Research direction
Start by reading the WindowResolution API and the separate window creation backend logic described in the issue, then review the related discussion in #11015. The work is done when the project agrees on a clearer target/current resolution design and the affected API and backend behavior are updated consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, desktop, game-dev
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100