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

Rename "inner size" to "surface size" #3889

Merged
merged 6 commits into from
Sep 4, 2024
Merged
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
4 changes: 2 additions & 2 deletions examples/child_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn main() -> Result<(), impl std::error::Error> {
let attributes = WindowAttributes::default()
.with_title("parent window")
.with_position(Position::Logical(LogicalPosition::new(0.0, 0.0)))
.with_inner_size(LogicalSize::new(640.0f32, 480.0f32));
.with_surface_size(LogicalSize::new(640.0f32, 480.0f32));
let window = event_loop.create_window(attributes).unwrap();

println!("Parent window id: {:?})", window.id());
Expand Down Expand Up @@ -79,7 +79,7 @@ fn main() -> Result<(), impl std::error::Error> {
let parent = parent.raw_window_handle().unwrap();
let mut window_attributes = WindowAttributes::default()
.with_title("child window")
.with_inner_size(LogicalSize::new(200.0f32, 200.0f32))
.with_surface_size(LogicalSize::new(200.0f32, 200.0f32))
.with_position(Position::Logical(LogicalPosition::new(0.0, 0.0)))
.with_visible(true);
// `with_parent_window` is unsafe. Parent window must be a valid window.
Expand Down
2 changes: 1 addition & 1 deletion examples/run_on_demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
fn can_create_surfaces(&mut self, event_loop: &dyn ActiveEventLoop) {
let window_attributes = WindowAttributes::default()
.with_title("Fantastic window number one!")
.with_inner_size(winit::dpi::LogicalSize::new(128.0, 128.0));
.with_surface_size(winit::dpi::LogicalSize::new(128.0, 128.0));
let window = event_loop.create_window(window_attributes).unwrap();
self.window_id = Some(window.id());
self.window = Some(window);
Expand Down
2 changes: 1 addition & 1 deletion examples/util/fill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ mod platform {

pub fn fill_window(window: &dyn Window) {
GC.with(|gc| {
let size = window.inner_size();
let size = window.surface_size();
let (Some(width), Some(height)) =
(NonZeroU32::new(size.width), NonZeroU32::new(size.height))
else {
Expand Down
32 changes: 16 additions & 16 deletions examples/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ impl ApplicationHandler for Application {
};

match event {
WindowEvent::Resized(size) => {
WindowEvent::SurfaceResized(size) => {
window.resize(size);
},
WindowEvent::Focused(focused) => {
Expand Down Expand Up @@ -607,7 +607,7 @@ impl WindowState {
let ime = true;
window.set_ime_allowed(ime);

let size = window.inner_size();
let size = window.surface_size();
let mut state = Self {
#[cfg(macos_platform)]
option_as_alt: window.option_as_alt(),
Expand Down Expand Up @@ -681,12 +681,12 @@ impl WindowState {

/// Toggle resize increments on a window.
fn toggle_resize_increments(&mut self) {
let new_increments = match self.window.resize_increments() {
let new_increments = match self.window.surface_resize_increments() {
Some(_) => None,
None => Some(LogicalSize::new(25.0, 25.0).into()),
};
info!("Had increments: {}", new_increments.is_none());
self.window.set_resize_increments(new_increments);
self.window.set_surface_resize_increments(new_increments);
}

/// Toggle fullscreen.
Expand Down Expand Up @@ -725,22 +725,22 @@ impl WindowState {
self.window.set_option_as_alt(self.option_as_alt);
}

/// Swap the window dimensions with `request_inner_size`.
/// Swap the window dimensions with `request_surface_size`.
fn swap_dimensions(&mut self) {
let old_inner_size = self.window.inner_size();
let mut inner_size = old_inner_size;
let old_surface_size = self.window.surface_size();
let mut surface_size = old_surface_size;

mem::swap(&mut inner_size.width, &mut inner_size.height);
info!("Requesting resize from {old_inner_size:?} to {inner_size:?}");
mem::swap(&mut surface_size.width, &mut surface_size.height);
info!("Requesting resize from {old_surface_size:?} to {surface_size:?}");

if let Some(new_inner_size) = self.window.request_inner_size(inner_size.into()) {
if old_inner_size == new_inner_size {
if let Some(new_surface_size) = self.window.request_surface_size(surface_size.into()) {
if old_surface_size == new_surface_size {
info!("Inner size change got ignored");
} else {
self.resize(new_inner_size);
self.resize(new_surface_size);
}
} else {
info!("Request inner size is asynchronous");
info!("Requesting surface size is asynchronous");
}
}

Expand Down Expand Up @@ -793,9 +793,9 @@ impl WindowState {
Ok(())
}

/// Resize the window to the new size.
/// Resize the surface to the new size.
fn resize(&mut self, size: PhysicalSize<u32>) {
info!("Resized to {size:?}");
info!("Surface resized to {size:?}");
#[cfg(not(any(android_platform, ios_platform)))]
{
let (width, height) = match (NonZeroU32::new(size.width), NonZeroU32::new(size.height))
Expand Down Expand Up @@ -840,7 +840,7 @@ impl WindowState {
},
};

let win_size = self.window.inner_size();
let win_size = self.window.surface_size();
let border_size = BORDER_SIZE * self.window.scale_factor();

let x_direction = if position.x < border_size {
Expand Down
2 changes: 1 addition & 1 deletion examples/x11_embed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn main() -> Result<(), Box<dyn Error>> {
fn can_create_surfaces(&mut self, event_loop: &dyn ActiveEventLoop) {
let window_attributes = WindowAttributes::default()
.with_title("An embedded window!")
.with_inner_size(winit::dpi::LogicalSize::new(128.0, 128.0))
.with_surface_size(winit::dpi::LogicalSize::new(128.0, 128.0))
.with_embed_parent_window(self.parent_window_id);

self.window = Some(event_loop.create_window(window_attributes).unwrap());
Expand Down
17 changes: 17 additions & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,23 @@ changelog entry.
- On iOS, no longer act as-if the application successfully open all URLs. Override
`application:didFinishLaunchingWithOptions:` and provide the desired behaviour yourself.
- On X11, remove our dependency on libXcursor. (#3749)
- Renamed the following APIs to make it clearer that the sizes apply to the underlying surface:
- `WindowEvent::Resized` to `SurfaceResized`.
- `InnerSizeWriter` to `SurfaceSizeWriter`.
- `WindowAttributes.inner_size` to `surface_size`.
- `WindowAttributes.min_inner_size` to `min_surface_size`.
- `WindowAttributes.max_inner_size` to `max_surface_size`.
- `WindowAttributes.resize_increments` to `surface_resize_increments`.
- `WindowAttributes::with_inner_size` to `with_surface_size`.
- `WindowAttributes::with_min_inner_size` to `with_min_surface_size`.
- `WindowAttributes::with_max_inner_size` to `with_max_surface_size`.
- `WindowAttributes::with_resize_increments` to `with_surface_resize_increments`.
- `Window::inner_size` to `surface_size`.
- `Window::request_inner_size` to `request_surface_size`.
- `Window::set_min_inner_size` to `set_min_surface_size`.
- `Window::set_max_inner_size` to `set_max_surface_size`.

To migrate, you can probably just replace all instances of `inner_size` with `surface_size` in your codebase.

### Removed

Expand Down
48 changes: 26 additions & 22 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,13 @@ pub enum WindowEvent {
/// [`request_activation_token`]: crate::platform::startup_notify::WindowExtStartupNotify::request_activation_token
ActivationTokenDone { serial: AsyncRequestSerial, token: ActivationToken },

/// The size of the window has changed. Contains the client area's new dimensions.
Resized(PhysicalSize<u32>),
/// The size of the window's surface has changed.
///
/// Contains the new dimensions of the surface (can also be retrieved with
/// [`Window::surface_size`]).
///
/// [`Window::surface_size`]: crate::window::Window::surface_size
SurfaceResized(PhysicalSize<u32>),

/// The position of the window has changed. Contains the window's new position.
///
Expand Down Expand Up @@ -360,16 +365,16 @@ pub enum WindowEvent {
/// * Changing the display's scale factor (e.g. in Control Panel on Windows).
/// * Moving the window to a display with a different scale factor.
///
/// To update the window size, use the provided [`InnerSizeWriter`] handle. By default, the
/// To update the window size, use the provided [`SurfaceSizeWriter`] handle. By default, the
/// window is resized to the value suggested by the OS, but it can be changed to any value.
///
/// For more information about DPI in general, see the [`dpi`] crate.
ScaleFactorChanged {
scale_factor: f64,
/// Handle to update inner size during scale changes.
/// Handle to update surface size during scale changes.
///
/// See [`InnerSizeWriter`] docs for more details.
inner_size_writer: InnerSizeWriter,
/// See [`SurfaceSizeWriter`] docs for more details.
surface_size_writer: SurfaceSizeWriter,
},

/// The system window theme has changed.
Expand Down Expand Up @@ -995,40 +1000,39 @@ pub enum MouseScrollDelta {
PixelDelta(PhysicalPosition<f64>),
}

/// Handle to synchronously change the size of the window from the
/// [`WindowEvent`].
/// Handle to synchronously change the size of the window from the [`WindowEvent`].
#[derive(Debug, Clone)]
pub struct InnerSizeWriter {
pub(crate) new_inner_size: Weak<Mutex<PhysicalSize<u32>>>,
pub struct SurfaceSizeWriter {
pub(crate) new_surface_size: Weak<Mutex<PhysicalSize<u32>>>,
}

impl InnerSizeWriter {
impl SurfaceSizeWriter {
#[cfg(not(orbital_platform))]
pub(crate) fn new(new_inner_size: Weak<Mutex<PhysicalSize<u32>>>) -> Self {
Self { new_inner_size }
pub(crate) fn new(new_surface_size: Weak<Mutex<PhysicalSize<u32>>>) -> Self {
Self { new_surface_size }
}

/// Try to request inner size which will be set synchronously on the window.
pub fn request_inner_size(
/// Try to request surface size which will be set synchronously on the window.
pub fn request_surface_size(
&mut self,
new_inner_size: PhysicalSize<u32>,
new_surface_size: PhysicalSize<u32>,
) -> Result<(), ExternalError> {
if let Some(inner) = self.new_inner_size.upgrade() {
*inner.lock().unwrap() = new_inner_size;
if let Some(inner) = self.new_surface_size.upgrade() {
*inner.lock().unwrap() = new_surface_size;
Ok(())
} else {
Err(ExternalError::Ignored)
}
}
}

impl PartialEq for InnerSizeWriter {
impl PartialEq for SurfaceSizeWriter {
fn eq(&self, other: &Self) -> bool {
self.new_inner_size.as_ptr() == other.new_inner_size.as_ptr()
self.new_surface_size.as_ptr() == other.new_surface_size.as_ptr()
}
}

impl Eq for InnerSizeWriter {}
impl Eq for SurfaceSizeWriter {}

#[cfg(test)]
mod tests {
Expand Down Expand Up @@ -1066,7 +1070,7 @@ mod tests {
with_window_event(Destroyed);
with_window_event(Focused(true));
with_window_event(Moved((0, 0).into()));
with_window_event(Resized((0, 0).into()));
with_window_event(SurfaceResized((0, 0).into()));
with_window_event(DroppedFile("x.txt".into()));
with_window_event(HoveredFile("x.txt".into()));
with_window_event(HoveredFileCancelled);
Expand Down
4 changes: 2 additions & 2 deletions src/monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ impl Ord for VideoModeHandle {

impl VideoModeHandle {
/// Returns the resolution of this video mode. This **must not** be used to create your
/// rendering surface. Use [`Window::inner_size()`] instead.
/// rendering surface. Use [`Window::surface_size()`] instead.
///
/// [`Window::inner_size()`]: crate::window::Window::inner_size
/// [`Window::surface_size()`]: crate::window::Window::surface_size
#[inline]
pub fn size(&self) -> PhysicalSize<u32> {
self.video_mode.size()
Expand Down
6 changes: 3 additions & 3 deletions src/platform/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
//! - [`padding`](https://developer.mozilla.org/en-US/docs/Web/CSS/padding)
//!
//! The following APIs can't take them into account and will therefore provide inaccurate results:
//! - [`WindowEvent::Resized`] and [`Window::(set_)inner_size()`]
//! - [`WindowEvent::SurfaceResized`] and [`Window::(set_)surface_size()`]
//! - [`WindowEvent::Occluded`]
//! - [`WindowEvent::CursorMoved`], [`WindowEvent::CursorEntered`], [`WindowEvent::CursorLeft`], and
//! [`WindowEvent::Touch`].
//! - [`Window::set_outer_position()`]
//!
//! [`WindowEvent::Resized`]: crate::event::WindowEvent::Resized
//! [`Window::(set_)inner_size()`]: crate::window::Window::inner_size
//! [`WindowEvent::SurfaceResized`]: crate::event::WindowEvent::SurfaceResized
//! [`Window::(set_)surface_size()`]: crate::window::Window::surface_size
//! [`WindowEvent::Occluded`]: crate::event::WindowEvent::Occluded
//! [`WindowEvent::CursorMoved`]: crate::event::WindowEvent::CursorMoved
//! [`WindowEvent::CursorEntered`]: crate::event::WindowEvent::CursorEntered
Expand Down
24 changes: 12 additions & 12 deletions src/platform_impl/android/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::application::ApplicationHandler;
use crate::cursor::Cursor;
use crate::dpi::{PhysicalPosition, PhysicalSize, Position, Size};
use crate::error::{self, EventLoopError, ExternalError, NotSupportedError};
use crate::event::{self, Force, InnerSizeWriter, StartCause};
use crate::event::{self, Force, StartCause, SurfaceSizeWriter};
use crate::event_loop::{
ActiveEventLoop as RootActiveEventLoop, ControlFlow, DeviceEvents,
EventLoopProxy as RootEventLoopProxy, OwnedDisplayHandle as RootOwnedDisplayHandle,
Expand Down Expand Up @@ -201,11 +201,11 @@ impl EventLoop {
let old_scale_factor = scale_factor(&self.android_app);
let scale_factor = scale_factor(&self.android_app);
Comment on lines 201 to 202
Copy link
Member

Choose a reason for hiding this comment

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

Barring race conditions, this function appears reentrant and would always return the same value?

if (scale_factor - old_scale_factor).abs() < f64::EPSILON {
let new_inner_size = Arc::new(Mutex::new(screen_size(&self.android_app)));
let new_surface_size = Arc::new(Mutex::new(screen_size(&self.android_app)));
Comment on lines 203 to +204
Copy link
Member

Choose a reason for hiding this comment

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

Is the surface size in pixels on Android affected by a scale factor, or has it been observed to change when the scale factor changed)?

let window_id = window::WindowId(WindowId);
let event = event::WindowEvent::ScaleFactorChanged {
inner_size_writer: InnerSizeWriter::new(Arc::downgrade(
&new_inner_size,
surface_size_writer: SurfaceSizeWriter::new(Arc::downgrade(
&new_surface_size,
)),
scale_factor,
};
Expand Down Expand Up @@ -287,7 +287,7 @@ impl EventLoop {
PhysicalSize::new(0, 0)
};
let window_id = window::WindowId(WindowId);
let event = event::WindowEvent::Resized(size);
let event = event::WindowEvent::SurfaceResized(size);
app.window_event(&self.window_target, window_id, event);
}

Expand Down Expand Up @@ -808,27 +808,27 @@ impl CoreWindow for Window {
// no effect
}

fn inner_size(&self) -> PhysicalSize<u32> {
fn surface_size(&self) -> PhysicalSize<u32> {
self.outer_size()
Comment on lines +811 to 812
Copy link
Member

Choose a reason for hiding this comment

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

Should we invert this - make fn inner_size() return screen_size() and make fn outer_size() return self.inner_size()?

Copy link
Member Author

@madsmtm madsmtm Aug 24, 2024

Choose a reason for hiding this comment

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

I suspect "inner_size" here may refer to the safe area, and "outer size" to without safe area? Not sure though

Copy link
Member

Choose a reason for hiding this comment

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

Not sure either, but that could be how we intend to do it later in which case we better leave the surface_size() == outer_size() (with possible cut-outs, transparent system UI overlaps etc).

I was thinking of this in terms like Windows' windows for a while, where the drawable area/surface excludes the window decorations AFAIK. Android has a floating-window mode as well but that "outer size" is probably not easily exposed, not via the NDK at least...

}

fn request_inner_size(&self, _size: Size) -> Option<PhysicalSize<u32>> {
Some(self.inner_size())
fn request_surface_size(&self, _size: Size) -> Option<PhysicalSize<u32>> {
Some(self.surface_size())
}

fn outer_size(&self) -> PhysicalSize<u32> {
screen_size(&self.app)
}

fn set_min_inner_size(&self, _: Option<Size>) {}
fn set_min_surface_size(&self, _: Option<Size>) {}

fn set_max_inner_size(&self, _: Option<Size>) {}
fn set_max_surface_size(&self, _: Option<Size>) {}

fn resize_increments(&self) -> Option<PhysicalSize<u32>> {
fn surface_resize_increments(&self) -> Option<PhysicalSize<u32>> {
None
}

fn set_resize_increments(&self, _increments: Option<Size>) {}
fn set_surface_resize_increments(&self, _increments: Option<Size>) {}

fn set_title(&self, _title: &str) {}

Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/apple/appkit/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ declare_class!(
// 2. Even when a window resize does occur on a new tabbed window, it contains the wrong size (includes tab height).
let logical_size = LogicalSize::new(rect.size.width as f64, rect.size.height as f64);
let size = logical_size.to_physical::<u32>(self.scale_factor());
self.queue_event(WindowEvent::Resized(size));
self.queue_event(WindowEvent::SurfaceResized(size));
}

#[method(drawRect:)]
Expand Down
Loading