Skip to content

Make checkboxes not use interior mutability #2976

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.

Sign up for GitHub

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

Merged
Keavon merged 3 commits into from
Aug 3, 2025
Merged

Make checkboxes not use interior mutability #2976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1a4637
Make checkboxes not use interior mutability
timon-schelling Aug 3, 2025
dd6f28a
Use copy instead of cloning
timon-schelling Aug 3, 2025
af1da07
Fix
Keavon Aug 3, 2025
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
6 changes: 3 additions & 3 deletions editor/src/messages/dialog/export_dialog/export_dialog_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,14 @@ impl LayoutHolder for ExportDialogMessageHandler {
DropdownInput::new(entries).selected_index(Some(index as u32)).widget_holder(),
];

let mut checkbox_id = CheckboxId::default();
let checkbox_id = CheckboxId::new();
let transparent_background = vec![
TextLabel::new("Transparency").table_align(true).min_width(100).for_checkbox(&mut checkbox_id).widget_holder(),
TextLabel::new("Transparency").table_align(true).min_width(100).for_checkbox(checkbox_id).widget_holder(),
Separator::new(SeparatorType::Unrelated).widget_holder(),
CheckboxInput::new(self.transparent_background)
.disabled(self.file_type == FileType::Jpg)
.on_update(move |value: &CheckboxInput| ExportDialogMessage::TransparentBackground(value.checked).into())
.for_label(checkbox_id.clone())
.for_label(checkbox_id)
.widget_holder(),
];

Expand Down
6 changes: 3 additions & 3 deletions editor/src/messages/dialog/new_document_dialog/new_document_dialog_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,13 @@ impl LayoutHolder for NewDocumentDialogMessageHandler {
.widget_holder(),
];

let mut checkbox_id = CheckboxId::default();
let checkbox_id = CheckboxId::new();
let infinite = vec![
TextLabel::new("Infinite Canvas").table_align(true).min_width(90).for_checkbox(&mut checkbox_id).widget_holder(),
TextLabel::new("Infinite Canvas").table_align(true).min_width(90).for_checkbox(checkbox_id).widget_holder(),
Separator::new(SeparatorType::Unrelated).widget_holder(),
CheckboxInput::new(self.infinite)
.on_update(|checkbox_input: &CheckboxInput| NewDocumentDialogMessage::Infinite(checkbox_input.checked).into())
.for_label(checkbox_id.clone())
.for_label(checkbox_id)
.widget_holder(),
];

Expand Down
22 changes: 9 additions & 13 deletions editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl PreferencesDialogMessageHandler {
.widget_holder(),
];

let mut checkbox_id = CheckboxId::default();
let checkbox_id = CheckboxId::new();
let zoom_with_scroll_tooltip = "Use the scroll wheel for zooming instead of vertically panning (not recommended for trackpads)";
let zoom_with_scroll = vec![
Separator::new(SeparatorType::Unrelated).widget_holder(),
Expand All @@ -81,12 +81,12 @@ impl PreferencesDialogMessageHandler {
}
.into()
})
.for_label(checkbox_id.clone())
.for_label(checkbox_id)
.widget_holder(),
TextLabel::new("Zoom with Scroll")
.table_align(true)
.tooltip(zoom_with_scroll_tooltip)
.for_checkbox(&mut checkbox_id)
.for_checkbox(checkbox_id)
.widget_holder(),
];

Expand Down Expand Up @@ -169,7 +169,7 @@ impl PreferencesDialogMessageHandler {
graph_wire_style,
];

