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

madsmtm · 2024-08-24T21:53:05Z

As discussed in #2308, using "inner" in the name is confusing, and could mean many different things. Let's rename it to "surface", and define that to mean the size of the drawable surface, i.e. the dimensions that wgpu and glutin should be configured with.

See the changelog entry for the full list of changed APIs.

Some backends (at least iOS/UIKit) are still not be returning the actual surface size, but I wanted to address that separately (along with introducing Window::safe_area).

Tested on all platforms changed
Added an entry to the changelog module if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior

To make it clear which size the increments apply to.

MarijnS95

Makes a lot of sense for Android, but it looks like there's some more (slightly irrelevant to this PR) cruft in its backend to clean up.

MarijnS95 · 2024-08-24T21:56:39Z

src/platform_impl/android/mod.rs

                    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)));


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)?

MarijnS95 · 2024-08-24T21:58:41Z

src/platform_impl/android/mod.rs

                    let old_scale_factor = scale_factor(&self.android_app);
                    let scale_factor = scale_factor(&self.android_app);


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

MarijnS95 · 2024-08-24T21:59:31Z

src/platform_impl/android/mod.rs

+    fn surface_size(&self) -> PhysicalSize<u32> {
        self.outer_size()


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

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

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...

madsmtm · 2024-08-24T22:13:34Z

I will clean up the iOS backend, and attempt the same for the Android backend later (and will use your comments as a guide), but I want to keep it out if this PR which is strictly a rename

kchibisov · 2024-08-25T00:47:20Z

inner_size is the rendering surface size.
outer_size is the size of the window with other stuff (like decorations).
resize_increments is resizing step for the window, not surface.

safe area is completely different concept and doesn't affect any rendering or buffers created, in fact, using safe area as a size for rendering surface is completely wrong. Safe area should only be used to offset your content within the surfare_size buffer.

madsmtm · 2024-08-25T01:11:18Z

inner_size is the rendering surface size.

Yeah, and that's why I'm changing it to be named that way.

resize_increments is resizing step for the window, not surface.

Not on macOS at least, we set contentResizeIncrements there, see discussion here.

Also IMO what's desired, you don't care about window decorations when setting resize increments.

safe area is completely different concept and doesn't affect any rendering or buffers created, in fact, using safe area as a size for rendering surface is completely wrong. Safe area should only be used to offset your content within the surfare_size buffer.

I'm aware, and I intend to address this discrepancy in a different PR than this one. EDIT: See #3890

xorgy · 2024-08-25T23:39:44Z

This is a major improvement for us as well. I can wire through a basic Android implementation of inner vs surface sizing, if you don't want to @madsmtm .

madsmtm · 2024-08-26T03:18:43Z

Yeah, I think I could use some help with the Android backend, if you or Marijin could open a PR targetting the branch in #3890, that'd be nice!

MarijnS95 · 2024-08-26T09:25:04Z

@madsmtm unfortunately on Android that info only seems to be available inside DisplayCutout which is some more JNI that I'm not ready to have in winit just yet - but we could/should push it into android-activity instead.

madsmtm · 2024-08-26T15:53:38Z

@madsmtm unfortunately on Android that info only seems to be available inside DisplayCutout which is some more JNI that I'm not ready to have in winit just yet - but we could/should push it into android-activity instead.

Unfortunate. But then I'd be happy with just ensuring that the implementation for surface_position is correct, and then return (0, 0), (width, height) from safe_area on Android for now.

xorgy · 2024-08-27T01:04:46Z

@madsmtm unfortunately on Android that info only seems to be available inside DisplayCutout which is some more JNI that I'm not ready to have in winit just yet - but we could/should push it into android-activity instead.

Shouldn't be a huge mess, just a bit of ugly JNI. Mads' model for a safe area might also involve specifically dealing with RoundedCorner (which affects the safe rectangle in fullscreen mode, but generally not otherwise).

daxpedda

LGTM, very nice improvement!

kchibisov · 2024-09-04T13:05:10Z

src/window.rs

+    ///
+    /// See [`Window::set_surface_resize_increments`] for details.
+    #[inline]
+    pub fn with_surface_resize_increments<S: Into<Size>>(


Resize increments are for window.

kchibisov · 2024-09-04T13:05:31Z

src/window.rs

    ///
    /// ## Platform-specific
    ///
    /// - **iOS / Android / Web / Wayland / Orbital:** Always returns [`None`].
-    fn resize_increments(&self) -> Option<PhysicalSize<u32>>;
+    fn surface_resize_increments(&self) -> Option<PhysicalSize<u32>>;


it's window.

kchibisov · 2024-09-04T13:05:37Z

src/window.rs

    ///
    /// ## Platform-specific
    ///
    /// - **macOS:** Increments are converted to logical size and then macOS rounds them to whole
    ///   numbers.
    /// - **Wayland:** Not implemented.
    /// - **iOS / Android / Web / Orbital:** Unsupported.
-    fn set_resize_increments(&self, increments: Option<Size>);
+    fn set_surface_resize_increments(&self, increments: Option<Size>);


madsmtm · 2024-09-04T13:09:32Z

I've opened #3900 for the resize increments discussion

madsmtm added 5 commits August 24, 2024 23:13

Rename WindowEvent::Resized to SurfaceResized

6ba3171

Rename InnerSizeWriter to SurfaceSizeWriter

a560dfb

Replace inner_size with surface_size

528f939

Rename resize_increments to surface_resize_increments

faa2d95

To make it clear which size the increments apply to.

Fix documentation

8305a7c

madsmtm added S - api Design and usability S - docs Awareness, docs, examples, etc. I - BREAKING CHANGE labels Aug 24, 2024

madsmtm requested review from jackpot51, notgull, daxpedda, kchibisov and MarijnS95 as code owners August 24, 2024 21:53

madsmtm removed request for MarijnS95 and jackpot51 August 24, 2024 21:53

MarijnS95 approved these changes Aug 24, 2024

View reviewed changes

madsmtm mentioned this pull request Aug 26, 2024

Add safe area and document coordinate systems #3890

Merged

daxpedda approved these changes Sep 4, 2024

View reviewed changes

Merge branch 'master' into madsmtm/inner-to-surface

ed16345

madsmtm merged commit 8db3e0e into master Sep 4, 2024
58 checks passed

madsmtm deleted the madsmtm/inner-to-surface branch September 4, 2024 13:04

kchibisov reviewed Sep 4, 2024

View reviewed changes

madsmtm mentioned this pull request Sep 4, 2024

Which dimensions should resize increments be relative to? #3900

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

madsmtm commented Aug 24, 2024 •

edited

Loading

MarijnS95 left a comment

MarijnS95 Aug 24, 2024

MarijnS95 Aug 24, 2024

MarijnS95 Aug 24, 2024

madsmtm Aug 24, 2024 •

edited

Loading

MarijnS95 Aug 24, 2024

madsmtm commented Aug 24, 2024

kchibisov commented Aug 25, 2024

madsmtm commented Aug 25, 2024 •

edited

Loading

xorgy commented Aug 25, 2024 •

edited

Loading

madsmtm commented Aug 26, 2024

MarijnS95 commented Aug 26, 2024

madsmtm commented Aug 26, 2024

xorgy commented Aug 27, 2024

daxpedda left a comment

kchibisov Sep 4, 2024

kchibisov Sep 4, 2024

kchibisov Sep 4, 2024

madsmtm commented Sep 4, 2024

		let old_scale_factor = scale_factor(&self.android_app);
		let scale_factor = scale_factor(&self.android_app);

		fn surface_size(&self) -> PhysicalSize<u32> {
		self.outer_size()

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

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

Conversation

madsmtm commented Aug 24, 2024 • edited Loading

MarijnS95 left a comment

Choose a reason for hiding this comment

MarijnS95 Aug 24, 2024

Choose a reason for hiding this comment

MarijnS95 Aug 24, 2024

Choose a reason for hiding this comment

MarijnS95 Aug 24, 2024

Choose a reason for hiding this comment

madsmtm Aug 24, 2024 • edited Loading

Choose a reason for hiding this comment

MarijnS95 Aug 24, 2024

Choose a reason for hiding this comment

madsmtm commented Aug 24, 2024

kchibisov commented Aug 25, 2024

madsmtm commented Aug 25, 2024 • edited Loading

xorgy commented Aug 25, 2024 • edited Loading

madsmtm commented Aug 26, 2024

MarijnS95 commented Aug 26, 2024

madsmtm commented Aug 26, 2024

xorgy commented Aug 27, 2024

daxpedda left a comment

Choose a reason for hiding this comment

kchibisov Sep 4, 2024

Choose a reason for hiding this comment

kchibisov Sep 4, 2024

Choose a reason for hiding this comment

kchibisov Sep 4, 2024

Choose a reason for hiding this comment

madsmtm commented Sep 4, 2024

madsmtm commented Aug 24, 2024 •

edited

Loading

madsmtm Aug 24, 2024 •

edited

Loading

madsmtm commented Aug 25, 2024 •

edited

Loading

xorgy commented Aug 25, 2024 •

edited

Loading