Defer OCIO LUT processor generation in render worker and fix stale graph snapshots
- Override OCIOLutNode::Value() to ensure the processor is created/updated before the color transform job is emitted. - In the worker process, mark the processor dirty on input/config changes instead of creating it synchronously during LoadGraph, which blocked the main process waiting for the graph load acknowledgement. - Keep eager processor generation in the main GUI process so the viewer cache is invalidated immediately when the LUT file or direction changes. - Mark the ProjectCopier's internal render-proxy project as modified when its update queue is processed, and reset the flag after RenderWorkerPool writes a new graph snapshot. This fixes the worker loading a stale graph snapshot after switching cube files or direction, which caused the old LUT effect to persist. All existing tests pass.
This commit is contained in:
@@ -45,6 +45,11 @@ bool IsSupportedLutExtension(const QString &suffix)
|
||||
return lower == QStringLiteral("cube") || lower == QStringLiteral("3dl");
|
||||
}
|
||||
|
||||
bool IsMainProcess()
|
||||
{
|
||||
return qobject_cast<QApplication *>(QCoreApplication::instance()) != nullptr;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
OCIOLutNode::OCIOLutNode()
|
||||
@@ -98,72 +103,121 @@ void OCIOLutNode::InputValueChangedEvent(const QString &input, int element)
|
||||
Q_UNUSED(element)
|
||||
|
||||
if (input == kFileInput || input == kDirectionInput) {
|
||||
GenerateProcessor();
|
||||
// In the worker process, creating the OCIO processor can be slow and we
|
||||
// are often called from LoadGraph while the main process is blocked
|
||||
// waiting for a response. Defer generation to Value() time so the worker
|
||||
// can ack the graph load immediately.
|
||||
if (IsMainProcess()) {
|
||||
GenerateProcessor();
|
||||
} else {
|
||||
QMutexLocker locker(&gen_mutex_);
|
||||
processor_dirty_ = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void OCIOLutNode::ConfigChanged()
|
||||
{
|
||||
GenerateProcessor();
|
||||
if (IsMainProcess()) {
|
||||
GenerateProcessor();
|
||||
} else {
|
||||
QMutexLocker locker(&gen_mutex_);
|
||||
processor_dirty_ = true;
|
||||
}
|
||||
}
|
||||
|
||||
void OCIOLutNode::Value(const NodeValueRow &value, const NodeGlobals &globals,
|
||||
NodeValueTable *table) const
|
||||
{
|
||||
// Ensure the processor is up-to-date before the base class emits the color
|
||||
// transform job. This is especially important in the render worker, where
|
||||
// processor creation is deferred until the first render.
|
||||
EnsureProcessor();
|
||||
|
||||
super::Value(value, globals, table);
|
||||
}
|
||||
|
||||
void OCIOLutNode::GenerateProcessor()
|
||||
{
|
||||
EnsureProcessor();
|
||||
|
||||
// The processor has changed. In the main GUI process, refresh the viewer by
|
||||
// invalidating the cache and cancelling background cache jobs.
|
||||
// Invalidating first ensures any in-flight renders that complete afterwards
|
||||
// won't write stale frames back. The worker process uses QGuiApplication and
|
||||
// has no RenderManager/PreviewAutoCacher, so skip this step to avoid crashing.
|
||||
if (IsMainProcess()) {
|
||||
InvalidateAll(kTextureInput);
|
||||
if (RenderManager *rm = RenderManager::instance()) {
|
||||
if (PreviewAutoCacher *cacher = rm->GetCacher()) {
|
||||
cacher->CancelVideoTasks(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void OCIOLutNode::EnsureProcessor() const
|
||||
{
|
||||
QMutexLocker locker(&gen_mutex_);
|
||||
|
||||
if (!processor_dirty_ && last_processor_) {
|
||||
return;
|
||||
}
|
||||
|
||||
CreateProcessorFromInputs();
|
||||
}
|
||||
|
||||
bool OCIOLutNode::CreateProcessorFromInputs() const
|
||||
{
|
||||
if (!manager()) {
|
||||
set_processor(nullptr);
|
||||
const_cast<OCIOLutNode *>(this)->set_processor(nullptr);
|
||||
last_processor_.reset();
|
||||
last_path_.clear();
|
||||
last_direction_ = -1;
|
||||
return;
|
||||
processor_dirty_ = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
const QString path = GetStandardValue(kFileInput).toString();
|
||||
const int direction = GetStandardValue(kDirectionInput).toInt();
|
||||
|
||||
if (path.isEmpty()) {
|
||||
set_processor(nullptr);
|
||||
const_cast<OCIOLutNode *>(this)->set_processor(nullptr);
|
||||
last_processor_.reset();
|
||||
last_path_.clear();
|
||||
last_direction_ = -1;
|
||||
return;
|
||||
processor_dirty_ = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Re-use the existing processor if the file and direction haven't changed.
|
||||
// NOTE: This is intentionally disabled while debugging LUT switching issues
|
||||
// to ensure the processor is always recreated when the input changes.
|
||||
// if (path == last_path_ && direction == last_direction_ && last_processor_) {
|
||||
// return;
|
||||
// }
|
||||
if (path == last_path_ && direction == last_direction_ && last_processor_) {
|
||||
processor_dirty_ = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
const QFileInfo info(path);
|
||||
if (!info.exists() || !info.isFile()) {
|
||||
qWarning() << "OCIO LUT file does not exist:" << path;
|
||||
set_processor(nullptr);
|
||||
const_cast<OCIOLutNode *>(this)->set_processor(nullptr);
|
||||
last_processor_.reset();
|
||||
last_path_.clear();
|
||||
last_direction_ = -1;
|
||||
return;
|
||||
processor_dirty_ = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
const QString suffix = info.suffix();
|
||||
if (!IsSupportedLutExtension(suffix)) {
|
||||
qWarning() << "Unsupported OCIO LUT file extension:" << path;
|
||||
set_processor(nullptr);
|
||||
const_cast<OCIOLutNode *>(this)->set_processor(nullptr);
|
||||
last_processor_.reset();
|
||||
last_path_.clear();
|
||||
last_direction_ = -1;
|
||||
return;
|
||||
processor_dirty_ = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
pending_path_ = path;
|
||||
pending_direction_ = direction;
|
||||
pending_generation_++;
|
||||
|
||||
// Synchronous generation: for typical 33^3 .cube files this is well under
|
||||
// 100ms and avoids the complexity and potential deadlocks of background
|
||||
// generation + cache invalidation.
|
||||
ColorProcessorPtr processor;
|
||||
try {
|
||||
OCIO::FileTransformRcPtr transform = OCIO::FileTransform::Create();
|
||||
@@ -184,21 +238,10 @@ void OCIOLutNode::GenerateProcessor()
|
||||
last_path_ = path;
|
||||
last_direction_ = direction;
|
||||
last_processor_ = processor;
|
||||
set_processor(processor);
|
||||
const_cast<OCIOLutNode *>(this)->set_processor(processor);
|
||||
processor_dirty_ = false;
|
||||
|
||||
// The processor has changed. In the main GUI process (QApplication), refresh
|
||||
// the viewer by invalidating the cache and cancelling background cache jobs.
|
||||
// Invalidating first ensures any in-flight renders that complete afterwards
|
||||
// won't write stale frames back. The worker process uses QGuiApplication and
|
||||
// has no RenderManager/PreviewAutoCacher, so skip this step to avoid crashing.
|
||||
if (qobject_cast<QApplication *>(QCoreApplication::instance())) {
|
||||
InvalidateAll(kTextureInput);
|
||||
if (RenderManager *rm = RenderManager::instance()) {
|
||||
if (PreviewAutoCacher *cacher = rm->GetCacher()) {
|
||||
cacher->CancelVideoTasks(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
} // namespace olive
|
||||
|
||||
@@ -44,6 +44,8 @@ public:
|
||||
virtual void Retranslate() override;
|
||||
virtual void InputValueChangedEvent(const QString &input,
|
||||
int element) override;
|
||||
virtual void Value(const NodeValueRow &value, const NodeGlobals &globals,
|
||||
NodeValueTable *table) const override;
|
||||
|
||||
static const QString kFileInput;
|
||||
static const QString kDirectionInput;
|
||||
@@ -53,15 +55,14 @@ protected slots:
|
||||
|
||||
private:
|
||||
void GenerateProcessor();
|
||||
void EnsureProcessor() const;
|
||||
bool CreateProcessorFromInputs() const;
|
||||
|
||||
QMutex gen_mutex_;
|
||||
QString last_path_;
|
||||
int last_direction_ = -1;
|
||||
ColorProcessorPtr last_processor_;
|
||||
|
||||
QString pending_path_;
|
||||
int pending_direction_ = -1;
|
||||
int pending_generation_ = 0;
|
||||
mutable QMutex gen_mutex_;
|
||||
mutable bool processor_dirty_ = true;
|
||||
mutable QString last_path_;
|
||||
mutable int last_direction_ = -1;
|
||||
mutable ColorProcessorPtr last_processor_;
|
||||
};
|
||||
|
||||
} // namespace olive
|
||||
|
||||
@@ -62,6 +62,11 @@ void ProjectCopier::SetProject(Project *project)
|
||||
original_ = project;
|
||||
|
||||
if (original_) {
|
||||
// The copied project is only used as an in-memory render proxy. Mark it so
|
||||
// downstream code (e.g. RenderWorkerPool) knows it is safe to reset its
|
||||
// modified flag after serializing a snapshot.
|
||||
copy_->setProperty("_oak_render_proxy", true);
|
||||
|
||||
// Add all nodes
|
||||
for (int i = 0; i < copy_->nodes().size(); i++) {
|
||||
InsertIntoCopyMap(original_->nodes().at(i), copy_->nodes().at(i));
|
||||
@@ -108,10 +113,13 @@ void ProjectCopier::SetProject(Project *project)
|
||||
|
||||
void ProjectCopier::ProcessUpdateQueue()
|
||||
{
|
||||
bool copy_changed = false;
|
||||
|
||||
// Iterate everything that happened to the graph and do the same thing on our end
|
||||
while (!graph_update_queue_.empty()) {
|
||||
QueuedJob job = graph_update_queue_.front();
|
||||
graph_update_queue_.pop_front();
|
||||
copy_changed = true;
|
||||
|
||||
switch (job.type) {
|
||||
case QueuedJob::kNodeAdded:
|
||||
@@ -138,6 +146,13 @@ void ProjectCopier::ProcessUpdateQueue()
|
||||
}
|
||||
}
|
||||
|
||||
// The copied project is not saved, so its modified flag is only used by the
|
||||
// render worker pool to decide whether the serialized graph snapshot is stale.
|
||||
// Mark it modified whenever the copy has actually changed.
|
||||
if (copy_changed) {
|
||||
copy_->set_modified(true);
|
||||
}
|
||||
|
||||
// Indicate that we have synchronized to this point, which is compared with the graph change
|
||||
// time to see if our copied graph is up to date
|
||||
UpdateLastSyncedValue();
|
||||
|
||||
@@ -572,6 +572,12 @@ bool RenderWorkerPool::PrepareJob(RenderTicketPtr ticket,
|
||||
return false;
|
||||
}
|
||||
wrote_new_snapshot = true;
|
||||
// For internal render-proxy copies, the snapshot now represents the
|
||||
// serialized state, so reset the modified flag so the snapshot can be
|
||||
// reused until the copy changes again.
|
||||
if (project->property("_oak_render_proxy").toBool()) {
|
||||
project->set_modified(false);
|
||||
}
|
||||
locker.relock();
|
||||
graph_cache_.insert(project_uuid, {graph_path});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user