let mut checkbox_id = CheckboxId::default();
let checkbox_id = CheckboxId::new();
let vello_tooltip = "Use the experimental Vello renderer (your browser must support WebGPU)";
let use_vello = vec![
Separator::new(SeparatorType::Unrelated).widget_holder(),
Expand All @@ -178,17 +178,17 @@ impl PreferencesDialogMessageHandler {
.tooltip(vello_tooltip)
.disabled(!preferences.supports_wgpu())
.on_update(|checkbox_input: &CheckboxInput| PreferencesMessage::UseVello { use_vello: checkbox_input.checked }.into())
.for_label(checkbox_id.clone())
.for_label(checkbox_id)
.widget_holder(),
TextLabel::new("Vello Renderer")
.table_align(true)
.tooltip(vello_tooltip)
.disabled(!preferences.supports_wgpu())
.for_checkbox(&mut checkbox_id)
.for_checkbox(checkbox_id)
.widget_holder(),
];

let mut checkbox_id = CheckboxId::default();
let checkbox_id = CheckboxId::new();
let vector_mesh_tooltip =
"Allow tools to produce vector meshes, where more than two segments can connect to an anchor point.\n\nCurrently this does not properly handle stroke joins and fills.";
let vector_meshes = vec![
Expand All @@ -197,13 +197,9 @@ impl PreferencesDialogMessageHandler {
CheckboxInput::new(preferences.vector_meshes)
.tooltip(vector_mesh_tooltip)
.on_update(|checkbox_input: &CheckboxInput| PreferencesMessage::VectorMeshes { enabled: checkbox_input.checked }.into())
.for_label(checkbox_id.clone())
.widget_holder(),
TextLabel::new("Vector Meshes")
.table_align(true)
.tooltip(vector_mesh_tooltip)
.for_checkbox(&mut checkbox_id)
.for_label(checkbox_id)
.widget_holder(),
TextLabel::new("Vector Meshes").table_align(true).tooltip(vector_mesh_tooltip).for_checkbox(checkbox_id).widget_holder(),
];

Layout::WidgetLayout(WidgetLayout::new(vec![
Expand Down
44 changes: 11 additions & 33 deletions editor/src/messages/layout/utility_types/widgets/input_widgets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ use graphene_std::Color;
use graphene_std::raster::curve::Curve;
use graphene_std::transform::ReferencePoint;
use graphite_proc_macros::WidgetBuilder;
use once_cell::sync::OnceCell;
use std::sync::Arc;

#[derive(Clone, Derivative, serde::Serialize, serde::Deserialize, WidgetBuilder, specta::Type)]
#[derivative(Debug, PartialEq)]
Expand All @@ -20,7 +18,7 @@ pub struct CheckboxInput {

pub tooltip: String,

#[serde(rename = "forLabel", skip_serializing_if = "checkbox_id_is_empty")]
#[serde(rename = "forLabel")]
pub for_label: CheckboxId,

#[serde(skip)]
Expand All @@ -44,19 +42,24 @@ impl Default for CheckboxInput {
icon: "Checkmark".into(),
tooltip: Default::default(),
tooltip_shortcut: Default::default(),
for_label: CheckboxId::default(),
for_label: CheckboxId::new(),
on_update: Default::default(),
on_commit: Default::default(),
}
}
}

#[derive(Clone, Default, Debug, Eq, PartialEq)]
pub struct CheckboxId(Arc<OnceCell<u64>>);
#[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct CheckboxId(u64);

impl CheckboxId {
pub fn fill(&mut self) {
let _ = self.0.set(graphene_std::uuid::generate_uuid());
pub fn new() -> Self {
Self(graphene_std::uuid::generate_uuid())
}
}
impl Default for CheckboxId {
fn default() -> Self {
Self::new()
}
}
impl specta::Type for CheckboxId {
Expand All @@ -65,31 +68,6 @@ impl specta::Type for CheckboxId {
specta::datatype::DataType::Primitive(specta::datatype::PrimitiveType::u64)
}
}
impl serde::Serialize for CheckboxId {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
self.0.get().copied().serialize(serializer)
}
}
impl<'a> serde::Deserialize<'a> for CheckboxId {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'a>,
{
let optional_id: Option<u64> = Option::deserialize(deserializer)?;
// TODO: This is potentially weird because after deserialization the two labels will be decoupled if the value not existent
let id = optional_id.unwrap_or(0);
let checkbox_id = CheckboxId(OnceCell::new().into());
checkbox_id.0.set(id).map_err(serde::de::Error::custom)?;
Ok(checkbox_id)
}
}

fn checkbox_id_is_empty(id: &CheckboxId) -> bool {
id.0.get().is_none()
}

#[derive(Clone, serde::Serialize, serde::Deserialize, Derivative, WidgetBuilder, specta::Type)]
#[derivative(Debug, PartialEq, Default)]
Expand Down
13 changes: 2 additions & 11 deletions editor/src/messages/layout/utility_types/widgets/label_widgets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,12 @@ pub struct TextLabel {

pub tooltip: String,

#[serde(rename = "checkboxId")]
#[widget_builder(skip)]
pub checkbox_id: CheckboxId,
#[serde(rename = "forCheckbox")]
pub for_checkbox: CheckboxId,

// Body
#[widget_builder(constructor)]
pub value: String,
}

impl TextLabel {
pub fn for_checkbox(mut self, id: &mut CheckboxId) -> Self {
id.fill();
self.checkbox_id = id.clone();
self
}
}

// TODO: Add UserInputLabel
Loading
Loading