From 245f204c819fe718f867a61006284570d778c802 Mon Sep 17 00:00:00 2001 From: Mike Solar Date: Mon, 20 Jul 2026 16:17:37 +0800 Subject: [PATCH] engine: timeline panel leftover commands migrate to the facade (round 3) - new primitives: clip_toggle_enabled (per-block flip), clip_set_linked, sequence_add_default_transition (config-driven, sequence timebase), node_set_label_many, node_set_color_label, plus observation getters - toggle-links, default transitions, enable toggles, color labels, and block renaming now go through the facade; the stray empty undo entry from block renaming is gone along the way - nest/multicam/waveform-sync stay as documented composites: they mix redo_now intermediate state, graph surgery, and app-side computation that a single primitive cannot express faithfully - the timeline panel's command execution paths are now fully migrated --- app/widget/timelinewidget/timelinewidget.cpp | 89 ++++++++++---- engine/include/oakengine/node.h | 27 +++++ engine/include/oakengine/timeline.h | 46 +++++++ engine/src/capi/node.cpp | 60 ++++++++++ engine/src/capi/timeline.cpp | 113 ++++++++++++++++++ engine/tests/oakengine_node_test.cpp | 38 ++++++ engine/tests/oakengine_timeline_edit_test.cpp | 78 ++++++++++++ 7 files changed, 427 insertions(+), 24 deletions(-) diff --git a/app/widget/timelinewidget/timelinewidget.cpp b/app/widget/timelinewidget/timelinewidget.cpp index 1cc1a98f5..23a3a0d5c 100644 --- a/app/widget/timelinewidget/timelinewidget.cpp +++ b/app/widget/timelinewidget/timelinewidget.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ #include "node/nodeundo.h" #include "node/project/footage/footage.h" #include "node/project/serializer/serializer.h" +#include "oakengine/node.h" #include "oakengine/timeline.h" #include "render/audiowaveformcache.h" #include "task/project/import/import.h" @@ -763,8 +765,15 @@ void TimelineWidget::toggle_links_on_selected() return; } - Core::instance()->undo_stack()->push(new NodeLinkManyCommand(blocks, link), - tr("Linked Clips")); + // Link/unlink through the liboakengine C ABI facade (one undoable + // command, same as the old NodeLinkManyCommand push). + QVector clips; + clips.reserve(blocks.size()); + foreach (Node *n, blocks) { + clips.append( + reinterpret_cast(static_cast(n))); + } + oakengine_clip_set_linked(clips.data(), clips.size(), link ? 1 : 0); } void TimelineWidget::add_default_transitions_to_selected() @@ -779,9 +788,16 @@ void TimelineWidget::add_default_transitions_to_selected() } if (!blocks.isEmpty()) { - Core::instance()->undo_stack()->push( - new TimelineAddDefaultTransitionCommand(blocks, timebase()), - tr("Added Default Transitions")); + // Through the liboakengine C ABI facade (one undoable command with + // the same engine semantics as the old app-side push). + QVector clips; + clips.reserve(blocks.size()); + foreach (ClipBlock *clip, blocks) { + clips.append(reinterpret_cast(clip)); + } + oakengine_sequence_add_default_transition( + reinterpret_cast(sequence()), clips.data(), + clips.size()); } } @@ -896,25 +912,28 @@ void TimelineWidget::toggle_selected_enabled() return; } - MultiUndoCommand *command = new MultiUndoCommand(); - + // Flip each selected clip through the liboakengine C ABI facade (one + // undoable command, same per-block inversion as the old children). + QVector clips; + clips.reserve(items.size()); foreach (Block *i, items) { - command->add_child(new BlockEnableDisableCommand(i, !i->is_enabled())); + if (ClipBlock *clip = dynamic_cast(i)) { + clips.append(reinterpret_cast(clip)); + } } - - Core::instance()->undo_stack()->push(command, tr("Toggled Clips Enabled")); + oakengine_clip_toggle_enabled(clips.data(), clips.size()); } void TimelineWidget::set_color_label(int index) { - MultiUndoCommand *command = new MultiUndoCommand(); - + // Batch color labels through the liboakengine C ABI facade (one + // undoable command, same as the old per-block children). + QVector nodes; + nodes.reserve(selected_blocks_.size()); foreach (Block *b, selected_blocks_) { - command->add_child(new NodeOverrideColorCommand(b, index)); + nodes.append(reinterpret_cast(b)); } - - Core::instance()->undo_stack()->push( - command, tr("Set Colors of %1 Clips").arg(selected_blocks_.size())); + oakengine_node_set_color_label(nodes.data(), nodes.size(), index); } void TimelineWidget::nudge_left() @@ -2068,16 +2087,38 @@ void TimelineWidget::reveal_in_project() void TimelineWidget::rename_selected_blocks() { - MultiUndoCommand *command = new MultiUndoCommand(); - QVector nodes(selected_blocks_.size()); - - for (int i = 0; i < nodes.size(); i++) { - nodes[i] = selected_blocks_[i]; + if (selected_blocks_.isEmpty()) { + return; } - Core::instance()->label_nodes(nodes); - Core::instance()->undo_stack()->push( - command, tr("Renamed %1 Clip(s)").arg(nodes.size())); + // Same rename dialog as Core::label_nodes(), but the write goes + // through the liboakengine C ABI facade (one undoable multi-node + // rename command; the old code also pushed a stray empty command, + // which is gone now). + QString start_label = selected_blocks_.first()->get_label(); + for (int i = 1; i < selected_blocks_.size(); i++) { + if (selected_blocks_.at(i)->get_label() != start_label) { + start_label.clear(); + break; + } + } + + bool ok; + const QString s = QInputDialog::getText(this, tr("Label Node"), + tr("Set node label"), + QLineEdit::Normal, start_label, + &ok); + if (!ok) { + return; + } + + QVector nodes; + nodes.reserve(selected_blocks_.size()); + foreach (Block *b, selected_blocks_) { + nodes.append(reinterpret_cast(b)); + } + oakengine_node_set_label_many(nodes.data(), nodes.size(), + s.toUtf8().constData()); } void TimelineWidget::track_about_to_be_deleted(Track *track) diff --git a/engine/include/oakengine/node.h b/engine/include/oakengine/node.h index bd6b5daeb..598509891 100644 --- a/engine/include/oakengine/node.h +++ b/engine/include/oakengine/node.h @@ -151,6 +151,33 @@ OAKENGINE_API int oakengine_node_set_label(OakEngineNode *self, OAKENGINE_API int oakengine_node_set_label_ex(OakEngineNode *self, const char *label, int undoable); +/** + * @brief Set one label on several nodes at once (undoable, ONE command; + * olive::NodeRenameCommand with all of them, like the application's + * Core::label_nodes() after its rename dialog). + * + * `nodes` holds borrowed handles (the handle is the engine node pointer + * in this family, so the application can pass its own nodes directly). + * Every entry must be non-NULL. `label` may be NULL for an empty label. + */ +OAKENGINE_API int oakengine_node_set_label_many(OakEngineNode **nodes, + int count, + const char *label); + +/** + * @brief Set the color-label index of several nodes at once (undoable, + * ONE command; olive::NodeOverrideColorCommand per node, like the + * timeline panel's color-label menu). `nodes` holds borrowed handles. + */ +OAKENGINE_API int oakengine_node_set_color_label(OakEngineNode **nodes, + int count, int color_index); + +/** + * @brief The node's color-label index (Node::get_override_color(); -1 = + * none). -1 on a NULL handle. + */ +OAKENGINE_API int oakengine_node_get_color_label(const OakEngineNode *self); + /* ---- Input introspection ---------------------------------------------------- */ /** diff --git a/engine/include/oakengine/timeline.h b/engine/include/oakengine/timeline.h index 79967cd42..559e8b7f4 100644 --- a/engine/include/oakengine/timeline.h +++ b/engine/include/oakengine/timeline.h @@ -494,6 +494,52 @@ OAKENGINE_API int oakengine_sequence_delete_clips( OAKENGINE_API int oakengine_sequence_ripple_delete_range( OakEngineSequence *seq, int64_t in_ts, int64_t out_ts); +/** + * @brief Flip the enabled flag of every given clip (undoable, ONE + * command; olive::BlockEnableDisableCommand per clip with the inverted + * current state -- the timeline panel's "toggle enabled"). Returns the + * number of toggled clips (>= 0) or a negative code. + */ +OAKENGINE_API int oakengine_clip_toggle_enabled(OakEngineClip **clips, + int count); + +/** + * @brief Link or unlink every given clip with each other (undoable, ONE + * command; olive::NodeLinkManyCommand -- the timeline panel's + * "link/unlink clips"). `linked` != 0 links, 0 unlinks. + */ +OAKENGINE_API int oakengine_clip_set_linked(OakEngineClip **clips, + int count, int linked); + +/** + * @brief Add the configured default transitions around the given clips + * (undoable, ONE command; + * olive::TimelineAddDefaultTransitionCommand -- the timeline panel's + * "add default transitions"). + * + * In/out transitions are created at clip boundaries that touch a gap or + * the sequence edge, dual transitions between adjacent selected clips; + * the transition node type and length come from the engine's config + * (DefaultVideoTransition/DefaultAudioTransition/ + * DefaultTransitionLength), exactly like the application. The sequence's + * frame-rate timebase is used for the length arithmetic. + */ +OAKENGINE_API int oakengine_sequence_add_default_transition( + OakEngineSequence *seq, OakEngineClip **clips, int count); + +/** + * @brief 1 if the clip is enabled (Block::is_enabled()). 0 on a NULL + * handle. + */ +OAKENGINE_API int oakengine_clip_is_enabled(const OakEngineClip *self); + +/** + * @brief 1 if the two clips are linked (Block::are_linked()). 0 on a + * NULL handle. + */ +OAKENGINE_API int oakengine_clip_are_linked(const OakEngineClip *a, + const OakEngineClip *b); + /** * @brief Delete the workarea range on every track (undoable, ONE command; * the application's TimelineWidget::delete_in_to_out). diff --git a/engine/src/capi/node.cpp b/engine/src/capi/node.cpp index dfc791dd1..4fabd8503 100644 --- a/engine/src/capi/node.cpp +++ b/engine/src/capi/node.cpp @@ -549,6 +549,66 @@ int oakengine_node_set_label_ex(OakEngineNode *self, const char *label, return OAKENGINE_OK; } +int oakengine_node_set_label_many(OakEngineNode **nodes, int count, + const char *label) +{ + set_error(QString()); + if (count < 0 || (count > 0 && !nodes)) { + set_error(QStringLiteral("invalid arguments")); + return OAKENGINE_E_INVALID; + } + if (count == 0) { + return OAKENGINE_OK; + } + // One multi-node rename command, like Core::label_nodes(). + auto *command = new olive::NodeRenameCommand(); + const QString text = QString::fromUtf8(label ? label : ""); + for (int i = 0; i < count; i++) { + if (!nodes[i]) { + set_error(QStringLiteral("invalid node at index %1").arg(i)); + delete command; + return OAKENGINE_E_INVALID; + } + command->add_node(impl(nodes[i]), text); + } + push_or_run(command, QStringLiteral("Rename Nodes")); + return OAKENGINE_OK; +} + +int oakengine_node_set_color_label(OakEngineNode **nodes, int count, + int color_index) +{ + set_error(QString()); + if (count < 0 || (count > 0 && !nodes)) { + set_error(QStringLiteral("invalid arguments")); + return OAKENGINE_E_INVALID; + } + if (count == 0) { + return OAKENGINE_OK; + } + olive::MultiUndoCommand *command = new olive::MultiUndoCommand(); + for (int i = 0; i < count; i++) { + if (!nodes[i]) { + set_error(QStringLiteral("invalid node at index %1").arg(i)); + delete command; + return OAKENGINE_E_INVALID; + } + command->add_child( + new olive::NodeOverrideColorCommand(impl(nodes[i]), + color_index)); + } + push_or_run(command, QStringLiteral("Set Node Color Labels")); + return OAKENGINE_OK; +} + +int oakengine_node_get_color_label(const OakEngineNode *self) +{ + if (!self) { + return -1; + } + return impl(self)->get_override_color(); +} + int oakengine_node_input_count(const OakEngineNode *self) { return self ? impl(self)->inputs().size() : 0; diff --git a/engine/src/capi/timeline.cpp b/engine/src/capi/timeline.cpp index 245ac719c..2625612b2 100644 --- a/engine/src/capi/timeline.cpp +++ b/engine/src/capi/timeline.cpp @@ -1264,6 +1264,119 @@ int oakengine_sequence_ripple_delete_range(OakEngineSequence *seq, return OAKENGINE_OK; } +int oakengine_clip_toggle_enabled(OakEngineClip **clips, int count) +{ + set_seq_error(QString()); + if (count < 0 || (count > 0 && !clips)) { + set_seq_error(QStringLiteral("invalid arguments")); + return OAKENGINE_E_INVALID; + } + olive::MultiUndoCommand *command = new olive::MultiUndoCommand(); + for (int i = 0; i < count; i++) { + olive::ClipBlock *clip = + reinterpret_cast(clips[i]); + if (!clip) { + set_seq_error(QStringLiteral("invalid clip at index %1").arg(i)); + delete command; + return OAKENGINE_E_INVALID; + } + // The panel's toggle: each clip flips its own current state. + command->add_child( + new olive::BlockEnableDisableCommand(clip, !clip->is_enabled())); + } + if (count > 0) { + push_or_run(command, QStringLiteral("Toggle Clips Enabled")); + } else { + delete command; + } + return count; +} + +int oakengine_clip_set_linked(OakEngineClip **clips, int count, int linked) +{ + set_seq_error(QString()); + if (count < 0 || (count > 0 && !clips)) { + set_seq_error(QStringLiteral("invalid arguments")); + return OAKENGINE_E_INVALID; + } + if (count == 0) { + return OAKENGINE_OK; + } + QVector nodes; + nodes.reserve(count); + for (int i = 0; i < count; i++) { + olive::ClipBlock *clip = + reinterpret_cast(clips[i]); + if (!clip) { + set_seq_error(QStringLiteral("invalid clip at index %1").arg(i)); + return OAKENGINE_E_INVALID; + } + nodes.append(clip); + } + push_or_run(new olive::NodeLinkManyCommand(nodes, linked != 0), + QStringLiteral("Link Clips")); + return OAKENGINE_OK; +} + +int oakengine_sequence_add_default_transition(OakEngineSequence *seq, + OakEngineClip **clips, int count) +{ + set_seq_error(QString()); + olive::Sequence *sequence = reinterpret_cast(seq); + if (!sequence || count < 0 || (count > 0 && !clips)) { + set_seq_error(QStringLiteral("invalid arguments")); + return OAKENGINE_E_INVALID; + } + if (count == 0) { + return OAKENGINE_OK; + } + olive::Rational tb; + if (!time_base_of(sequence, &tb)) { + set_seq_error(QStringLiteral("sequence has no valid frame rate")); + return OAKENGINE_E_STATE; + } + QVector blocks; + blocks.reserve(count); + for (int i = 0; i < count; i++) { + olive::ClipBlock *clip = + reinterpret_cast(clips[i]); + if (!clip) { + set_seq_error(QStringLiteral("invalid clip at index %1").arg(i)); + return OAKENGINE_E_INVALID; + } + blocks.append(clip); + } + push_or_run(new olive::TimelineAddDefaultTransitionCommand(blocks, tb), + QStringLiteral("Add Default Transitions")); + return OAKENGINE_OK; +} + +int oakengine_clip_is_enabled(const OakEngineClip *self) +{ + if (!self) { + return 0; + } + return reinterpret_cast(self)->is_enabled() ? + 1 : + 0; +} + +int oakengine_clip_are_linked(const OakEngineClip *a, const OakEngineClip *b) +{ + if (!a || !b) { + return 0; + } + // Block::are_linked() takes non-const pointers (it is read-only in + // practice). + return olive::Block::are_linked( + const_cast( + reinterpret_cast(a)), + const_cast( + reinterpret_cast(b))) ? + 1 : + 0; +} + int oakengine_sequence_ripple_delete_in_to_out(OakEngineSequence *seq, int ripple, int64_t in_ts, int64_t out_ts) diff --git a/engine/tests/oakengine_node_test.cpp b/engine/tests/oakengine_node_test.cpp index e67d2aa3e..ce1ae83e6 100644 --- a/engine/tests/oakengine_node_test.cpp +++ b/engine/tests/oakengine_node_test.cpp @@ -332,6 +332,43 @@ static void test_remove(OakEngineProject *project, OakEngineNode *solid, (void)solid; } +static void test_label_and_color_many(OakEngineProject *project) +{ + OakEngineNode *a = oakengine_project_add_node( + project, "org.olivevideoeditor.Olive.solidgenerator"); + OakEngineNode *b = oakengine_project_add_node( + project, "org.olivevideoeditor.Olive.text3"); + assert(a != NULL && b != NULL); + OakEngineNode *two[2] = { a, b }; + char buf[128]; + + // One command renames both; undo restores each node's own label. + assert(oakengine_node_set_label_many(two, 2, "Shared") == OAKENGINE_OK); + assert(oakengine_node_get_label(a, buf, sizeof(buf)) > 0); + assert(strcmp(buf, "Shared") == 0); + assert(oakengine_node_get_label(b, buf, sizeof(buf)) > 0); + assert(strcmp(buf, "Shared") == 0); + assert(oakengine_project_undo(project) == OAKENGINE_OK); + // The original labels were empty. + assert(oakengine_node_get_label(a, buf, sizeof(buf)) == 0); + assert(oakengine_node_get_label(b, buf, sizeof(buf)) == 0); + assert(oakengine_node_set_label_many(NULL, 1, "x") == + OAKENGINE_E_INVALID); + assert(oakengine_node_set_label_many(two, 0, "x") == OAKENGINE_OK); + + // Color labels batch in one command too. + assert(oakengine_node_get_color_label(a) == -1); + assert(oakengine_node_set_color_label(two, 2, 5) == OAKENGINE_OK); + assert(oakengine_node_get_color_label(a) == 5); + assert(oakengine_node_get_color_label(b) == 5); + assert(oakengine_project_undo(project) == OAKENGINE_OK); + assert(oakengine_node_get_color_label(a) == -1); + assert(oakengine_node_get_color_label(b) == -1); + assert(oakengine_node_get_color_label(NULL) == -1); + assert(oakengine_node_set_color_label(NULL, 1, 1) == + OAKENGINE_E_INVALID); +} + int main(void) { make_tmpdir(); @@ -356,6 +393,7 @@ int main(void) test_inputs_and_params(project, solid, lut); test_edges(project, solid, lut); test_remove(project, solid, lut); + test_label_and_color_many(project); // Graph nodes are not timeline clips: a sequence's track list stays // empty no matter what the project graph holds. diff --git a/engine/tests/oakengine_timeline_edit_test.cpp b/engine/tests/oakengine_timeline_edit_test.cpp index 6277a1d9e..018f06d55 100644 --- a/engine/tests/oakengine_timeline_edit_test.cpp +++ b/engine/tests/oakengine_timeline_edit_test.cpp @@ -1128,6 +1128,83 @@ static void test_batch_editing_round2(const char *media_path) oakengine_project_free(project); } +static void test_batch_editing_round3(const char *media_path) +{ + OakEngineProject *project = oakengine_project_create(); + assert(project != NULL); + assert(oakengine_project_new(project) == OAKENGINE_OK); + OakEngineSequence *seq = oakengine_sequence_new(project, "Batch3"); + assert(seq != NULL); + assert(oakengine_sequence_add_track(seq, OAKENGINE_TRACK_TYPE_VIDEO) == + 0); + OakEngineFootage *footage = + oakengine_project_import_footage(project, media_path); + assert(footage != NULL); + + int64_t in = -1, out = -1; + + // Two adjacent clips: [0, 60) and [60, 100). + OakEngineClip *c0 = oakengine_sequence_add_footage_clip( + seq, footage, OAKENGINE_TRACK_TYPE_VIDEO, 0, 0, 60, 0); + OakEngineClip *c1 = oakengine_sequence_add_footage_clip( + seq, footage, OAKENGINE_TRACK_TYPE_VIDEO, 0, 60, 100, 0); + assert(c0 != NULL && c1 != NULL); + OakEngineClip *two[2] = { c0, c1 }; + + // toggle_enabled flips each clip in one undoable command. + assert(oakengine_clip_is_enabled(c0) == 1 && + oakengine_clip_is_enabled(c1) == 1); + assert(oakengine_clip_toggle_enabled(two, 2) == 2); + assert(oakengine_clip_is_enabled(c0) == 0 && + oakengine_clip_is_enabled(c1) == 0); + assert(oakengine_project_undo(project) == OAKENGINE_OK); + assert(oakengine_clip_is_enabled(c0) == 1 && + oakengine_clip_is_enabled(c1) == 1); + assert(oakengine_clip_toggle_enabled(NULL, 1) == OAKENGINE_E_INVALID); + assert(oakengine_clip_toggle_enabled(two, 0) == 0); + assert(oakengine_clip_is_enabled(NULL) == 0); + + // set_linked: link and unlink are one undoable command each. + assert(oakengine_clip_are_linked(c0, c1) == 0); + assert(oakengine_clip_set_linked(two, 2, 1) == OAKENGINE_OK); + assert(oakengine_clip_are_linked(c0, c1) == 1); + assert(oakengine_clip_set_linked(two, 2, 0) == OAKENGINE_OK); + assert(oakengine_clip_are_linked(c0, c1) == 0); + assert(oakengine_project_undo(project) == OAKENGINE_OK); + assert(oakengine_clip_are_linked(c0, c1) == 1); + assert(oakengine_project_undo(project) == OAKENGINE_OK); + assert(oakengine_clip_are_linked(c0, c1) == 0); + assert(oakengine_clip_are_linked(c0, NULL) == 0); + assert(oakengine_clip_set_linked(NULL, 1, 1) == OAKENGINE_E_INVALID); + assert(oakengine_clip_set_linked(two, 0, 1) == OAKENGINE_OK); + + // add_default_transition: an in-transition before c0, a dual between + // the adjacent pair and an out-transition after c1 shrink both clips; + // one undo restores the exact ranges. + assert(oakengine_clip_get_range(c0, &in, &out, NULL) == OAKENGINE_OK); + const int64_t c0_in = in, c0_len = out - in; + assert(oakengine_clip_get_range(c1, &in, &out, NULL) == OAKENGINE_OK); + const int64_t c1_len = out - in; + assert(oakengine_sequence_add_default_transition(seq, two, 2) == + OAKENGINE_OK); + assert(oakengine_clip_get_range(c0, &in, &out, NULL) == OAKENGINE_OK); + assert(in > c0_in && out - in < c0_len); + assert(oakengine_clip_get_range(c1, &in, &out, NULL) == OAKENGINE_OK); + assert(out - in < c1_len); + assert(oakengine_project_undo(project) == OAKENGINE_OK); + assert(oakengine_clip_get_range(c0, &in, &out, NULL) == OAKENGINE_OK); + assert(in == c0_in && out - in == c0_len); + assert(oakengine_clip_get_range(c1, &in, &out, NULL) == OAKENGINE_OK); + assert(out - in == c1_len); + assert(oakengine_sequence_add_default_transition(seq, NULL, 0) == + OAKENGINE_OK); + assert(oakengine_sequence_add_default_transition(NULL, two, 2) == + OAKENGINE_E_INVALID); + + oakengine_footage_free(footage); + oakengine_project_free(project); +} + int main(void) { make_tmpdir(); @@ -1159,6 +1236,7 @@ int main(void) test_sequence_params(); test_batch_editing(path); test_batch_editing_round2(path); + test_batch_editing_round3(path); oakengine_project_free(project); assert(oakengine_shutdown() == OAKENGINE_OK);