wayland: use correct rounding in logical->physical conversions #3955
Conversation
```rust
/// A wp-fractional-scale scale.
///
/// This type implements the `physical_size = round_half_up(logical_size * scale)`
/// operation with infinite precision as required by the protocol.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub struct Scale {
scale: u64,
}
```
|
While I don't mind changing the algorithm is unspecified, which we brought before here https://gitlab.freedesktop.org/wayland/wayland-protocols/-/issues/132 What winit does is what majority of compositors do (probably not yours since you've sent the patch), so we error the same. Unless upstream issue progresses, I don't see why this new rounding is correct when there's no specification of what is correct. |
|
See https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/309/diffs
Actually my compositor also uses f64 just like winit. A mistake for sure. |
|
|
||
| impl Scale { | ||
| pub fn from_wp_fractional_scale(v: u32) -> Self { | ||
| assert!(v > 0); |
There was a problem hiding this comment.
You should use NonZero rather than an assert.
| self.scale as f64 / BASE_F64 | ||
| } | ||
|
|
||
| pub fn round_up(self) -> u32 { |
There was a problem hiding this comment.
Do we really need a roundup? The old logic was trunc. Though, the path were it can happen is always having an integer, so it would never round in reality.
|
|
||
| const BASE: u32 = 120; | ||
| const BASE_U64: u64 = BASE as u64; | ||
| const BASE_F64: f64 = BASE as f64; |
There was a problem hiding this comment.
it's used exactly in one place, so you can inline and just call it DENOMINATOR: u64 = 120.
You should've have linked it right away in the PR description. Will wait until it merges and then I'll merge it here as well once review is done. |
changelogmodule if knowledge of this change could be valuable to users