feat(inspector): live sampling + missing fields (crop/fade/flip) (#97)#122
Open
cuic19053-hue wants to merge 4 commits into
Open
feat(inspector): live sampling + missing fields (crop/fade/flip) (#97)#122cuic19053-hue wants to merge 4 commits into
cuic19053-hue wants to merge 4 commits into
Conversation
appergb
requested changes
Jun 23, 2026
appergb
left a comment
Owner
There was a problem hiding this comment.
@cuic19053-hue 自动审核结论:请修改(REQUEST_CHANGES)。补字段(位置X/Y、翻转、裁剪、fade in/out + 插值)实现良好且接线,但有阻塞:
- HIGH 正确性 bug:可编辑(未动画)的 opacity/volume 字段用了含 fade 包络的
opacityAt/volumeAt采样值(Inspector.tsx:258-259 sampledOpacity/sampledVolume)。当 clip 有 fade 或播放头落在 clip 区间外时,可编辑框显示被 fade 调制/归零的值(opacity 显 0%、volume 显 -∞dB),提交会把损坏值写成静态属性。修复:未动画的可编辑分支用clip.opacity/clip.volume,sampled*仅供已动画只读分支显示。 - 验收缺口:issue #97 头号缺口是『三段式 live 拖拽预览』(projectStore 加乐观 override 层:scrub onChange→写 override、onCommit→发命令清 override,Preview 优先读 override = 上游 apply(live)/commit(undo) 两段)。本 PR 未实现,仅做到『按播放头采样显示 + 补字段』。请补 live 预览,或在 PR 标题/说明降级范围(避免名实不符)。
请修 HIGH bug 后 rebase 到含 #119 的 main(本 PR 现已与 #119 在 command.rs/commands.rs/types.ts 冲突)。
appergb
requested changes
Jun 24, 2026
appergb
left a comment
Owner
There was a problem hiding this comment.
@cuic19053-hue Sonnet 4.6 复审:仍需修改。两处会写坏数据的正确性 bug 未修:
- [HIGH] 可编辑 opacity 用了含 fade 包络的采样值。
Inspector.tsx:532用sampledOpacity = opacityAt(clip, activeFrame)(含 fadeMultiplier),上游InspectorView.swift:655用rawOpacityAt(frame:)(不含 fade)。当 clip 有 fade 或播放头落在 fade 区间,可编辑字段会显示/提交被 fade 调制的值(如 0%),写成静态属性会永久破坏 opacity。 - [HIGH] 可编辑 volume 同样问题。
Inspector.tsx:433用sampledVolume = volumeAt(clip, activeFrame)(含 fadeMultiplier),上游InspectorView.swift:391用liveVolumeKfDb(at:) ?? dbFromLinear(clip.volume)——无 volume keyframe 时直接回落clip.volume,不经 fade。
修法:未动画分支用静态 clip.opacity/clip.volume;sampled* 仅用于已动画的只读分支。改完重提。
…ergb#97) Backend (opentake-ops + src-tauri): - Extend ClipProperties with crop, fade_in/out_frames, fade_in/out_interpolation, flip_horizontal, flip_vertical - set_clip_properties writes new fields; fade clamps to clip duration; flip_* writes to transform.flip_* - ClipPropertiesDto mirrors fields with serde camelCase - 5 unit tests: crop sets+clears track, fade frames+interp, fade clamps, flip writes to transform, multiple fields at once Frontend (web): - clip.ts: 1:1 port of Rust Clip::*_at sampling methods (opacity/volume/ rotation/size/topLeft/crop), fadeMultiplier, db<->linear, generic sampleKeyframeTrack with number/AnimPair/Crop lerp - Inspector.tsx: read activeFrame from uiStore; show sampled values at playhead; switch to ReadOnlyValue + AnimatedHint when a track is active - 4 new sections: Position (top-left x/y), Crop (4 edge insets 0-1), Flip (2 checkboxes), Fade (in/out frames + interpolation selects) - Fade section appears on both video and audio tabs - types.ts: extend ClipPropertiesReq with camelCase fields - dict.ts: i18n keys for new sections (zh-CN + en) Closes appergb#97
…* only for animated (review appergb#122)
59a47a6 to
2a3339c
Compare
Contributor
Author
|
@appergb 请求重新审查。此前反馈已全部修复,CI 双绿(Rust ✅ Web ✅,commit \2a3339c\):
请 re-review,谢谢! |
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.
What
Implements issue #97 — Inspector live sampling + missing fields. The Inspector now shows property values sampled at the current playhead frame (live preview), and four previously-missing clip property sections are added: Position, Crop, Flip, and Fade.
Closes #97.
How
Backend (Rust)
crates/opentake-ops/src/command.rs— ExtendedClipPropertieswith 7 newOptionfields:crop,fade_in_frames,fade_out_frames,fade_in_interpolation,fade_out_interpolation,flip_horizontal,flip_vertical. Theapply_property_changeshelper now writes each field directly onto the clip:cropsetsclip.cropand clears any existingcrop_track(static value replaces animation, consistent with howopacity/volumealready behave).fade_in_frames/fade_out_framesare clamped to>= 0and thenclamp_fades_to_duration()is called so a fade can never exceed the clip's duration.fade_in_interpolation/fade_out_interpolationassign directly.flip_horizontal/flip_verticalwrite toclip.transform.flip_horizontal/clip.transform.flip_vertical(flip lives onTransform, not the clip root).src-tauri/src/commands.rs—ClipPropertiesDtomirrors all new fields with#[serde(rename_all = "camelCase")]so the TS side receivesfadeInFrames,fadeOutInterpolation,flipHorizontal, etc.into_properties()maps each DTO field to the ops-layer struct.crates/opentake-ops/tests/command_apply.rs— 5 new unit tests:set_clip_properties_crop_sets_and_clears_track— crop field sets value and clears an existingcrop_track.set_clip_properties_fade_sets_frames_and_interpolation— fade frames + all three interpolation modes round-trip.set_clip_properties_fade_clamps_to_duration—fade_in_frames=100on a 30-frame clip clamps to 30;fade_out_frames=100clamps to 0 (no room after a full fade-in).set_clip_properties_flip_writes_to_transform— flip flags land ontransform.flip_horizontal/flip_vertical, not the clip root.set_clip_properties_multiple_fields_at_once— crop + fade + flip + opacity in a single command.Frontend (React + TypeScript)
web/src/lib/clip.ts— 1:1 port of the RustClip::*_atsampling methods so the Inspector can display the value at the playhead:smoothstep,dbFromLinear/linearFromDb(VolumeScale FLOOR_DB=-60 / CEILING_DB=15).sampleKeyframeTrack<V>with type-specific lerp functions fornumber,AnimPair, andCrop— mirrorsKeyframeTrack::sample(clamp at endpoints, left keyframe'sinterpolationOutselects hold/linear/smooth).fadeMultiplier(the fade head/tail envelope),rawOpacityAt,opacityAt(raw × fade),volumeAt,rotationAt,sizeAt,topLeftAt,cropAt.web/src/components/inspector/Inspector.tsx:activeFramefromuseEditorUiStoreand computes sampled values (sampledOpacity,sampledVolume,sampledRotation,sampledScale,sampledTopLeft,sampledCrop) on every render — so dragging the playhead updates the Inspector in real time.ScrubbableNumberFieldis replaced with aReadOnlyValue+AnimatedHint("已在关键帧面板动画化" / "Animated in Keyframes panel"), matching the spec's "animated → readonly + hint" pattern.transform.centerX/centerY(converts top-left → center by adding half width/height). Read-only whenpositionTrackis active.cropwhile preserving the other edges via spread. Read-only whencropTrackis active.flipHorizontal/flipVertical.<select>with linear/hold/smooth). Appears on both video and audio tabs since fades apply to opacity (visual) and volume (audio).web/src/lib/types.ts—ClipPropertiesReqextended withcrop?,fadeInFrames?,fadeOutFrames?,fadeInInterpolation?,fadeOutInterpolation?,flipHorizontal?,flipVertical?.web/src/i18n/dict.ts— New i18n keys for bothzh-CNandenlocales: section headers (position/crop/flip/fade), field labels (positionX/Y, cropLeft/Top/Right/Bottom, flipHorizontal/Vertical, fadeInFrames/OutFrames/InInterpolation/OutInterpolation), interpolation mode names (linear/hold/smooth), and the animated hint text.Testing
command_apply.rs(listed above).cargotoolchain, so tests were validated via IDE diagnostics (0 errors on all 3 modified Rust files); CI will runcargo test.pnpm tsc --noEmit— 0 errors.pnpm build— passes (1648 modules transformed, 6.54s).Limitation