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

wayland: use correct rounding in logical->physical conversions #3955

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/platform_impl/linux/wayland/event_loop/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use sink::EventSink;

use super::state::{WindowCompositorUpdate, WinitState};
use super::window::state::FrameCallbackState;
use super::{logical_to_physical_rounded, WindowId};
use super::WindowId;

type WaylandDispatcher = calloop::Dispatcher<'static, WaylandSource<WinitState>, WinitState>;

Expand Down Expand Up @@ -304,8 +304,8 @@ impl EventLoop {
let windows = state.windows.get_mut();
let window = windows.get(&window_id).unwrap().lock().unwrap();
let scale_factor = window.scale_factor();
let size = logical_to_physical_rounded(window.surface_size(), scale_factor);
(size, scale_factor)
let size = window.surface_size() * scale_factor;
(size, scale_factor.to_f64())
});

// Stash the old window size.
Expand Down Expand Up @@ -346,7 +346,7 @@ impl EventLoop {
let window = windows.get(&window_id).unwrap().lock().unwrap();

let scale_factor = window.scale_factor();
let size = logical_to_physical_rounded(window.surface_size(), scale_factor);
let size = window.surface_size() * scale_factor;

// Mark the window as needed a redraw.
state
Expand Down
9 changes: 1 addition & 8 deletions src/platform_impl/linux/wayland/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ use sctk::reexports::client::Proxy;
pub use window::Window;

pub(super) use crate::cursor::OnlyCursorImage as CustomCursor;
use crate::dpi::{LogicalSize, PhysicalSize};
use crate::window::WindowId;

mod event_loop;
mod output;
mod scale;
mod seat;
mod state;
mod types;
Expand All @@ -32,10 +32,3 @@ impl FingerId {
fn make_wid(surface: &WlSurface) -> WindowId {
WindowId::from_raw(surface.id().as_ptr() as usize)
}

/// The default routine does floor, but we need round on Wayland.
fn logical_to_physical_rounded(size: LogicalSize<u32>, scale_factor: f64) -> PhysicalSize<u32> {
let width = size.width as f64 * scale_factor;
let height = size.height as f64 * scale_factor;
(width.round(), height.round()).into()
}
52 changes: 52 additions & 0 deletions src/platform_impl/linux/wayland/scale.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use std::ops::Mul;

use dpi::{LogicalSize, PhysicalSize};

/// 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,
}

const BASE: u32 = 120;
const BASE_U64: u64 = BASE as u64;
const BASE_F64: f64 = BASE as f64;
Copy link
Member

Choose a reason for hiding this comment

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

it's used exactly in one place, so you can inline and just call it DENOMINATOR: u64 = 120.


impl Scale {
pub fn from_wp_fractional_scale(v: u32) -> Self {
assert!(v > 0);
Copy link
Member

Choose a reason for hiding this comment

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

You should use NonZero rather than an assert.

Self { scale: v as u64 }
}

pub fn from_integer_scale(v: u32) -> Self {
Self::from_wp_fractional_scale(v.saturating_mul(BASE))
}

pub fn to_f64(self) -> f64 {
self.scale as f64 / BASE_F64
}

pub fn round_up(self) -> u32 {
Copy link
Member

Choose a reason for hiding this comment

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

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.

((self.scale + BASE_U64 - 1) / BASE_U64) as u32
}

fn surface_to_buffer<const N: usize>(self, sizes: [u32; N]) -> [u32; N] {
sizes.map(|surface| {
// buffer = floor((surface * scale + 60) / 120)
let buffer = (surface as u64 * self.scale + BASE_U64 / 2) / BASE_U64;
buffer.min(u32::MAX as u64) as u32
})
}
}

impl Mul<Scale> for LogicalSize<u32> {
type Output = PhysicalSize<u32>;

fn mul(self, scale: Scale) -> Self::Output {
let [width, height] = scale.surface_to_buffer([self.width, self.height]);
PhysicalSize { width, height }
}
}
2 changes: 1 addition & 1 deletion src/platform_impl/linux/wayland/seat/pointer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl PointerHandler for WinitState {
None => continue,
};

let scale_factor = window.scale_factor();
let scale_factor = window.scale_factor_f64();
let position: PhysicalPosition<f64> =
LogicalPosition::new(event.position.0, event.position.1).to_physical(scale_factor);

Expand Down
8 changes: 4 additions & 4 deletions src/platform_impl/linux/wayland/seat/touch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl TouchHandler for WinitState {
) {
let window_id = wayland::make_wid(&surface);
let scale_factor = match self.windows.get_mut().get(&window_id) {
Some(window) => window.lock().unwrap().scale_factor(),
Some(window) => window.lock().unwrap().scale_factor_f64(),
None => return,
};

Expand Down Expand Up @@ -90,7 +90,7 @@ impl TouchHandler for WinitState {

let window_id = wayland::make_wid(&touch_point.surface);
let scale_factor = match self.windows.get_mut().get(&window_id) {
Some(window) => window.lock().unwrap().scale_factor(),
Some(window) => window.lock().unwrap().scale_factor_f64(),
None => return,
};

Expand Down Expand Up @@ -142,7 +142,7 @@ impl TouchHandler for WinitState {

let window_id = wayland::make_wid(&touch_point.surface);
let scale_factor = match self.windows.get_mut().get(&window_id) {
Some(window) => window.lock().unwrap().scale_factor(),
Some(window) => window.lock().unwrap().scale_factor_f64(),
None => return,
};

Expand Down Expand Up @@ -175,7 +175,7 @@ impl TouchHandler for WinitState {
for (id, touch_point) in seat_state.touch_map.drain() {
let window_id = wayland::make_wid(&touch_point.surface);
let scale_factor = match self.windows.get_mut().get(&window_id) {
Some(window) => window.lock().unwrap().scale_factor(),
Some(window) => window.lock().unwrap().scale_factor_f64(),
None => return,
};

Expand Down
7 changes: 5 additions & 2 deletions src/platform_impl/linux/wayland/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use sctk::subcompositor::SubcompositorState;
use crate::error::OsError;
use crate::platform_impl::wayland::event_loop::sink::EventSink;
use crate::platform_impl::wayland::output::MonitorHandle;
use crate::platform_impl::wayland::scale::Scale;
use crate::platform_impl::wayland::seat::{
PointerConstraintsState, RelativePointerState, TextInputState, WinitPointerData,
WinitPointerDataExt, WinitSeatState,
Expand Down Expand Up @@ -200,7 +201,7 @@ impl WinitState {
pub fn scale_factor_changed(
&mut self,
surface: &WlSurface,
scale_factor: f64,
scale_factor: Scale,
is_legacy: bool,
) {
// Check if the cursor surface.
Expand Down Expand Up @@ -372,7 +373,9 @@ impl CompositorHandler for WinitState {
surface: &WlSurface,
scale_factor: i32,
) {
self.scale_factor_changed(surface, scale_factor as f64, true)
assert!(scale_factor >= 0);
let scale = Scale::from_integer_scale(scale_factor as u32);
self.scale_factor_changed(surface, scale, true)
}

fn frame(&mut self, _: &Connection, _: &QueueHandle<Self>, surface: &WlSurface, _: u32) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ use sctk::reexports::protocols::wp::fractional_scale::v1::client::wp_fractional_
Event as FractionalScalingEvent, WpFractionalScaleV1,
};

use crate::platform_impl::wayland::scale::Scale;
use crate::platform_impl::wayland::state::WinitState;

/// The scaling factor denominator.
const SCALE_DENOMINATOR: f64 = 120.;

/// Fractional scaling manager.
#[derive(Debug)]
pub struct FractionalScalingManager {
Expand Down Expand Up @@ -68,7 +66,8 @@ impl Dispatch<WpFractionalScaleV1, FractionalScaling, WinitState> for Fractional
_: &QueueHandle<WinitState>,
) {
if let FractionalScalingEvent::PreferredScale { scale } = event {
state.scale_factor_changed(&data.surface, scale as f64 / SCALE_DENOMINATOR, false);
let scale = Scale::from_wp_fractional_scale(scale);
state.scale_factor_changed(&data.surface, scale, false);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/platform_impl/linux/wayland/window/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl CoreWindow for Window {
fn surface_size(&self) -> PhysicalSize<u32> {
let window_state = self.window_state.lock().unwrap();
let scale_factor = window_state.scale_factor();
super::logical_to_physical_rounded(window_state.surface_size(), scale_factor)
window_state.surface_size() * scale_factor
}

fn request_surface_size(&self, size: Size) -> Option<PhysicalSize<u32>> {
Expand All @@ -335,7 +335,7 @@ impl CoreWindow for Window {
fn outer_size(&self) -> PhysicalSize<u32> {
let window_state = self.window_state.lock().unwrap();
let scale_factor = window_state.scale_factor();
super::logical_to_physical_rounded(window_state.outer_size(), scale_factor)
window_state.outer_size() * scale_factor
}

fn set_min_surface_size(&self, min_size: Option<Size>) {
Expand Down Expand Up @@ -474,7 +474,7 @@ impl CoreWindow for Window {

#[inline]
fn scale_factor(&self) -> f64 {
self.window_state.lock().unwrap().scale_factor()
self.window_state.lock().unwrap().scale_factor_f64()
}

#[inline]
Expand All @@ -500,7 +500,7 @@ impl CoreWindow for Window {
fn set_ime_cursor_area(&self, position: Position, size: Size) {
let window_state = self.window_state.lock().unwrap();
if window_state.ime_allowed() {
let scale_factor = window_state.scale_factor();
let scale_factor = window_state.scale_factor_f64();
let position = position.to_logical(scale_factor);
let size = size.to_logical(scale_factor);
window_state.set_ime_cursor_area(position, size);
Expand Down
28 changes: 17 additions & 11 deletions src/platform_impl/linux/wayland/window/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use wayland_protocols_plasma::blur::client::org_kde_kwin_blur::OrgKdeKwinBlur;
use crate::cursor::CustomCursor as RootCustomCursor;
use crate::dpi::{LogicalPosition, LogicalSize, PhysicalSize, Size};
use crate::error::{NotSupportedError, RequestError};
use crate::platform_impl::wayland::logical_to_physical_rounded;
use crate::platform_impl::wayland::scale::Scale;
use crate::platform_impl::wayland::seat::{
PointerConstraintsState, WinitPointerData, WinitPointerDataExt, ZwpTextInputV3Ext,
};
Expand Down Expand Up @@ -92,7 +92,7 @@ pub struct WindowState {
seat_focus: HashSet<ObjectId>,

/// The scale factor of the window.
scale_factor: f64,
scale_factor: Scale,

/// Whether the window is transparent.
transparent: bool,
Expand Down Expand Up @@ -203,7 +203,7 @@ impl WindowState {
pointers: Default::default(),
queue_handle: queue_handle.clone(),
resizable: true,
scale_factor: 1.,
scale_factor: Scale::from_integer_scale(1),
shm: winit_state.shm.wl_shm().clone(),
custom_cursor_pool: winit_state.custom_cursor_pool.clone(),
size: initial_size.to_logical(1.),
Expand Down Expand Up @@ -266,7 +266,7 @@ impl WindowState {
// should be delivered before the first configure, thus apply it to
// properly scale the physical sizes provided by the users.
if let Some(initial_size) = self.initial_size.take() {
self.size = initial_size.to_logical(self.scale_factor());
self.size = initial_size.to_logical(self.scale_factor_f64());
self.stateless_size = self.size;
}

Expand All @@ -287,7 +287,7 @@ impl WindowState {
) {
Ok(mut frame) => {
frame.set_title(&self.title);
frame.set_scaling_factor(self.scale_factor);
frame.set_scaling_factor(self.scale_factor.to_f64());
// Hide the frame if we were asked to not decorate.
frame.set_hidden(!self.decorate);
self.frame = Some(frame);
Expand Down Expand Up @@ -630,10 +630,10 @@ impl WindowState {
/// Try to resize the window when the user can do so.
pub fn request_surface_size(&mut self, surface_size: Size) -> PhysicalSize<u32> {
if self.last_configure.as_ref().map(Self::is_stateless).unwrap_or(true) {
self.resize(surface_size.to_logical(self.scale_factor()))
self.resize(surface_size.to_logical(self.scale_factor_f64()))
}

logical_to_physical_rounded(self.surface_size(), self.scale_factor())
self.surface_size() * self.scale_factor()
}

/// Resize the window to the new surface size.
Expand Down Expand Up @@ -680,10 +680,16 @@ impl WindowState {

/// Get the scale factor of the window.
#[inline]
pub fn scale_factor(&self) -> f64 {
pub fn scale_factor(&self) -> Scale {
self.scale_factor
}

/// Get the f64 approximation of the scale factor of the window.
#[inline]
pub fn scale_factor_f64(&self) -> f64 {
self.scale_factor.to_f64()
}

/// Set the cursor icon.
pub fn set_cursor(&mut self, cursor_icon: CursorIcon) {
self.selected_cursor = SelectedCursor::Named(cursor_icon);
Expand Down Expand Up @@ -993,16 +999,16 @@ impl WindowState {

/// Set the scale factor for the given window.
#[inline]
pub fn set_scale_factor(&mut self, scale_factor: f64) {
pub fn set_scale_factor(&mut self, scale_factor: Scale) {
self.scale_factor = scale_factor;

// NOTE: When fractional scaling is not used update the buffer scale.
if self.fractional_scale.is_none() {
let _ = self.window.set_buffer_scale(self.scale_factor as _);
let _ = self.window.set_buffer_scale(self.scale_factor.round_up());
}

if let Some(frame) = self.frame.as_mut() {
frame.set_scaling_factor(scale_factor);
frame.set_scaling_factor(scale_factor.to_f64());
}
}

Expand Down