Remove Option<Option<X>> and simplify ViewportBuilder

crates/eframe/src/native/epi_integration.rs

@@ -1,4 +1,4 @@
-use std::{sync::Arc, time::Instant};
+use std::time::Instant;
 
 use winit::event_loop::EventLoopWindowTarget;
 
@@ -92,17 +92,19 @@ pub fn window_builder<E>(
         .with_maximized(*maximized)
         .with_resizable(*resizable)
         .with_transparent(*transparent)
-        .with_window_icon(icon_data.clone().map(|d| {
-            Arc::new(egui::ColorImage::from_rgba_premultiplied(
-                [d.width as usize, d.height as usize],
-                &d.rgba,
-            ))
-        }))
         .with_active(*active)
         // Keep hidden until we've painted something. See https://github.com/emilk/egui/pull/2279
         // We must also keep the window hidden until AccessKit is initialized.
         .with_visible(false);
 
+    if let Some(icon_data) = icon_data {
+        viewport_builder =
+            viewport_builder.with_window_icon(egui::ColorImage::from_rgba_premultiplied(
+                [icon_data.width as usize, icon_data.height as usize],
+                &icon_data.rgba,
+            ));
+    }
+
     #[cfg(target_os = "macos")]
     if *fullsize_content {
         viewport_builder = viewport_builder
@@ -120,10 +122,10 @@ pub fn window_builder<E>(
     }
 
     if let Some(min_size) = *min_window_size {
-        viewport_builder = viewport_builder.with_min_inner_size(Some(min_size));
+        viewport_builder = viewport_builder.with_min_inner_size(min_size);
     }
     if let Some(max_size) = *max_window_size {
-        viewport_builder = viewport_builder.with_max_inner_size(Some(max_size));
+        viewport_builder = viewport_builder.with_max_inner_size(max_size);
     }
 
     viewport_builder = viewport_builder.with_drag_and_drop(*drag_and_drop_support);
@@ -141,13 +143,13 @@ pub fn window_builder<E>(
         window_settings.inner_size_points()
     } else {
         if let Some(pos) = *initial_window_pos {
-            viewport_builder = viewport_builder.with_position(Some(pos));
+            viewport_builder = viewport_builder.with_position(pos);
         }
 
         if let Some(initial_window_size) = *initial_window_size {
             let initial_window_size =
                 initial_window_size.at_most(largest_monitor_point_size(event_loop));
-            viewport_builder = viewport_builder.with_inner_size(Some(initial_window_size));
+            viewport_builder = viewport_builder.with_inner_size(initial_window_size);
         }
 
         *initial_window_size
@@ -161,7 +163,7 @@ pub fn window_builder<E>(
             if monitor_size.width > 0.0 && monitor_size.height > 0.0 {
                 let x = (monitor_size.width - inner_size.x) / 2.0;
                 let y = (monitor_size.height - inner_size.y) / 2.0;
-                viewport_builder = viewport_builder.with_position(Some(egui::Pos2::new(x, y)));
+                viewport_builder = viewport_builder.with_position([x, y]);
             }
         }
     }

crates/egui-winit/src/lib.rs

@@ -1035,10 +1035,14 @@ pub fn process_viewport_commands(
                 window.set_outer_position(LogicalPosition::new(pos.x, pos.y));
             }
             ViewportCommand::MinInnerSize(s) => {
-                window.set_min_inner_size(s.map(|s| LogicalSize::new(s.x, s.y)));
+                window.set_min_inner_size(
+                    (s.is_finite() && s != Vec2::ZERO).then_some(LogicalSize::new(s.x, s.y)),
+                );
             }
             ViewportCommand::MaxInnerSize(s) => {
-                window.set_max_inner_size(s.map(|s| LogicalSize::new(s.x, s.y)));
+                window.set_max_inner_size(
+                    (s.is_finite() && s != Vec2::INFINITY).then_some(LogicalSize::new(s.x, s.y)),
+                );
             }
             ViewportCommand::ResizeIncrements(s) => {
                 window.set_resize_increments(s.map(|s| LogicalSize::new(s.x, s.y)));
@@ -1168,31 +1172,31 @@ pub fn create_winit_window_builder(builder: &ViewportBuilder) -> winit::window::
         })
         .with_active(builder.active.unwrap_or(true));
 
-    if let Some(Some(inner_size)) = builder.inner_size {
+    if let Some(inner_size) = builder.inner_size {
         window_builder = window_builder
             .with_inner_size(winit::dpi::LogicalSize::new(inner_size.x, inner_size.y));
     }
 
-    if let Some(Some(min_inner_size)) = builder.min_inner_size {
+    if let Some(min_inner_size) = builder.min_inner_size {
         window_builder = window_builder.with_min_inner_size(winit::dpi::LogicalSize::new(
             min_inner_size.x,
             min_inner_size.y,
         ));
     }
 
-    if let Some(Some(max_inner_size)) = builder.max_inner_size {
+    if let Some(max_inner_size) = builder.max_inner_size {
         window_builder = window_builder.with_max_inner_size(winit::dpi::LogicalSize::new(
             max_inner_size.x,
             max_inner_size.y,
         ));
     }
 
-    if let Some(Some(position)) = builder.position {
+    if let Some(position) = builder.position {
         window_builder =
             window_builder.with_position(winit::dpi::LogicalPosition::new(position.x, position.y));
     }
 
-    if let Some(Some(icon)) = builder.icon.clone() {
+    if let Some(icon) = builder.icon.clone() {
         window_builder = window_builder.with_window_icon(Some(
             winit::window::Icon::from_rgba(
                 icon.as_raw().to_owned(),

crates/egui-winit/src/window_settings.rs

@@ -60,12 +60,12 @@ impl WindowSettings {
             self.outer_position_pixels
         };
         if let Some(pos) = pos_px {
-            viewport_builder = viewport_builder.with_position(Some(pos));
+            viewport_builder = viewport_builder.with_position(pos);
         }
 
         if let Some(inner_size_points) = self.inner_size_points {
             viewport_builder = viewport_builder
-                .with_inner_size(Some(inner_size_points))
+                .with_inner_size(inner_size_points)
                 .with_fullscreen(self.fullscreen);
         }
 

crates/egui/src/viewport.rs

@@ -112,21 +112,22 @@ pub struct ViewportBuilder {
     /// This is wayland only. See [`Self::with_name`].
     pub name: Option<(String, String)>,
 
-    pub position: Option<Option<Pos2>>,
+    pub position: Option<Pos2>,
-    pub inner_size: Option<Option<Vec2>>,
+    pub inner_size: Option<Vec2>,
+    pub min_inner_size: Option<Vec2>,
+    pub max_inner_size: Option<Vec2>,
+
     pub fullscreen: Option<bool>,
     pub maximized: Option<bool>,
     pub resizable: Option<bool>,
     pub transparent: Option<bool>,
     pub decorations: Option<bool>,
-    pub icon: Option<Option<Arc<ColorImage>>>,
+    pub icon: Option<Arc<ColorImage>>,
     pub active: Option<bool>,
     pub visible: Option<bool>,
     pub title_hidden: Option<bool>,
     pub titlebar_transparent: Option<bool>,
     pub fullsize_content_view: Option<bool>,
-    pub min_inner_size: Option<Option<Vec2>>,
-    pub max_inner_size: Option<Option<Vec2>>,
     pub drag_and_drop: Option<bool>,
 
     pub close_button: Option<bool>,
@@ -209,8 +210,8 @@ impl ViewportBuilder {
 
     /// The icon needs to be wrapped in Arc because will be cloned every frame
     #[inline]
-    pub fn with_window_icon(mut self, icon: Option<Arc<ColorImage>>) -> Self {
+    pub fn with_window_icon(mut self, icon: impl Into<Arc<ColorImage>>) -> Self {
-        self.icon = Some(icon);
+        self.icon = Some(icon.into());
         self
     }
 
@@ -274,8 +275,8 @@ impl ViewportBuilder {
     /// Should be bigger then 0
     /// Look at winit for more details
     #[inline]
-    pub fn with_inner_size(mut self, value: Option<Vec2>) -> Self {
+    pub fn with_inner_size(mut self, size: impl Into<Vec2>) -> Self {
-        self.inner_size = Some(value);
+        self.inner_size = Some(size.into());
         self
     }
 
@@ -287,8 +288,8 @@ impl ViewportBuilder {
     /// Should be bigger then 0
     /// Look at winit for more details
     #[inline]
-    pub fn with_min_inner_size(mut self, value: Option<Vec2>) -> Self {
+    pub fn with_min_inner_size(mut self, size: impl Into<Vec2>) -> Self {
-        self.min_inner_size = Some(value);
+        self.min_inner_size = Some(size.into());
         self
     }
 
@@ -300,8 +301,8 @@ impl ViewportBuilder {
     /// Should be bigger then 0
     /// Look at winit for more details
     #[inline]
-    pub fn with_max_inner_size(mut self, value: Option<Vec2>) -> Self {
+    pub fn with_max_inner_size(mut self, size: impl Into<Vec2>) -> Self {
-        self.max_inner_size = Some(value);
+        self.max_inner_size = Some(size.into());
         self
     }
 
@@ -335,8 +336,8 @@ impl ViewportBuilder {
 
     /// This will probably not work as expected!
     #[inline]
-    pub fn with_position(mut self, value: Option<Pos2>) -> Self {
+    pub fn with_position(mut self, pos: impl Into<Pos2>) -> Self {
-        self.position = Some(value);
+        self.position = Some(pos.into());
         self
     }
 
@@ -378,18 +379,14 @@ impl ViewportBuilder {
         if let Some(new_position) = new.position {
             if Some(new_position) != self.position {
                 self.position = Some(new_position);
-                if let Some(position) = new_position {
+                commands.push(ViewportCommand::OuterPosition(new_position));
-                    commands.push(ViewportCommand::OuterPosition(position));
-                }
             }
         }
 
         if let Some(new_inner_size) = new.inner_size {
             if Some(new_inner_size) != self.inner_size {
                 self.inner_size = Some(new_inner_size);
-                if let Some(inner_size) = new_inner_size {
+                commands.push(ViewportCommand::InnerSize(new_inner_size));
-                    commands.push(ViewportCommand::InnerSize(inner_size));
-                }
             }
         }
 
@@ -442,23 +439,15 @@ impl ViewportBuilder {
             }
         }
 
-        if let Some(new_icon) = new.icon.clone() {
+        if let Some(new_icon) = &new.icon {
-            let eq = match &new_icon {
+            let is_new = match &self.icon {
-                Some(icon) => {
+                Some(existing) => !Arc::ptr_eq(new_icon, existing),
-                    if let Some(self_icon) = &self.icon {
+                None => true,
-                        matches!(self_icon, Some(self_icon) if Arc::ptr_eq(icon, self_icon))
-                    } else {
-                        false
-                    }
-                }
-                None => self.icon == Some(None),
             };
 
-            if !eq {
+            if is_new {
-                commands.push(ViewportCommand::WindowIcon(
+                commands.push(ViewportCommand::WindowIcon(Some(new_icon.clone())));
-                    new_icon.as_ref().map(|i| i.as_ref().clone()),
+                self.icon = Some(new_icon.clone());
-                ));
-                self.icon = Some(new_icon);
             }
         }
 
@@ -587,6 +576,8 @@ pub enum ResizeDirection {
 /// You can send a [`ViewportCommand`] to the viewport with [`Context::viewport_command`].
 ///
 /// All coordinates are in logical points.
+///
+/// This is essentially a way to diff [`ViewportBuilder`].
 #[derive(Clone, Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub enum ViewportCommand {
@@ -612,10 +603,10 @@ pub enum ViewportCommand {
     InnerSize(Vec2),
 
     /// Should be bigger then 0
-    MinInnerSize(Option<Vec2>),
+    MinInnerSize(Vec2),
 
     /// Should be bigger then 0
-    MaxInnerSize(Option<Vec2>),
+    MaxInnerSize(Vec2),
 
     /// Should be bigger then 0
     ResizeIncrements(Option<Vec2>),
@@ -644,7 +635,7 @@ pub enum ViewportCommand {
     Decorations(bool),
 
     WindowLevel(WindowLevel),
-    WindowIcon(Option<ColorImage>),
+    WindowIcon(Option<Arc<ColorImage>>),
 
     IMEPosition(Pos2),
     IMEAllowed(bool),

examples/multiple_viewports/src/main.rs

@@ -52,7 +52,9 @@ impl eframe::App for MyApp {
         if self.show_immediate_viewport {
             ctx.show_viewport_immediate(
                 egui::ViewportId::from_hash_of("immediate_viewport"),
-                egui::ViewportBuilder::default().with_title("Immediate Viewport"),
+                egui::ViewportBuilder::default()
+                    .with_title("Immediate Viewport")
+                    .with_inner_size([200.0, 100.0]),
                 |ctx| {
                     egui::CentralPanel::default().show(ctx, |ui| {
                         ui.label("Hello from immediate viewport");
@@ -70,7 +72,9 @@ impl eframe::App for MyApp {
             let show_deferred_viewport = self.show_deferred_viewport.clone();
             ctx.show_viewport_immediate(
                 egui::ViewportId::from_hash_of("deferred_viewport"),
-                egui::ViewportBuilder::default().with_title("Deferred Viewport"),
+                egui::ViewportBuilder::default()
+                    .with_title("Deferred Viewport")
+                    .with_inner_size([200.0, 100.0]),
                 |ctx| {
                     egui::CentralPanel::default().show(ctx, |ui| {
                         ui.label("Hello from deferred viewport");

examples/test_viewports/src/main.rs

@@ -67,7 +67,7 @@ impl ViewportState {
 
         let viewport = ViewportBuilder::default()
             .with_title(&title)
-            .with_inner_size(Some(egui::vec2(450.0, 400.0)));
+            .with_inner_size([450.0, 400.0]);
 
         if immediate {
             let mut vp_state = vp_state.write();