From 244b25708de4cf398488d16dd2dbbd37f596df68 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 28 Jun 2026 17:34:27 -0700 Subject: [PATCH] Add color picker gradient color stop duplication by holding Alt (#4291) * Add color picker gradient color stop duplication by holding Alt * Fix bugs * Bug fix --- .../color_picker_message_handler.rs | 25 ++++ .../utility_types/widgets/input_widgets.rs | 10 ++ .../node_graph/node_graph_message_handler.rs | 1 + .../widgets/inputs/SpectrumInput.svelte | 131 ++++++++++++++++-- .../libraries/vector-types/src/gradient.rs | 12 ++ 5 files changed, 171 insertions(+), 8 deletions(-) diff --git a/editor/src/messages/color_picker/color_picker_message_handler.rs b/editor/src/messages/color_picker/color_picker_message_handler.rs index 9894e463b9..73361b4588 100644 --- a/editor/src/messages/color_picker/color_picker_message_handler.rs +++ b/editor/src/messages/color_picker/color_picker_message_handler.rs @@ -339,6 +339,31 @@ impl ColorPickerMessageHandler { self.snapshot_old(); } } + SpectrumInputUpdate::InsertDuplicate { index, position } => { + let source = index as usize; + let Some(insert_index) = gradient.duplicate_stop(source, position) else { return }; + // The dragged stop (the duplication source) stays active. Its index shifts up if the frozen copy landed at or before it. + let dragged_index = if insert_index <= source { source + 1 } else { source }; + self.active_marker_index = Some(dragged_index as u32); + self.active_marker_is_midpoint = false; + } + SpectrumInputUpdate::RemoveDuplicate { index } => { + let anchor = index as usize; + if anchor >= gradient.position.len() || gradient.position.len() <= 2 { + return; + } + // Never remove the active (dragged) stop itself, this should only ever target the frozen copy. + if self.active_marker_index == Some(anchor as u32) { + return; + } + gradient.remove(anchor); + // Keep the dragged stop active. Its index shifts down if the removed copy came before it. + if let Some(active) = self.active_marker_index + && (anchor as u32) < active + { + self.active_marker_index = Some(active - 1); + } + } SpectrumInputUpdate::DeleteMarker { index } => { // Enforce minimum stop count. The gradient editor needs at least 2 stops to remain meaningful. if gradient.position.len() <= 2 || (index as usize) >= gradient.position.len() { diff --git a/editor/src/messages/layout/utility_types/widgets/input_widgets.rs b/editor/src/messages/layout/utility_types/widgets/input_widgets.rs index 419d5b3318..727b353f5b 100644 --- a/editor/src/messages/layout/utility_types/widgets/input_widgets.rs +++ b/editor/src/messages/layout/utility_types/widgets/input_widgets.rs @@ -612,6 +612,16 @@ pub enum SpectrumInputUpdate { DeleteMarker { index: u32, }, + /// Insert a copy (same color and midpoint) of the marker at `index` at `position`, keeping the marker at `index` active. + InsertDuplicate { + index: u32, + position: f64, + }, + /// Remove the marker at `index` while keeping the currently active marker active (renumbered to account for the removal). + /// Used to un-duplicate when Alt is released mid-drag, deleting the frozen copy left by [`InsertDuplicate`](Self::InsertDuplicate). + RemoveDuplicate { + index: u32, + }, /// Emitted when the user double-clicks a marker. The consumer decides what (if anything) to reset the marker to. ResetMarker { index: u32, diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index 410b8cfb72..c9023c732d 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -1785,6 +1785,7 @@ impl<'a> MessageHandler> for NodeG responses.add(OverlaysMessage::Draw); } if is_stroke_node || is_fill_node || is_shape_generator_node || is_text_node { + responses.add(SelectToolMessage::SelectionChanged); responses.add(PenToolMessage::SelectionChanged); responses.add(FreehandToolMessage::SelectionChanged); responses.add(SplineToolMessage::SelectionChanged); diff --git a/frontend/src/components/widgets/inputs/SpectrumInput.svelte b/frontend/src/components/widgets/inputs/SpectrumInput.svelte index 68a9142b71..0f31e5e782 100644 --- a/frontend/src/components/widgets/inputs/SpectrumInput.svelte +++ b/frontend/src/components/widgets/inputs/SpectrumInput.svelte @@ -36,6 +36,13 @@ let activeMarkerIsMidpointRestore = false; // Tracks whether a midpoint drag has actually moved by at least one frame, to distinguish click-to-select from drag. let midpointDragged = false; + // Mirrors whether Alt is currently held during the drag (the desired state). + let duplicateRequested = false; + // Mirrors whether a frozen copy currently exists in the gradient (the materialized state). + let duplicateActive = false; + // Set when a key-triggered reconcile inserts/removes the frozen copy, so the next pointer move skips emitting a `MoveMarker` + // that would otherwise race the structural change before Rust has reported the dragged marker's new index. + let skipNextMove = false; function emit(intent: SpectrumInputUpdate) { dispatch("update", intent); @@ -68,6 +75,9 @@ activeMarkerIsMidpointRestore = activeMarkerIsMidpoint; dragRestorePosition = markers[index].position; dragInsertedMarker = false; + // Only offer duplication where new stops are allowed. Don't materialize yet: wait for the first move so an Alt-click without a drag leaves no stray copy. + duplicateRequested = allowInsert && e.altKey; + duplicateActive = false; setActive(index, false); addEvents(); return; @@ -118,7 +128,10 @@ activeMarkerIsMidpointRestore = activeMarkerIsMidpoint; dragRestorePosition = position; dragInsertedMarker = true; - // Don't dispatch an `ActiveMarker` here — the Rust handler already updates the active marker in response to `InsertMarker` and a duplicate `ActiveMarker` would race the layout update. + // A stop being created by this drag can't be duplicated; duplication is only for dragging an existing stop. + duplicateRequested = false; + duplicateActive = false; + // Don't dispatch an `ActiveMarker` here. The Rust handler already updates the active marker in response to `InsertMarker` and a duplicate `ActiveMarker` would race the layout update. activeMarkerIndex = insertIndex; activeMarkerIsMidpoint = false; addEvents(); @@ -134,10 +147,78 @@ else if (allowDelete) emit({ DeleteMarker: { index: activeMarkerIndex } }); } + // Locate the frozen copy left by a duplicate: the non-dragged marker sitting (within epsilon) at the drag's start position. + // Returns undefined if no marker is close enough, so a stale `markers` prop can never delete an unrelated stop. + function findDuplicateAnchorIndex(): number | undefined { + // The frozen copy is inserted at exactly the drag's start position and round-trips through Rust losslessly, so it matches within this tolerance + const DUPLICATE_POSITION_EPSILON = 1e-6; + + if (dragRestorePosition === undefined) return undefined; + + const startPosition = dragRestorePosition; + + let best: number | undefined = undefined; + let bestDistance = DUPLICATE_POSITION_EPSILON; + + markers.forEach((marker, index) => { + if (index === activeMarkerIndex) return; + + const distance = Math.abs(marker.position - startPosition); + if (distance < bestDistance) { + bestDistance = distance; + best = index; + } + }); + + return best; + } + + // Bring the materialized duplicate state in line with whether Alt is currently held, inserting or removing the frozen copy. + // Returns whether a structural change was emitted, so callers can skip the next move that would race it. + function reconcileDuplicate(): boolean { + if (!allowInsert || activeMarkerIndex === undefined || activeMarkerIsMidpoint) return false; + + if (duplicateRequested && !duplicateActive) { + // Drop a frozen copy at the drag's start position. The dragged marker stays active and becomes the duplicate being moved. + + if (dragRestorePosition === undefined) return false; + + emit({ InsertDuplicate: { index: activeMarkerIndex, position: dragRestorePosition } }); + duplicateActive = true; + + return true; + } else if (!duplicateRequested && duplicateActive) { + // Remove the frozen copy so only the dragged marker remains, as if it had been dragged all along. + + const anchor = findDuplicateAnchorIndex(); + if (anchor === undefined) return false; + + emit({ RemoveDuplicate: { index: anchor } }); + duplicateActive = false; + + return true; + } + + return false; + } + function moveActiveMarker(e: PointerEvent) { if (disabled || activeMarkerIndex === undefined) return; if (e.buttons === 0) { - stopDrag(); + endDrag(); + return; + } + + // Materialize/remove the frozen copy if Alt's state changed without a key event reconciling it first (e.g. Alt held at drag start). + // Skip this frame's move. The next move runs once Rust has reported the dragged marker's new index. + if (duplicateRequested !== duplicateActive) { + reconcileDuplicate(); + return; + } + + // A key event (Alt press/release) already reconciled. Skip the one move that would race that structural change. + if (skipNextMove) { + skipNextMove = false; return; } @@ -152,7 +233,7 @@ function moveActiveMidpoint(e: PointerEvent) { if (disabled || activeMarkerIndex === undefined) return; if (e.buttons === 0) { - stopDrag(); + endDrag(); return; } @@ -173,18 +254,30 @@ function abortDrag() { if (disabled || activeMarkerIndex === undefined) return; - // Restore the dragged value, or delete the marker if it was inserted as part of this drag. + const dragged = activeMarkerIndex; + const anchor = duplicateActive ? findDuplicateAnchorIndex() : undefined; + if (dragInsertedMarker) { - emit({ DeleteMarker: { index: activeMarkerIndex } }); + // The dragged marker was created by this drag, so delete it. + emit({ DeleteMarker: { index: dragged } }); + } else if (anchor !== undefined) { + // A duplicated pre-existing marker: the frozen copy already sits at the start position, so deleting the dragged copy restores the original. + emit({ DeleteMarker: { index: dragged } }); } else if (dragRestorePosition !== undefined) { - if (activeMarkerIsMidpoint) emit({ MoveMidpoint: { index: activeMarkerIndex, position: dragRestorePosition } }); - else emit({ MoveMarker: { index: activeMarkerIndex, position: dragRestorePosition } }); + // Plain drag: return the marker (or midpoint) to where it began. + if (activeMarkerIsMidpoint) emit({ MoveMidpoint: { index: dragged, position: dragRestorePosition } }); + else emit({ MoveMarker: { index: dragged, position: dragRestorePosition } }); } setActive(activeMarkerIndexRestore, activeMarkerIsMidpointRestore); stopDrag(); } + function endDrag() { + if (!duplicateRequested && duplicateActive) reconcileDuplicate(); + stopDrag(); + } + function stopDrag() { removeEvents(); dragRestorePosition = undefined; @@ -192,6 +285,9 @@ activeMarkerIndexRestore = undefined; activeMarkerIsMidpointRestore = false; midpointDragged = false; + duplicateRequested = false; + duplicateActive = false; + skipNextMove = false; dispatch("dragging", false); } @@ -201,7 +297,7 @@ } function onPointerUp() { - stopDrag(); + endDrag(); } function onMouseDown(e: MouseEvent) { @@ -214,6 +310,23 @@ const element = markerTrackElement?.div(); if (element) preventEscapeClosingParentFloatingMenu(element); abortDrag(); + return; + } + + // Pressing Alt mid-drag duplicates the marker, leaving a frozen copy where the drag began. Reconcile immediately for instant + // feedback, and arm a skip so the next pointer move doesn't race the just-emitted structural change. Only when dragging an + // existing stop (not one being created by this drag). + if (e.key === "Alt" && allowInsert && !activeMarkerIsMidpoint && !dragInsertedMarker && !duplicateRequested) { + duplicateRequested = true; + if (reconcileDuplicate()) skipNextMove = true; + } + } + + function onKeyUp(e: KeyboardEvent) { + // Releasing Alt mid-drag removes the frozen copy immediately, as if the marker had been dragged all along (likewise armed to skip the next move). + if (e.key === "Alt" && duplicateRequested) { + duplicateRequested = false; + if (reconcileDuplicate()) skipNextMove = true; } } @@ -222,6 +335,7 @@ document.addEventListener("pointerup", onPointerUp); document.addEventListener("mousedown", onMouseDown); document.addEventListener("keydown", onKeyDown); + document.addEventListener("keyup", onKeyUp); } function removeEvents() { @@ -229,6 +343,7 @@ document.removeEventListener("pointerup", onPointerUp); document.removeEventListener("mousedown", onMouseDown); document.removeEventListener("keydown", onKeyDown); + document.removeEventListener("keyup", onKeyUp); } // Map midpoint pairs to absolute track positions for rendering the diamond markers. diff --git a/node-graph/libraries/vector-types/src/gradient.rs b/node-graph/libraries/vector-types/src/gradient.rs index 8dccedfbc0..359cf42684 100644 --- a/node-graph/libraries/vector-types/src/gradient.rs +++ b/node-graph/libraries/vector-types/src/gradient.rs @@ -274,6 +274,18 @@ impl GradientStops { index } + /// Insert a copy of the stop at `source_index` (same color and midpoint) at `position`, keeping the stops sorted by position. + /// Returns the index where the copy was inserted, or `None` if `source_index` is out of range. + pub fn duplicate_stop(&mut self, source_index: usize, position: f64) -> Option { + let color = *self.color.get(source_index)?; + let midpoint = *self.midpoint.get(source_index)?; + let index = self.position.iter().position(|p| *p > position).unwrap_or(self.position.len()); + self.position.insert(index, position); + self.midpoint.insert(index, midpoint); + self.color.insert(index, color); + Some(index) + } + /// Reset the midpoint for the interval starting at `index` to its default `0.5`. pub fn reset_midpoint(&mut self, index: usize) { if let Some(midpoint) = self.midpoint.get_mut(index) {