Repository navigation
feat(main-editor): reorder layers within the current history step - #887
Merged
Merged
Conversation
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.
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem.
moveLayerListPositionalways 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 reorderedactiveLayersin 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.
moveLayerListPositiontakesskipUpdateHistory. With it, the layers are reordered in the current history step and redrawn through the layer stream. This mirrorsreplaceLayer's flag rather than adding a separate method, so a caller that already owns the history step has one obvious way to reorder.doneEditingwould otherwise return the image with the old layer order.newIndexis the position after the layer is removed, sonewIndex == layers.lengthused to pass the check and then throw aRangeErroron insert. It is now ignored like any other out-of-range index. The example'sReorderLayerSheetpassesonReorderItemindices, which are already adjusted for the removal, so it never sends that value.Tests. New
move_layer_list_position_test.dartchecks the drawnLayerWidgetorder, not just the list:addHistoryfollowed by a skipped move undoes in one step;Verification.
flutter test(925) andflutter analyze .pass on Flutter 3.47.2;dart formatis clean.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