Merge pull request #57 from Mike-Solar/main

app: migrate playhead/undo/modified events off EventBridge (issues 5-8)

.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

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<Core *>(userdata)->undo_index_changed(
+				static_cast<int>(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<QMainWindow *>(userdata);
-				mw->setWindowModified(event->a != 0);
+				emit static_cast<Core *>(userdata)->project_modified_changed(
+					event->a != 0);
 			},
-			main_window_);
+			this);
 	}
 }
 

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

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<ExportDialog *>(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();

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;

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<HistoryModel *>(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<HistoryWidget *>(userdata);
-			self->index_changed(static_cast<int>(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),

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:

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,16 +161,23 @@ 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

app/widget/timebased/timebasedwidget.h

@@ -169,6 +169,8 @@ protected:
 
 	EngineEventBridge *bridge_ = nullptr;
 
+	QMetaObject::Connection playhead_conn_;
+
 	virtual void ConnectNodeEvent(OakEngineNode *)
 	{
 	}

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,9 +751,16 @@ 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,
@@ -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,

app/widget/timelinewidget/timelinewidget.h

@@ -290,6 +290,7 @@ public slots:
 	void rename_selected_blocks();
 
 signals:
+
 	void block_selection_changed(const QVector<OakEngineBlock *> &selected_blocks);
 
 	void request_capture_start(const TimeRange &time,
@@ -358,6 +359,8 @@ private:
 
 	RationalSlider *timecode_label_;
 
+	QMetaObject::Connection timecode_playhead_conn_;
+
 	QVector<OakEngineBlock *> selected_blocks_;
 
 	QVector<OakEngineBlock *> added_blocks_;

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.

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：编辑/撤销/保存后标题栏修改标记正确。

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<OakEngineNode *>(viewer_b), reinterpret_cast<OakEngineNode *>(node), reinterpret_cast<OakEngineBlock *>(clip), Rational(5));
 	EXPECT_EQ(widget.get_connected_node(), reinterpret_cast<OakEngineNode *>(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<OakEngineNode *>(viewer_a), Rational(1));
 	EXPECT_EQ(widget.get_connected_node(), reinterpret_cast<OakEngineNode *>(viewer_b));
 }