diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6ceefe73..2693b52e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,7 @@ jobs: strategy: fail-fast: false matrix: - os: [warp-ubuntu-latest-x64-16x, warp-macos-15-arm64-12x, warp-windows-latest-x64-32x] + os: [warp-ubuntu-latest-x64-8x, warp-macos-15-arm64-6x, warp-windows-latest-x64-16x] env: CMAKE_BUILD_TYPE: Release CCACHE_DIR: ${{ github.workspace }}/.ccache diff --git a/app/core.cpp b/app/core.cpp index 363b28e42..764ae453c 100644 --- a/app/core.cpp +++ b/app/core.cpp @@ -102,6 +102,18 @@ Core::Core(const OakEngineAppParams *params) oakengine_app_create(&default_params); } + // Re-broadcast undo-stack index changes as the app-internal Qt signal + // undo_index_changed (issue 7 of the EventBridge elimination plan), so + // widgets no longer need raw oakengine_event_subscribe callbacks. Core is + // a process-lifetime singleton, so the subscription is never unsubscribed. + oakengine_event_subscribe( + oakengine_undo_handle(), OAKENGINE_EVENT_UNDO_INDEX_CHANGED, + [](const oakengine_event *event, void *userdata) { + emit static_cast(userdata)->undo_index_changed( + static_cast(event->a)); + }, + this); + // Register the UI handlers that the engine uses to request user interaction // through the C ABI callback struct instead of engine_core_->set_*_handler(). { @@ -772,6 +784,11 @@ void Core::start_gui(bool full_screen) // Create main window and open it main_window_ = new MainWindow(); + // The title-bar modified marker follows the app-internal signal + // (re-broadcast from the active project in on_active_project_changed). + connect(this, &Core::project_modified_changed, main_window_, + &QMainWindow::setWindowModified); + // Route engine notifications to the UI connect(this, &Core::tool_changed, this, [](const Tool::Item &) {}); // Status-bar and lifecycle notifications are handled through the facade @@ -1103,14 +1120,17 @@ void Core::on_active_project_changed(OakEngineProject *p) main_window_->set_project(p); if (p) { - // Keep the window's modified state in sync via event subscription - // (connection is removed automatically when the project is deleted). + // Re-broadcast the project's modified flag as the app-internal Qt + // signal project_modified_changed (issue 8 of the EventBridge + // elimination plan); the main window drives setWindowModified from + // it. The subscription is removed automatically when the project is + // deleted. oakengine_event_subscribe(p, OAKENGINE_EVENT_PROJECT_MODIFIED_CHANGED, [](const oakengine_event *event, void *userdata) { - QMainWindow *mw = static_cast(userdata); - mw->setWindowModified(event->a != 0); + emit static_cast(userdata)->project_modified_changed( + event->a != 0); }, - main_window_); + this); } } diff --git a/app/core.h b/app/core.h index 75b056568..81f3d208b 100644 --- a/app/core.h +++ b/app/core.h @@ -288,6 +288,23 @@ signals: void color_picker_enabled(bool e); void color_picker_color_emitted(const Color &reference, const Color &display); + /** + * @brief App-internal re-broadcast of the engine undo-stack index change + * (issue 7 of the EventBridge elimination plan). Widgets connect to this + * instead of raw oakengine_event_subscribe callbacks on + * OAKENGINE_EVENT_UNDO_INDEX_CHANGED. The argument is the new stack index. + */ + void undo_index_changed(int index); + + /** + * @brief App-internal re-broadcast of the active project's modified-flag + * change (issue 8 of the EventBridge elimination plan). The main window + * drives setWindowModified from this instead of a raw + * oakengine_event_subscribe callback on + * OAKENGINE_EVENT_PROJECT_MODIFIED_CHANGED. + */ + void project_modified_changed(bool modified); + private: /** * @brief Get the file filter than can be used with QFileDialog to open and save compatible projects diff --git a/app/dialog/export/export.cpp b/app/dialog/export/export.cpp index 4b3ab2322..69a254601 100644 --- a/app/dialog/export/export.cpp +++ b/app/dialog/export/export.cpp @@ -308,16 +308,17 @@ ExportDialog::ExportDialog(OakEngineNode *viewer_node, bool stills_only_mode, video_tab_ = new ExportVideoTab(color_manager_); add_preferences_tab(video_tab_, tr("Video")); - // Set video tab time and make connections - viewer_sub_ = oakengine_event_subscribe( - viewer_node, - OAKENGINE_EVENT_VIEWER_PLAYHEAD_CHANGED, - [](const oakengine_event *event, void *userdata) { - auto *dlg = static_cast(userdata); - auto *tab = dlg->video_tab_; - tab->set_time(Rational(event->a, event->b)); - }, - this); + // Set video tab time and make connections. The dialog follows the + // playhead through PlaybackController instead of a raw C event + // subscription (issue 5 of the EventBridge elimination plan); the + // connection auto-disconnects with `this`. + connect(PlaybackController::instance(), + &PlaybackController::playhead_changed, this, + [this](OakEngineNode *n, const Rational &time) { + if (n == viewer_node_) { + video_tab_->set_time(time); + } + }); connect(video_tab_, &ExportVideoTab::time_changed, this, [viewer_node](const Rational &time) { PlaybackController::instance()->set_playhead( @@ -463,16 +464,6 @@ ExportDialog::ExportDialog(OakEngineNode *viewer_node, bool stills_only_mode, subtitle_tab_->setEnabled(subtitles_enabled_->isChecked()); } -ExportDialog::~ExportDialog() -{ - // Raw C-API subscription carries `this` as userdata; not covered by - // Qt's auto-disconnect. - if (viewer_sub_ > 0) { - oakengine_event_unsubscribe(viewer_sub_); - viewer_sub_ = 0; - } -} - Rational ExportDialog::get_selected_timebase() const { return video_tab_->get_selected_frame_rate().flipped(); diff --git a/app/dialog/export/export.h b/app/dialog/export/export.h index 01c0743df..b228b0550 100644 --- a/app/dialog/export/export.h +++ b/app/dialog/export/export.h @@ -49,7 +49,6 @@ public: : ExportDialog(viewer_node, false, parent) { } - ~ExportDialog() override; Rational get_selected_timebase() const; void set_selected_timebase(const Rational &r); @@ -77,8 +76,6 @@ private: OakEngineNode *viewer_node_; - int64_t viewer_sub_ = 0; - int previously_selected_format_; Rational get_export_length() const; diff --git a/app/widget/history/historywidget.cpp b/app/widget/history/historywidget.cpp index ab45e661d..ec8ea4f20 100644 --- a/app/widget/history/historywidget.cpp +++ b/app/widget/history/historywidget.cpp @@ -21,7 +21,7 @@ #include "historywidget.h" -#include "oakengine/events.h" +#include "core.h" #include "oakengine/undo.h" namespace olive @@ -30,22 +30,13 @@ namespace olive HistoryModel::HistoryModel(QObject *parent) : QAbstractItemModel(parent) { - sub_ = oakengine_event_subscribe( - oakengine_undo_handle(), OAKENGINE_EVENT_UNDO_INDEX_CHANGED, - [](const oakengine_event *event, void *userdata) { - Q_UNUSED(event) - auto *self = static_cast(userdata); - self->beginResetModel(); - self->endResetModel(); - }, - this); -} - -HistoryModel::~HistoryModel() -{ - if (sub_ > 0) { - oakengine_event_unsubscribe(sub_); - } + // Refresh on Core's app-internal undo signal (issue 7 of the EventBridge + // elimination plan) instead of a raw C event subscription; the connection + // auto-disconnects with `this`. + connect(Core::instance(), &Core::undo_index_changed, this, [this](int) { + beginResetModel(); + endResetModel(); + }); } QModelIndex HistoryModel::index(int row, int column, @@ -124,26 +115,12 @@ HistoryWidget::HistoryWidget(QWidget *parent) this->setModel(model_); this->setRootIsDecorated(false); - undo_sub_ = oakengine_event_subscribe( - oakengine_undo_handle(), OAKENGINE_EVENT_UNDO_INDEX_CHANGED, - [](const oakengine_event *event, void *userdata) { - auto *self = static_cast(userdata); - self->index_changed(static_cast(event->a)); - }, - this); + connect(Core::instance(), &Core::undo_index_changed, this, + &HistoryWidget::index_changed); connect(this->selectionModel(), &QItemSelectionModel::currentRowChanged, this, &HistoryWidget::current_row_changed); } -HistoryWidget::~HistoryWidget() -{ - // Raw subscription carries `this` as userdata; cancel it or the engine - // calls back into a dead widget (the undo stack outlives us). - if (undo_sub_ > 0) { - oakengine_event_unsubscribe(undo_sub_); - } -} - void HistoryWidget::index_changed(int i) { this->selectionModel()->select(this->model()->index(i - 1, 0), diff --git a/app/widget/history/historywidget.h b/app/widget/history/historywidget.h index dde6e8b6c..b9358730b 100644 --- a/app/widget/history/historywidget.h +++ b/app/widget/history/historywidget.h @@ -36,13 +36,12 @@ namespace olive * Replaces the direct use of the engine's UndoStack as a Qt item model. * Semantics mirror engine/undo/undostack.cpp: two columns (Number, Action), * rows are all commands on the stack (done first, then undone), undone rows - * are shown gray. Refreshes itself on OAKENGINE_EVENT_UNDO_INDEX_CHANGED. + * are shown gray. Refreshes itself on Core::undo_index_changed. */ class HistoryModel : public QAbstractItemModel { Q_OBJECT public: explicit HistoryModel(QObject *parent = nullptr); - ~HistoryModel() override; QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const override; @@ -53,22 +52,16 @@ public: int role = Qt::DisplayRole) const override; QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; - -private: - int64_t sub_ = 0; }; class HistoryWidget : public QTreeView { Q_OBJECT public: HistoryWidget(QWidget *parent = nullptr); - ~HistoryWidget() override; private: HistoryModel *model_; - int64_t undo_sub_ = 0; - size_t current_row_; private slots: diff --git a/app/widget/timebased/timebasedwidget.cpp b/app/widget/timebased/timebasedwidget.cpp index 06a9c2f24..963a6a3bc 100644 --- a/app/widget/timebased/timebasedwidget.cpp +++ b/app/widget/timebased/timebasedwidget.cpp @@ -117,6 +117,7 @@ void TimeBasedWidget::connect_viewer_node(OakEngineNode *node) // Disconnect old bridge subscriptions and connections disconnect(bridge_, nullptr, this, nullptr); bridge_->unsubscribe_all(); + disconnect(playhead_conn_); if (viewer_node_) { oak_video_params vp; @@ -160,17 +161,24 @@ void TimeBasedWidget::connect_viewer_node(OakEngineNode *node) if (viewer_node_) { // Subscribe to viewer events via bridge bridge_->subscribe(node, OAKENGINE_EVENT_VIEWER_LENGTH_CHANGED); - bridge_->subscribe(node, OAKENGINE_EVENT_VIEWER_PLAYHEAD_CHANGED); bridge_->subscribe(node, OAKENGINE_EVENT_NODE_REMOVED_FROM_GRAPH); connect(bridge_, &EngineEventBridge::viewer_length_changed, this, [this](OakEngineNode *, qint64, qint64) { update_maximum_scroll(); }); - connect(bridge_, &EngineEventBridge::viewer_playhead_changed, this, - [this](OakEngineNode *, qint64 num, qint64 den) { - playhead_time_changed(Rational(num, den)); - }); + + // Playhead changes arrive via PlaybackController (issue 6 of the + // EventBridge elimination plan) instead of the bridge's + // viewer_playhead_changed. + playhead_conn_ = connect( + PlaybackController::instance(), + &PlaybackController::playhead_changed, this, + [this](OakEngineNode *n, const Rational &time) { + if (n == get_connected_node()) { + playhead_time_changed(time); + } + }); // Node removed from graph - use the bridge signal connect(bridge_, &EngineEventBridge::node_removed_from_graph, this, diff --git a/app/widget/timebased/timebasedwidget.h b/app/widget/timebased/timebasedwidget.h index 7061dd7a2..4d1edc3ac 100644 --- a/app/widget/timebased/timebasedwidget.h +++ b/app/widget/timebased/timebasedwidget.h @@ -169,6 +169,8 @@ protected: EngineEventBridge *bridge_ = nullptr; + QMetaObject::Connection playhead_conn_; + virtual void ConnectNodeEvent(OakEngineNode *) { } diff --git a/app/widget/timelinewidget/timelinewidget.cpp b/app/widget/timelinewidget/timelinewidget.cpp index ca980d20b..f8c468290 100644 --- a/app/widget/timelinewidget/timelinewidget.cpp +++ b/app/widget/timelinewidget/timelinewidget.cpp @@ -742,7 +742,6 @@ void TimelineWidget::ConnectNodeEvent(OakEngineNode *n) // Subscribe to viewer events via bridge bridge_->subscribe(handle, OAKENGINE_EVENT_VIEWER_FRAME_RATE_CHANGED); bridge_->subscribe(handle, OAKENGINE_EVENT_VIEWER_SAMPLE_RATE_CHANGED); - bridge_->subscribe(handle, OAKENGINE_EVENT_VIEWER_PLAYHEAD_CHANGED); connect(bridge_, &EngineEventBridge::viewer_frame_rate_changed, this, [this](OakEngineNode *, qint64, qint64) { @@ -752,10 +751,17 @@ void TimelineWidget::ConnectNodeEvent(OakEngineNode *n) [this](OakEngineNode *, int) { sample_rate_changed(); }); - connect(bridge_, &EngineEventBridge::viewer_playhead_changed, this, - [this](OakEngineNode *, qint64 num, qint64 den) { - this->timecode_label_->set_value(Rational(num, den)); - }); + + // Keep the timecode label in sync via PlaybackController (issue 6 of the + // EventBridge elimination plan) instead of the bridge's + // viewer_playhead_changed. + timecode_playhead_conn_ = connect( + PlaybackController::instance(), &PlaybackController::playhead_changed, + this, [this](OakEngineNode *n, const Rational &time) { + if (n == get_connected_node()) { + this->timecode_label_->set_value(time); + } + }); connect(timecode_label_, &RationalSlider::value_changed, this, [handle](const Rational &time) { @@ -806,6 +812,8 @@ void TimelineWidget::DisconnectNodeEvent(OakEngineNode *n) // Bridge subscriptions and connections are cleaned up by // TimeBasedWidget::connect_viewer_node (disconnect(bridge_, nullptr, this, nullptr)) + disconnect(timecode_playhead_conn_); + deselect_all(); foreach (OakEngineTrack *track, diff --git a/app/widget/timelinewidget/timelinewidget.h b/app/widget/timelinewidget/timelinewidget.h index ff8e37ebd..98921c778 100644 --- a/app/widget/timelinewidget/timelinewidget.h +++ b/app/widget/timelinewidget/timelinewidget.h @@ -290,6 +290,7 @@ public slots: void rename_selected_blocks(); signals: + void block_selection_changed(const QVector &selected_blocks); void request_capture_start(const TimeRange &time, @@ -358,6 +359,8 @@ private: RationalSlider *timecode_label_; + QMetaObject::Connection timecode_playhead_conn_; + QVector selected_blocks_; QVector added_blocks_; diff --git a/docs/plans/eliminate-event-bridge-issues.md b/docs/plans/eliminate-event-bridge-issues.md index 27c381669..eee0ba2f3 100644 --- a/docs/plans/eliminate-event-bridge-issues.md +++ b/docs/plans/eliminate-event-bridge-issues.md @@ -106,13 +106,13 @@ In other words: one app-side controller that every - Acceptance: the value tree refreshes with the playhead. - **Required: build + ctest all green.** -### issue 5 — ExportDialog playhead +### issue 5 ✅ (done) — ExportDialog playhead `app/dialog/export/export.cpp:311` (raw C). - Acceptance: the export dialog's in/out times stay in sync with the playhead. - **Required: build + ctest all green.** -### issue 6 — timelinewidget / viewerdisplay playhead family (bridge subscriptions) +### issue 6 ✅ (done) — timelinewidget / viewerdisplay playhead family (bridge subscriptions) `app/widget/timelinewidget/timelinewidget.cpp` (around :741-743) and the related viewerdisplay subscriptions. Reconnect to PlaybackController. - Acceptance: timeline timecode/playhead position display correctly. @@ -122,7 +122,7 @@ related viewerdisplay subscriptions. Reconnect to PlaybackController. ## Undo & modified migrations (pattern A, about half a day each) -### issue 7 — historywidget UNDO_INDEX_CHANGED +### issue 7 ✅ (done) — historywidget UNDO_INDEX_CHANGED `app/widget/history/historywidget.cpp:33,127` (two raw C callbacks). Emit an app-internal `undo_index_changed(int)` at Core's undo/redo/push exit points and reconnect historywidget to it. @@ -130,7 +130,7 @@ exit points and reconnect historywidget to it. the correct row. - **Required: build + ctest all green.** -### issue 8 — core.cpp PROJECT_MODIFIED_CHANGED +### issue 8 ✅ (done) — core.cpp PROJECT_MODIFIED_CHANGED `app/core.cpp:1108` (raw C). The modified flag is driven by the undo stack, so the app can derive it at push/undo/redo/load; drive `setWindowModified` from an app-internal signal instead. diff --git a/docs/zh/plans/eliminate-event-bridge-issues.md b/docs/zh/plans/eliminate-event-bridge-issues.md index 634ca21c6..2cc802a3d 100644 --- a/docs/zh/plans/eliminate-event-bridge-issues.md +++ b/docs/zh/plans/eliminate-event-bridge-issues.md @@ -104,12 +104,12 @@ PlaybackController::playhead_changed。 - 验收 / Acceptance：值树随播放头刷新。 - **必做 / Required：build + ctest 全绿。** -### issue 5 — ExportDialog playhead +### issue 5 ✅ (done) — ExportDialog playhead `app/dialog/export/export.cpp:311`（裸 C）。 - 验收 / Acceptance：导出对话框 in/out 时间随播放头同步。 - **必做 / Required：build + ctest 全绿。** -### issue 6 — timelinewidget / viewerdisplay 的 playhead 族（bridge 订阅） +### issue 6 ✅ (done) — timelinewidget / viewerdisplay 的 playhead 族（bridge 订阅） `app/widget/timelinewidget/timelinewidget.cpp`（:741-743 一带）与 viewerdisplay 的相关订阅。改连 PlaybackController。 - 验收 / Acceptance：时间线时间码/播放头位置显示正确。 @@ -119,14 +119,14 @@ PlaybackController::playhead_changed。 ## undo/modified 族迁移 / Undo & modified migrations（模式 A，约半天/个） -### issue 7 — historywidget 的 UNDO_INDEX_CHANGED +### issue 7 ✅ (done) — historywidget 的 UNDO_INDEX_CHANGED `app/widget/history/historywidget.cpp:33,127`（裸 C 两处）。 在 Core 的 undo/redo/push 出口 emit app 内 `undo_index_changed(int)`， historywidget 改连它。 - 验收 / Acceptance：undo/redo 后历史列表 model reset 且选中行正确。 - **必做 / Required：build + ctest 全绿。** -### issue 8 — core.cpp 的 PROJECT_MODIFIED_CHANGED +### issue 8 ✅ (done) — core.cpp 的 PROJECT_MODIFIED_CHANGED `app/core.cpp:1108`（裸 C）。modified 由 undo 栈驱动，app 在 push/undo/redo/load 处即可推导；改为 app 内信号驱动 `setWindowModified`。 - 验收 / Acceptance：编辑/撤销/保存后标题栏修改标记正确。 diff --git a/tests/gtest/widget_panels_model_test.cpp b/tests/gtest/widget_panels_model_test.cpp index d6b502dae..97379757f 100644 --- a/tests/gtest/widget_panels_model_test.cpp +++ b/tests/gtest/widget_panels_model_test.cpp @@ -24,6 +24,7 @@ #include "render/rendermanager.h" #include "oakengine/app.h" #include "oakengine/undo.h" +#include "playback/playbackcontroller.h" #include "task/task.h" #include "undo/undostack.h" #include "widget/colorbutton/colorbutton.h" @@ -744,7 +745,10 @@ TEST_F(MulticamWidgetTest, FutureSwitchWaitsForPlayheadToAdvance) widget.set_multicam_node(reinterpret_cast(viewer_b), reinterpret_cast(node), reinterpret_cast(clip), Rational(5)); EXPECT_EQ(widget.get_connected_node(), reinterpret_cast(viewer_a)); - // Once playback time advances, the queued switch takes effect - viewer_a->set_playhead(Rational(1)); + // Once playback time advances, the queued switch takes effect. The app + // drives the playhead through PlaybackController, which is what + // TimeBasedWidget now listens to (EventBridge elimination, issue 6). + PlaybackController::instance()->set_playhead( + reinterpret_cast(viewer_a), Rational(1)); EXPECT_EQ(widget.get_connected_node(), reinterpret_cast(viewer_b)); }