From 3e203fea3c2f4e670e9ee56a25a2a887d74f980e Mon Sep 17 00:00:00 2001 From: Brandon Fowler Date: Fri, 1 Nov 2024 19:51:45 -0400 Subject: [PATCH] Use NonZeroU32 for popup IDs --- api/cpp/include/slint_window.h | 6 +++++- internal/compiler/generator/rust.rs | 12 +++++------- internal/core/window.rs | 20 +++++++++++--------- internal/interpreter/dynamic_item_tree.rs | 12 ++++++------ 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/api/cpp/include/slint_window.h b/api/cpp/include/slint_window.h index 990dc272d49..db105f51d71 100644 --- a/api/cpp/include/slint_window.h +++ b/api/cpp/include/slint_window.h @@ -117,7 +117,11 @@ class WindowAdapterRc &parent_item); } - void close_popup(uint32_t popup_id) const { cbindgen_private::slint_windowrc_close_popup(&inner, popup_id); } + void close_popup(uint32_t popup_id) const { + if (popup_id > 0) { + cbindgen_private::slint_windowrc_close_popup(&inner, popup_id); + } + } template F> std::optional set_rendering_notifier(F callback) const diff --git a/internal/compiler/generator/rust.rs b/internal/compiler/generator/rust.rs index c9b10b8e4c6..c6238b65551 100644 --- a/internal/compiler/generator/rust.rs +++ b/internal/compiler/generator/rust.rs @@ -1119,7 +1119,7 @@ fn generate_sub_component( struct #inner_component_id { #(#item_names : sp::#item_types,)* #(#sub_component_names : #sub_component_types,)* - #(#popup_id_names : ::core::cell::Cell,)* + #(#popup_id_names : ::core::cell::Cell>,)* #(#declared_property_vars : sp::Property<#declared_property_types>,)* #(#declared_callbacks : sp::Callback<(#(#declared_callbacks_types,)*), #declared_callbacks_ret>,)* #(#repeated_element_names : sp::Repeater<#repeated_element_components>,)* @@ -2676,17 +2676,16 @@ fn compile_builtin_function_call( let popup_instance_vrc = sp::VRc::map(popup_instance.clone(), |x| x); #popup_window_id::user_init(popup_instance_vrc.clone()); let position = { let _self = popup_instance_vrc.as_pin_ref(); #position }; - let current_id = #component_access_tokens.#popup_id_name.take(); - if current_id > 0 { + if let Some(current_id) = #component_access_tokens.#popup_id_name.take() { sp::WindowInner::from_pub(#window_adapter_tokens.window()).close_popup(current_id); } - #component_access_tokens.#popup_id_name.replace( + #component_access_tokens.#popup_id_name.set(Some( sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_popup( &sp::VRc::into_dyn(popup_instance.into()), position, #close_policy, #parent_component - ) + )) ); }) } else { @@ -2709,8 +2708,7 @@ fn compile_builtin_function_call( let window_adapter_tokens = access_window_adapter_field(ctx); let popup_id_name = internal_popup_id(*popup_index as usize); quote!( - let current_id = #component_access_tokens.#popup_id_name.take(); - if current_id > 0 { + if let Some(current_id) = #component_access_tokens.#popup_id_name.take() { sp::WindowInner::from_pub(#window_adapter_tokens.window()).close_popup(current_id); } ) diff --git a/internal/core/window.rs b/internal/core/window.rs index ca5d3d82741..e23473a2e55 100644 --- a/internal/core/window.rs +++ b/internal/core/window.rs @@ -28,6 +28,7 @@ use alloc::rc::{Rc, Weak}; #[cfg(not(feature = "std"))] use alloc::vec::Vec; use core::cell::{Cell, RefCell}; +use core::num::NonZeroU32; use core::pin::Pin; use euclid::num::Zero; use vtable::VRcMapped; @@ -385,7 +386,7 @@ enum PopupWindowLocation { /// UI content, for example to show a context menu. struct PopupWindow { /// The ID of the associated popup. - popup_id: u32, + popup_id: NonZeroU32, /// The location defines where the pop up is rendered. location: PopupWindowLocation, /// The component that is responsible for providing the popup content. @@ -436,7 +437,7 @@ pub struct WindowInner { /// Stack of currently active popups active_popups: RefCell>, - popup_counter: Cell, + next_popup_id: Cell, had_popup_on_press: Cell, close_requested: Callback<(), CloseRequestResponse>, click_state: ClickState, @@ -498,7 +499,7 @@ impl WindowInner { last_ime_text: Default::default(), cursor_blinker: Default::default(), active_popups: Default::default(), - popup_counter: Default::default(), + next_popup_id: Cell::new(NonZeroU32::MIN), had_popup_on_press: Default::default(), close_requested: Default::default(), click_state: ClickState::default(), @@ -983,7 +984,7 @@ impl WindowInner { position: Point, close_policy: PopupClosePolicy, parent_item: &ItemRc, - ) -> u32 { + ) -> NonZeroU32 { let position = parent_item.map_to_window( parent_item.geometry().origin + LogicalPoint::from_untyped(position).to_vector(), ); @@ -1051,8 +1052,8 @@ impl WindowInner { height_property.set(size.height_length()); }; - self.popup_counter.set(self.popup_counter.get() + 1); - let popup_id = self.popup_counter.get(); + let popup_id = self.next_popup_id.get(); + self.next_popup_id.set(self.next_popup_id.get().checked_add(1).unwrap()); let location = match parent_window_adapter .internal(crate::InternalToken) @@ -1110,7 +1111,7 @@ impl WindowInner { } /// Removes the popup matching the given ID. - pub fn close_popup(&self, popup_id: u32) { + pub fn close_popup(&self, popup_id: NonZeroU32) { let mut active_popups = self.active_popups.borrow_mut(); let maybe_index = active_popups.iter().position(|popup| popup.popup_id == popup_id); @@ -1402,7 +1403,7 @@ pub mod ffi { position: crate::graphics::Point, close_policy: PopupClosePolicy, parent_item: &ItemRc, - ) -> u32 { + ) -> NonZeroU32 { let window_adapter = &*(handle as *const Rc); return WindowInner::from_pub(window_adapter.window()).show_popup( popup, @@ -1411,11 +1412,12 @@ pub mod ffi { parent_item, ); } + /// Close the popup by the given ID. #[no_mangle] pub unsafe extern "C" fn slint_windowrc_close_popup( handle: *const WindowAdapterRcOpaque, - popup_id: u32, + popup_id: NonZeroU32, ) { let window_adapter = &*(handle as *const Rc); WindowInner::from_pub(window_adapter.window()).close_popup(popup_id); diff --git a/internal/interpreter/dynamic_item_tree.rs b/internal/interpreter/dynamic_item_tree.rs index 4bc6fbb6dad..fcbe5d83975 100644 --- a/internal/interpreter/dynamic_item_tree.rs +++ b/internal/interpreter/dynamic_item_tree.rs @@ -42,6 +42,7 @@ use once_cell::unsync::{Lazy, OnceCell}; use smol_str::{SmolStr, ToSmolStr}; use std::collections::BTreeMap; use std::collections::HashMap; +use std::num::NonZeroU32; use std::{pin::Pin, rc::Rc}; pub const SPECIAL_PROPERTY_INDEX: &str = "$index"; @@ -412,7 +413,7 @@ pub struct ItemTreeDescription<'id> { )>, timers: Vec, Timer>>, /// ID of the active popup - popup_id: std::cell::Cell, + popup_id: std::cell::Cell>, /// The collection of compiled globals compiled_globals: Option>, @@ -1343,7 +1344,7 @@ pub(crate) fn generate_item_tree<'id>( compiled_globals, change_trackers, timers, - popup_id: std::cell::Cell::from(0), + popup_id: std::cell::Cell::from(None), #[cfg(feature = "highlight")] type_loader: std::cell::OnceCell::new(), #[cfg(feature = "highlight")] @@ -2360,19 +2361,18 @@ pub fn show_popup( pos_getter(instance_ref) }; close_popup(instance, parent_window_adapter.clone()); - instance.description.popup_id.replace( + instance.description.popup_id.set(Some( WindowInner::from_pub(parent_window_adapter.window()).show_popup( &vtable::VRc::into_dyn(inst), pos, close_policy, parent_item, ), - ); + )); } pub fn close_popup(instance: InstanceRef, parent_window_adapter: WindowAdapterRc) { - let current_id = instance.description.popup_id.take(); - if current_id > 0 { + if let Some(current_id) = instance.description.popup_id.take() { WindowInner::from_pub(parent_window_adapter.window()).close_popup(current_id); } }