Skip to content

feat(main-editor): reorder layers within the current history step - #887

Merged
hm21 merged 2 commits into
stablefrom
fix/move-layer-skip-history
Oct 9, 2026
Merged

hm21 merged 2 commits into
stablefrom
fix/move-layer-skip-history

Conversation

@hm21

@hm21 hm21 commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Description

Problem. moveLayerListPosition always records a new history step. A video timeline that lets people drag a layer to another row already records its step when the drag starts, so it could not use the method and reordered activeLayers in place instead. Nothing redrew the layers after that: setLayerTimeline, called first for the drop, returns early when the drag leaves the time unchanged, so it sends no UI update either. The canvas kept the old stacking order until something unrelated rebuilt it. Playback did not help, because the play-time rebuild only compares the order of paint layers among themselves. In practice: add two stickers and a censor area, drag the censor row above them, and the preview keeps blurring the stickers while the export (which reads the list) does not.

Changes.

  • moveLayerListPosition takes skipUpdateHistory. With it, the layers are reordered in the current history step and redrawn through the layer stream. This mirrors replaceLayer's flag rather than adding a separate method, so a caller that already owns the history step has one obvious way to reorder.
  • With the flag, the current step's background screenshot is marked broken. It was taken when the step was recorded (e.g. at drag start), so doneEditing would otherwise return the image with the old layer order.
  • The bounds check is fixed on the way: newIndex is the position after the layer is removed, so newIndex == layers.length used to pass the check and then throw a RangeError on insert. It is now ignored like any other out-of-range index. The example's ReorderLayerSheet passes onReorderItem indices, which are already adjusted for the removal, so it never sends that value.

Tests. New move_layer_list_position_test.dart checks the drawn LayerWidget order, not just the list:

  • without the flag, the layers are redrawn and one history step is added;
  • with the flag, the layers are redrawn in the new order and no step is added (fails without the stream update);
  • addHistory followed by a skipped move undoes in one step;
  • with the flag, only the current step's screenshot is invalidated;
  • an index past the last layer is ignored.

Verification.

  • flutter test (925) and flutter analyze . pass on Flutter 3.47.2; dart format is clean.
  • Galaxy S26, in an app whose video timeline now calls moveLayerListPosition(..., skipUpdateHistory: true) on drop: a blur moved below two stickers, into the row between them, and back on top updates the preview immediately each time; sticker-over-sticker and pixelate moves do too; a drop that changes row and time at once places the layer correctly; undo and redo each take one step; the exported video matches the preview.

Version bumped to 14.11.0 with changelog entries.

Related Issue: Closes #none

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

moveLayerListPosition always recorded a new history step. A video
timeline that moves a layer to another row already records its step when
the drag starts, so it reordered activeLayers in place instead, and
nothing redrew the layers: setLayerTimeline returns early when the drag
leaves the time unchanged, so it sends no UI update either. The canvas
kept the old stacking order, e.g. a censor area still blurred a sticker
it had been moved below.

moveLayerListPosition now takes skipUpdateHistory. With it, the layers
are reordered in the current step and redrawn through the layer stream,
as replaceLayer does with the same flag.

The bounds check is fixed on the way: newIndex is the position after the
layer is removed, so newIndex == layers.length used to pass the check and
then throw a RangeError on insert. It is now ignored like any other
out-of-range index.
@hm21 hm21 self-assigned this Oct 9, 2026
…ut history

With skipUpdateHistory, moveLayerListPosition reordered the current
history step but left its background screenshot alone. That screenshot
was taken when the step was recorded, e.g. when a drag started, so
doneEditing returned the image with the layers in their old order.

Mark the active screenshot broken so the final capture renders the
layers again, as updateBackgroundImage does without history. Also
document that newIndex counts after the removal and shorten the
changelog entry.
@hm21
hm21 marked this pull request as ready for review October 9, 2026 12:00
@hm21
hm21 merged commit 84977fc into stable Oct 9, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant