Skip to content

Replace selected layer sources with geometry and Audio preservation - #1980

Open
yuraizmailov wants to merge 2 commits into
storytold:mainfrom
yuraizmailov:pr/replace-source
Open

yuraizmailov wants to merge 2 commits into
storytold:mainfrom
yuraizmailov:pr/replace-source

Conversation

@yuraizmailov

@yuraizmailov yuraizmailov commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Add layer.replaceSource, the default Command/Ctrl+Alt+/ shortcut, selected-layer Option-drop and
AVLayer.replaceSource(newSource, false). Remap source-relative mask/Anchor Point values, keys and
tangents to the new pixel dimensions. Keep Position, Scale, other properties and key times.

Use the standard source-capability rule and Audio builder to add missing controls while preserving
existing IDs, keys, expressions and mute through silent replacements. Validate all targets and
composition cycles before an atomic edit. Option-drop recipients are selected unlocked AV layers,
regardless of the hovered row; empty selection does nothing. fixExpressions=true is explicitly
unsupported.

Selectively adapts ki2pixel's #1929 at 8d887fd: scripting adapter,
named-item resolution and portions of eligibility/whole-layer drop feedback. Geometry remapping,
Audio preservation, checked ID allocation and selection-only batch semantics are retained here.
This overlaps the open #1929; the implementations should be reconciled during review.

Dependency: includes the small system-font coverage prerequisite (#1973) so the full
local UI suite passes with the Simplified Chinese catalog. It does not include the other feature PRs.

Validation: CARGO_PROFILE_RELEASE_LTO=off cargo xtask ci (format, release Clippy with warnings denied, complete release
workspace tests, dependency/layer/asset checks and WASM compilation). Native interaction validation
was performed on macOS; Windows/Linux native interaction has not been checked locally.

No dependency or project schema changes. No third-party media assets are added.

LTO is disabled only for this local validation run to reduce test-executable link time.
No tests or gate phases are excluded, and repository release settings are unchanged.

Local gate result: 3,683 passed, zero failed, 31 ignored across 199 test suites; all gate phases completed.

Summary by CodeRabbit

  • New Features

    • Replace selected layer sources by Alt/Option-dragging Project items or using the Command/Control+Alt+/ shortcut.
    • Added scripting support through AVLayer.replaceSource.
    • Preserves layer properties, audio settings, and undo/redo behavior while remapping dimensional geometry.
    • Validates unsupported sources, locked layers, cycles, and invalid selections.
  • Documentation

    • Added guidance for source replacement workflows, validation, scripting, and undo behavior.
  • Improvements

    • Improved font fallback coverage for translated CJK interfaces.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: storytold/effectcraft/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 273bf04e-7e1b-4c41-aec5-edcd94c55f7f
📥 Commits

Reviewing files that changed from the base of the PR and between 9db7f3b and 0e1099e.

📒 Files selected for processing: 16
  • crates/engine/src/commands/layer.rs
  • crates/engine/src/commands/layer_source.rs
  • crates/engine/src/commands/mod.rs
  • crates/engine/src/lib.rs
  • crates/engine/src/tests_layer_source.rs
  • crates/engine/src/tests_menu_cmds.rs
  • crates/project/src/build.rs
  • crates/script/src/prelude.js
  • crates/script/src/tests.rs
  • crates/ui-egui/src/i18n.rs
  • crates/ui-egui/src/i18n/ui.rs
  • crates/ui-egui/src/panels/timeline.rs
  • crates/ui-egui/src/theme.rs
  • crates/ui-egui/tests/ui_drag_drop.rs
  • docs/control-protocol.md
  • docs/footage.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Adds an engine command to replace selected layer sources, with validation, geometry remapping, and audio-control handling. Scripting and Timeline entry points invoke the command. Chinese font fallback setup also uses characters from the active translation catalog.

Changes: Layer Source Replacement

Layer / File(s) Summary
Engine validation and replacement
crates/engine/src/commands/*, crates/project/src/build.rs, crates/engine/src/tests_layer_source.rs, crates/engine/src/tests_menu_cmds.rs
The engine registers layer.replaceSource, validates replacement items and target layers, remaps anchor and mask geometry, and handles audio controls. Tests cover replacement behavior, validation, undo/redo, and rendered audio.
Scripting and Timeline entry points
crates/script/src/prelude.js, crates/script/src/tests.rs, crates/ui-egui/src/panels/timeline.rs, crates/ui-egui/tests/ui_drag_drop.rs, docs/control-protocol.md, docs/footage.md
AVLayer.replaceSource and Timeline Alt-drag dispatch source replacement. Tests cover scripting and drag-and-drop behavior. Documentation describes command inputs, validation, and replacement behavior.

Catalog-Based Font Fallback

Layer / File(s) Summary
Catalog character probes
crates/ui-egui/src/i18n.rs, crates/ui-egui/src/i18n/ui.rs, crates/ui-egui/src/theme.rs
Non-WASM code collects characters from the active translation catalog. Chinese font fallback setup uses eligible catalog characters as additional probes and assigns catalog-specific font names.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Timeline
  participant Eligibility as can_replace_source
  participant Command as layer.replaceSource
  participant Replacement as replace_source
  Timeline->>Eligibility: Check selected layers and dragged item
  Eligibility-->>Timeline: Return replacement availability
  Timeline->>Command: Dispatch selected layers and item
  Command->>Replacement: Replace layer sources
Loading

Suggested reviewers: echelon

Merge Risk: ⚪ Minimal · up to 0e109

Layer-source replacement is now available from the Timeline, keyboard shortcut, and scripting. It validates all targets before editing, remaps geometry, and preserves audio controls. Chinese font fallback also probes characters from the active translation catalog. No outstanding defect was found, and the change appears ready to merge.

Security architecture risk: 🔵 Low · up to 0e109

Source replacement uses the existing editing and undo mechanisms, validates targets and composition cycles, and preserves layer-owned audio controls. No material security weakness was established in the inspected paths. Broader access controls and platform behavior were not fully assessed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The direct mutation scope is a target set of unlocked AV layers within a requested composition in the current session project. The engine façade is shared by UI, CLI, control-channel, and MCP consumers, so the new capability is not inherently UI-only. The test-file public-entrypoint and high-fanout signals identify synthetic fixtures and test-local render sources rather than additional production authority.

Trust Boundaries and Controls

  • observed — Caller-supplied composition, layer, and item references are resolved by the engine rather than trusted solely because they came from a scripting wrapper or drag payload. Explicit layers must belong to the resolved composition, locked targets are filtered, and replacement accepts existing AV-capable project items. The inspected replacement path dispatches through the engine executor rather than scripting filesystem or socket natives.

Resilience and Maintainability Implications

  • observed — Failure containment uses commit-after-success editing: geometry remapping and checked Audio-ID allocation modify a cloned project, so an error does not publish a partially replaced batch or append its history entry. Tests assert unchanged project state after allocation exhaustion and mixed valid/invalid target rejection. The synchronous mutable-session API serializes this operation rather than exposing an asynchronous intermediate state.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 64.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 14 files. (2 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main change: replacing selected layer sources while preserving geometry and Audio properties.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 64.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 14 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.1)

Clippy execution timed out

Comment @coderabbitai help to get the list of available commands.

ki2pixel referenced this pull request Oct 27, 2026
)

Alt/Option-drag a Project item onto a Timeline layer, or press Ctrl+Alt+/ (Cmd+Opt+/) with layers and an item selected, to swap the layer's source. The layer keeps its name, timing, transforms, keyframes, expressions, effects, masks, switches, parenting and track matte; a source with audio adds Audio properties when missing. Precompositions can't be nested inside themselves.

AVLayer.replaceSource now runs the same engine command and closes the replaceSource part of #813.
@bflatastic

Copy link
Copy Markdown
Member

Hi @yuraizmailov, thanks for this PR! main has changed a lot since it was opened, and it now conflicts with main, so it needs a rebase before we can review it for merging.

Most of the conflict comes from the shared module layout (#2491), which moved or split files this PR touches:

  • crates/engine/src/commands/layer.rs: its folder moved to crates/engine/src/cmd/
  • crates/engine/src/commands/mod.rs: its folder moved to crates/engine/src/cmd/
  • crates/engine/src/lib.rs was split: most of its code is now in session/*.rs and siblings
  • crates/engine/src/tests_menu_cmds.rs → crates/engine/src/tests/menu_cmds.rs
  • crates/ui-egui/tests/ui_drag_drop.rs → crates/ui-egui/tests/it/ui_drag_drop.rs

When you rebase, please also make sure it fits the other recent changes:

CI (cargo xtask ci) now runs on PRs. Once it's green on the rebased branch, we'll take a look. Thank you!

This branch has not been deployed

No deployments
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.

2 participants