From 547c2480e0b01ec8aeba26b0a1dbefbf3214417a Mon Sep 17 00:00:00 2001 From: Mike Solar Date: Thu, 16 Jul 2026 22:53:33 +0800 Subject: [PATCH] color: global LUT library, LUT error reporting, clamp + display fixes - Add a global LUT library: user-configurable directories (new Preferences > LUT tab) scanned recursively for .cube/.3dl files; LUT node file pickers offer the library dirs as sidebar shortcuts via a 'lut_library' input property handled by the param view bridge - OCIOLutNode no longer fails silently: missing files, unsupported extensions and OCIO load errors are recorded in last_error() and surfaced in the status bar (input still passes through for rendering safety) - ColorDialog: re-enable the display -> reference conversion using ColorProcessor::kInverse with a validity guard, and re-enable the Display tab in ColorValuesWidget; covered by a round-trip regression test proving the old OCIO inverse crash no longer occurs - OCIOGradingTransformLinearNode: enforce the OCIO clampWhite > clampBlack invariant per frame in Value() so keyframed/connected values cannot produce invalid grading transforms, and constrain the white clamp UI minimum whenever the black clamp is static - Regression tests for LUT extension checks, direction switching, node error reporting, LUT library scanning, display inverse round-trip and clamp enforcement --- app/config/config.cpp | 3 + app/dialog/color/colordialog.cpp | 20 +- app/dialog/preferences/preferences.cpp | 2 + app/dialog/preferences/tabs/CMakeLists.txt | 2 + .../preferences/tabs/preferencesluttab.cpp | 89 +++++ .../preferences/tabs/preferencesluttab.h | 44 +++ .../ociogradingtransformlinear.cpp | 66 +++- .../ociogradingtransformlinear.h | 14 + app/node/color/ociolut/ociolut.cpp | 39 +- app/node/color/ociolut/ociolut.h | 15 + app/render/CMakeLists.txt | 2 + app/render/lutlibrary.cpp | 86 +++++ app/render/lutlibrary.h | 68 ++++ app/widget/colorwheel/colorvalueswidget.cpp | 3 - app/widget/filefield/filefield.cpp | 28 +- app/widget/filefield/filefield.h | 13 + .../nodeparamviewwidgetbridge.cpp | 12 + tests/gtest/color_lut_test.cpp | 356 ++++++++++++++++++ 18 files changed, 829 insertions(+), 33 deletions(-) create mode 100644 app/dialog/preferences/tabs/preferencesluttab.cpp create mode 100644 app/dialog/preferences/tabs/preferencesluttab.h create mode 100644 app/render/lutlibrary.cpp create mode 100644 app/render/lutlibrary.h diff --git a/app/config/config.cpp b/app/config/config.cpp index 1b437f52d..da9cced50 100644 --- a/app/config/config.cpp +++ b/app/config/config.cpp @@ -240,6 +240,9 @@ void Config::SetDefaults() SetEntryInternal(QStringLiteral("FFmpegPath"), NodeValue::kText, QString()); + SetEntryInternal(QStringLiteral("LUTLibraryPaths"), NodeValue::kText, + QString()); + SetEntryInternal(QStringLiteral("DefaultSequenceWidth"), NodeValue::kInt, 1920); SetEntryInternal(QStringLiteral("DefaultSequenceHeight"), NodeValue::kInt, diff --git a/app/dialog/color/colordialog.cpp b/app/dialog/color/colordialog.cpp index c8f75643b..edbafe955 100644 --- a/app/dialog/color/colordialog.cpp +++ b/app/dialog/color/colordialog.cpp @@ -192,21 +192,21 @@ void ColorDialog::ColorSpaceChanged(const QString &input, ColorProcessorPtr ref_to_input = ColorProcessor::Create( color_manager_, color_manager_->GetReferenceColorSpace(), input); - // FIXME: For some reason, using OCIO::TRANSFORM_DIR_INVERSE (wrapped by ColorProcessor::kInverse) causes OCIO to - // crash. We've disabled that functionality for now (also disabling display_tab_ in ColorValuesWidget) - - /*ColorProcessorPtr display_to_ref = ColorProcessor::Create(color_manager_->GetConfig(), - color_manager_->GetReferenceColorSpace(), - display, - view, - look, - ColorProcessor::kInverse);*/ + // Display -> reference is the inverse of the display transform. Older OCIO + // versions crashed on TRANSFORM_DIR_INVERSE; guard by requiring a valid + // processor and fall back to disabling the display tab if creation fails. + ColorProcessorPtr display_to_ref = ColorProcessor::Create( + color_manager_, color_manager_->GetReferenceColorSpace(), output, + ColorProcessor::kInverse); + if (display_to_ref && !display_to_ref->GetProcessor()) { + display_to_ref = nullptr; + } color_wheel_->SetColorProcessor(input_to_ref_processor_, ref_to_display); hsv_value_gradient_->SetColorProcessor(input_to_ref_processor_, ref_to_display); color_values_widget_->SetColorProcessor( - input_to_ref_processor_, ref_to_display, nullptr, ref_to_input); + input_to_ref_processor_, ref_to_display, display_to_ref, ref_to_input); } } diff --git a/app/dialog/preferences/preferences.cpp b/app/dialog/preferences/preferences.cpp index ad1cb7d65..adbf0fe5b 100644 --- a/app/dialog/preferences/preferences.cpp +++ b/app/dialog/preferences/preferences.cpp @@ -33,6 +33,7 @@ #include "tabs/preferencesdisktab.h" #include "tabs/preferencesaudiotab.h" #include "tabs/preferenceskeyboardtab.h" +#include "tabs/preferencesluttab.h" #include "window/mainwindow/mainwindow.h" namespace olive @@ -60,6 +61,7 @@ PreferencesDialog::PreferencesDialog(MainWindow *main_window, int start_tab) new PreferencesBehaviorTab(PreferencesBehaviorTab::kCategoryRendering), tr("Rendering")); AddTab(new PreferencesDiskTab(), tr("Disk")); + AddTab(new PreferencesLutTab(), tr("LUT")); AddTab(new PreferencesKeyboardTab(main_window), tr("Keyboard")); SetCurrentTab(start_tab); diff --git a/app/dialog/preferences/tabs/CMakeLists.txt b/app/dialog/preferences/tabs/CMakeLists.txt index 9b3029a17..b85428fb3 100644 --- a/app/dialog/preferences/tabs/CMakeLists.txt +++ b/app/dialog/preferences/tabs/CMakeLists.txt @@ -22,6 +22,8 @@ set(OLIVE_SOURCES dialog/preferences/tabs/preferencesbehaviortab.cpp dialog/preferences/tabs/preferencesdisktab.h dialog/preferences/tabs/preferencesdisktab.cpp + dialog/preferences/tabs/preferencesluttab.h + dialog/preferences/tabs/preferencesluttab.cpp dialog/preferences/tabs/preferencesappearancetab.h dialog/preferences/tabs/preferencesappearancetab.cpp dialog/preferences/tabs/preferencesaudiotab.h diff --git a/app/dialog/preferences/tabs/preferencesluttab.cpp b/app/dialog/preferences/tabs/preferencesluttab.cpp new file mode 100644 index 000000000..4952bca0a --- /dev/null +++ b/app/dialog/preferences/tabs/preferencesluttab.cpp @@ -0,0 +1,89 @@ +/*** + + Oak - Non-Linear Video Editor + Copyright (C) 2026 Oak Team + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + +***/ + +#include "preferencesluttab.h" + +#include +#include +#include +#include +#include +#include + +#include "render/lutlibrary.h" + +namespace olive +{ + +PreferencesLutTab::PreferencesLutTab() +{ + QVBoxLayout *outer_layout = new QVBoxLayout(this); + + QGroupBox *library_group = new QGroupBox(tr("LUT Library")); + outer_layout->addWidget(library_group); + + QVBoxLayout *library_layout = new QVBoxLayout(library_group); + + library_layout->addWidget(new QLabel( + tr("Directories scanned for .cube and .3dl LUT files. LUT nodes offer " + "these locations when picking a LUT file."))); + + library_dirs_list_ = new QListWidget(); + library_dirs_list_->addItems(LUTLibrary::GetDirectories()); + library_layout->addWidget(library_dirs_list_); + + QHBoxLayout *button_layout = new QHBoxLayout(); + button_layout->addStretch(); + + QPushButton *add_btn = new QPushButton(tr("Add...")); + connect(add_btn, &QPushButton::clicked, this, [this]() { + const QString dir = QFileDialog::getExistingDirectory( + this, tr("Add LUT Library Directory")); + if (!dir.isEmpty() && + library_dirs_list_->findItems(dir, Qt::MatchExactly).isEmpty()) { + library_dirs_list_->addItem(dir); + } + }); + button_layout->addWidget(add_btn); + + QPushButton *remove_btn = new QPushButton(tr("Remove")); + connect(remove_btn, &QPushButton::clicked, this, [this]() { + qDeleteAll(library_dirs_list_->selectedItems()); + }); + button_layout->addWidget(remove_btn); + + library_layout->addLayout(button_layout); + + outer_layout->addStretch(); +} + +void PreferencesLutTab::Accept(MultiUndoCommand *command) +{ + Q_UNUSED(command) + + QStringList dirs; + for (int i = 0; i < library_dirs_list_->count(); i++) { + dirs.append(library_dirs_list_->item(i)->text()); + } + + LUTLibrary::SetDirectories(dirs); +} + +} diff --git a/app/dialog/preferences/tabs/preferencesluttab.h b/app/dialog/preferences/tabs/preferencesluttab.h new file mode 100644 index 000000000..27bd87b88 --- /dev/null +++ b/app/dialog/preferences/tabs/preferencesluttab.h @@ -0,0 +1,44 @@ +/*** + + Oak - Non-Linear Video Editor + Copyright (C) 2026 Oak Team + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + +***/ + +#ifndef PREFERENCESLUTTAB_H +#define PREFERENCESLUTTAB_H + +#include + +#include "dialog/configbase/configdialogbase.h" + +namespace olive +{ + +class PreferencesLutTab : public ConfigDialogBaseTab { + Q_OBJECT +public: + PreferencesLutTab(); + + virtual void Accept(MultiUndoCommand *command) override; + +private: + QListWidget *library_dirs_list_; +}; + +} + +#endif // PREFERENCESLUTTAB_H diff --git a/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.cpp b/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.cpp index cdc1118ee..da6bbcd81 100644 --- a/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.cpp +++ b/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.cpp @@ -92,10 +92,10 @@ OCIOGradingTransformLinearNode::OCIOGradingTransformLinearNode() GetStandardValue(kClampWhiteEnableInput).toBool()); SetInputProperty(kClampWhiteInput, QStringLiteral("base"), 0.01); - // FIXME: Temporarily disabled. This will break if "clamp black" is keyframed or connected to - // something and there's currently no solution to remedy that. If there is in the future, - // we can look into re-enabling this. - //SetInputProperty(kClampWhiteInput, QStringLiteral("min"), GetStandardValue(kClampBlackInput).toDouble() + 0.000001); + // Constrain the white clamp minimum to just above the (static) black clamp + // as per OCIO::GradingPrimary::validate. When the black clamp is keyframed + // or connected, Value() enforces the invariant per frame instead. + UpdateClampWhiteMinimum(); } QString OCIOGradingTransformLinearNode::Name() const @@ -149,16 +149,49 @@ void OCIOGradingTransformLinearNode::InputValueChangedEvent( SetInputProperty(kClampBlackInput, QStringLiteral("enabled"), GetStandardValue(kClampBlackEnableInput).toBool()); } else if (input == kClampBlackInput) { - // Ensure the white clamp is always greater than the black clamp as per OCIO::GradingPrimary::validate - // FIXME: Temporarily disabled. This will break if "clamp black" is keyframed or connected to - // something and there's currently no solution to remedy that. If there is in the future, - // we can look into re-enabling this. - //SetInputProperty(kClampWhiteInput, QStringLiteral("min"), GetStandardValue(kClampBlackInput).toDouble() + 0.000001); + // Ensure the white clamp is always greater than the black clamp as per + // OCIO::GradingPrimary::validate + UpdateClampWhiteMinimum(); } GenerateProcessor(); } +void OCIOGradingTransformLinearNode::InputConnectedEvent(const QString &input, + int element, Node *output) +{ + super::InputConnectedEvent(input, element, output); + + if (input == kClampBlackInput) { + UpdateClampWhiteMinimum(); + } +} + +void OCIOGradingTransformLinearNode::InputDisconnectedEvent(const QString &input, + int element, + Node *output) +{ + super::InputDisconnectedEvent(input, element, output); + + if (input == kClampBlackInput) { + UpdateClampWhiteMinimum(); + } +} + +void OCIOGradingTransformLinearNode::UpdateClampWhiteMinimum() +{ + // A static UI minimum cannot follow an animated black clamp; for keyframed + // or connected values the white>black invariant is enforced per frame in + // Value() instead + if (IsInputKeyframing(kClampBlackInput) || + IsInputConnected(kClampBlackInput)) { + return; + } + + SetInputProperty(kClampWhiteInput, QStringLiteral("min"), + GetStandardValue(kClampBlackInput).toDouble() + 0.000001); +} + void OCIOGradingTransformLinearNode::GenerateProcessor() { if (manager()) { @@ -241,6 +274,21 @@ void OCIOGradingTransformLinearNode::Value(const NodeValueRow &value, OCIO::GradingPrimary::NoClampWhite())); } + if (value[kClampBlackEnableInput].toBool() && + value[kClampWhiteEnableInput].toBool()) { + // OCIO::GradingPrimary::validate requires the white clamp to be + // greater than the black clamp. Keyframed or connected values + // can violate this at arbitrary times, so enforce the invariant + // per frame here. + const double clamp_black = value[kClampBlackInput].toDouble(); + const double clamp_white = value[kClampWhiteInput].toDouble(); + if (clamp_white <= clamp_black) { + job.Insert(kClampWhiteInput, + NodeValue(NodeValue::kFloat, + clamp_black + 0.000001)); + } + } + table->Push(NodeValue::kTexture, tex->toJob(job), this); } } diff --git a/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.h b/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.h index 85a2c9bf3..4e201dbe6 100644 --- a/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.h +++ b/app/node/color/ociogradingtransformlinear/ociogradingtransformlinear.h @@ -43,6 +43,10 @@ public: virtual void Retranslate() override; virtual void InputValueChangedEvent(const QString &input, int element) override; + virtual void InputConnectedEvent(const QString &input, int element, + Node *output) override; + virtual void InputDisconnectedEvent(const QString &input, int element, + Node *output) override; void GenerateProcessor(); virtual void Value(const NodeValueRow &value, const NodeGlobals &globals, @@ -63,6 +67,16 @@ protected slots: private: void SetVec4InputColors(const QString &input); + + /** + * @brief Constrains the white clamp UI minimum to just above the black + * clamp, as required by OCIO::GradingPrimary::validate + * + * Only applies while the black clamp is a static value; when it is + * keyframed or connected the invariant is enforced per frame in Value() + * instead. + */ + void UpdateClampWhiteMinimum(); }; } // olive diff --git a/app/node/color/ociolut/ociolut.cpp b/app/node/color/ociolut/ociolut.cpp index 00a82e534..48fe3e76b 100644 --- a/app/node/color/ociolut/ociolut.cpp +++ b/app/node/color/ociolut/ociolut.cpp @@ -25,7 +25,9 @@ #include +#include "core.h" #include "node/color/colormanager/colormanager.h" +#include "render/lutlibrary.h" #include "render/previewautocacher.h" #include "render/rendermanager.h" @@ -40,12 +42,6 @@ const QString OCIOLutNode::kDirectionInput = QStringLiteral("lut_dir_in"); namespace { -bool IsSupportedLutExtension(const QString &suffix) -{ - const QString lower = suffix.toLower(); - return lower == QStringLiteral("cube") || lower == QStringLiteral("3dl"); -} - bool IsMainProcess() { return qobject_cast(QCoreApplication::instance()) != @@ -86,6 +82,8 @@ OCIOLutNode::OCIOLutNode() tr("LUT Files (*.cube *.3dl);;Cube LUT (*.cube);;3DL LUT (*.3dl);;All Files (*)")); SetInputProperty(kFileInput, QStringLiteral("placeholder"), tr("Select a .cube or .3dl LUT file")); + // Allow the UI to offer the global LUT library for this input + SetInputProperty(kFileInput, QStringLiteral("lut_library"), true); AddInput(kDirectionInput, NodeValue::kCombo, 0, InputFlags(kInputFlagNotKeyframable | kInputFlagNotConnectable)); @@ -194,6 +192,21 @@ void OCIOLutNode::EnsureProcessor() const CreateProcessorFromInputs(); } +void OCIOLutNode::SetLastError(const QString &error) const +{ + if (last_error_ == error) { + return; + } + + last_error_ = error; + + // Make the error visible to the user instead of failing silently, but only + // from the main process (the render worker has no status bar) + if (!error.isEmpty() && IsMainProcess() && Core::instance()) { + Core::instance()->ShowStatusBarMessage(error, 10000); + } +} + bool OCIOLutNode::CreateProcessorFromInputs() const { if (!manager()) { @@ -214,6 +227,7 @@ bool OCIOLutNode::CreateProcessorFromInputs() const last_path_.clear(); last_direction_ = -1; processor_dirty_ = false; + SetLastError(QString()); return false; } @@ -231,17 +245,22 @@ bool OCIOLutNode::CreateProcessorFromInputs() const last_path_.clear(); last_direction_ = -1; processor_dirty_ = false; + SetLastError(tr("OCIO LUT: file does not exist: %1").arg(path)); return false; } const QString suffix = info.suffix(); - if (!IsSupportedLutExtension(suffix)) { + if (!LUTLibrary::IsSupportedExtension(suffix)) { qWarning() << "Unsupported OCIO LUT file extension:" << path; const_cast(this)->set_processor(nullptr); last_processor_.reset(); last_path_.clear(); last_direction_ = -1; processor_dirty_ = false; + SetLastError( + tr("OCIO LUT: unsupported LUT file extension (expected .cube or " + ".3dl): %1") + .arg(path)); return false; } @@ -267,6 +286,12 @@ bool OCIOLutNode::CreateProcessorFromInputs() const processor = nullptr; } + if (!processor) { + SetLastError(tr("OCIO LUT: failed to load LUT file: %1").arg(path)); + } else { + SetLastError(QString()); + } + last_path_ = path; last_direction_ = direction; last_processor_ = processor; diff --git a/app/node/color/ociolut/ociolut.h b/app/node/color/ociolut/ociolut.h index a775fe9ee..0f36620cb 100644 --- a/app/node/color/ociolut/ociolut.h +++ b/app/node/color/ociolut/ociolut.h @@ -50,6 +50,18 @@ public: static const QString kFileInput; static const QString kDirectionInput; + /** + * @brief Human-readable description of why no LUT processor is active + * + * Empty when a valid LUT processor is in use or no LUT file has been + * selected yet. This allows the UI (and tests) to surface silent + * passthrough states (missing file, unsupported extension, OCIO errors). + */ + const QString &last_error() const + { + return last_error_; + } + protected slots: virtual void ConfigChanged() override; @@ -58,11 +70,14 @@ private: void EnsureProcessor() const; bool CreateProcessorFromInputs() const; + void SetLastError(const QString &error) const; + mutable QMutex gen_mutex_; mutable bool processor_dirty_ = true; mutable QString last_path_; mutable int last_direction_ = -1; mutable ColorProcessorPtr last_processor_; + mutable QString last_error_; }; } // namespace olive diff --git a/app/render/CMakeLists.txt b/app/render/CMakeLists.txt index 69a60d657..c4cfc36aa 100644 --- a/app/render/CMakeLists.txt +++ b/app/render/CMakeLists.txt @@ -40,6 +40,8 @@ set(OLIVE_SOURCES render/framehashcache.h render/framemanager.cpp render/framemanager.h + render/lutlibrary.cpp + render/lutlibrary.h render/interlacetexture.cpp render/loopmode.h render/managedcolor.cpp diff --git a/app/render/lutlibrary.cpp b/app/render/lutlibrary.cpp new file mode 100644 index 000000000..c9d4be2e8 --- /dev/null +++ b/app/render/lutlibrary.cpp @@ -0,0 +1,86 @@ +/*** + + Oak - Non-Linear Video Editor + Copyright (C) 2026 Oak Team + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + +***/ + +#include "lutlibrary.h" + +#include +#include +#include + +#include "config/config.h" + +namespace olive +{ + +bool LUTLibrary::IsSupportedExtension(const QString &suffix) +{ + QString s = suffix; + if (s.startsWith(QLatin1Char('.'))) { + s.remove(0, 1); + } + + const QString lower = s.toLower(); + return lower == QStringLiteral("cube") || lower == QStringLiteral("3dl"); +} + +QStringList LUTLibrary::GetDirectories() +{ + const QString serialized = OLIVE_CONFIG("LUTLibraryPaths").toString(); + + QStringList dirs = serialized.split(QLatin1Char(';'), Qt::SkipEmptyParts); + for (QString &dir : dirs) { + dir = QDir::fromNativeSeparators(dir.trimmed()); + } + return dirs; +} + +void LUTLibrary::SetDirectories(const QStringList &dirs) +{ + QStringList cleaned; + for (const QString &dir : dirs) { + const QString trimmed = dir.trimmed(); + if (!trimmed.isEmpty() && !cleaned.contains(trimmed)) { + cleaned.append(trimmed); + } + } + + Config::Current()[QStringLiteral("LUTLibraryPaths")] = + cleaned.join(QLatin1Char(';')); +} + +QStringList LUTLibrary::GetLutFiles() +{ + QStringList files; + + static const QStringList kFilters = { QStringLiteral("*.cube"), + QStringLiteral("*.3dl") }; + + for (const QString &dir : GetDirectories()) { + QDirIterator it(dir, kFilters, QDir::Files, + QDirIterator::Subdirectories); + while (it.hasNext()) { + files.append(it.next()); + } + } + + return files; +} + +} diff --git a/app/render/lutlibrary.h b/app/render/lutlibrary.h new file mode 100644 index 000000000..2dbb7e19c --- /dev/null +++ b/app/render/lutlibrary.h @@ -0,0 +1,68 @@ +/*** + + Oak - Non-Linear Video Editor + Copyright (C) 2026 Oak Team + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + +***/ + +#ifndef LUTLIBRARY_H +#define LUTLIBRARY_H + +#include +#include + +namespace olive +{ + +/** + * @brief A global, user-configurable library of LUT files + * + * The library is a list of directories (stored in the application config + * under "LUTLibraryPaths") that are scanned for supported LUT files. LUT + * nodes can offer the library contents as quick picks instead of forcing + * the user to browse for a file path on every node. + */ +class LUTLibrary { +public: + /** + * @brief Returns true if the given file suffix is a supported LUT + * extension (.cube or .3dl, case-insensitive, leading dot tolerated) + */ + static bool IsSupportedExtension(const QString &suffix); + + /** + * @brief The directories that make up the LUT library + */ + static QStringList GetDirectories(); + + /** + * @brief Replaces the LUT library directories and saves them to the + * application config + */ + static void SetDirectories(const QStringList &dirs); + + /** + * @brief All supported LUT files found under the library directories + * + * Directories are scanned recursively. Files in earlier directories + * are listed first. + */ + static QStringList GetLutFiles(); +}; + +} + +#endif // LUTLIBRARY_H diff --git a/app/widget/colorwheel/colorvalueswidget.cpp b/app/widget/colorwheel/colorvalueswidget.cpp index a2229ff34..6f835c59f 100644 --- a/app/widget/colorwheel/colorvalueswidget.cpp +++ b/app/widget/colorwheel/colorvalueswidget.cpp @@ -91,9 +91,6 @@ ColorValuesWidget::ColorValuesWidget(ColorManager *manager, QWidget *parent) connect(display_tab_, &ColorValuesTab::ColorChanged, this, &ColorValuesWidget::UpdateValuesFromDisplay); - // FIXME: Display -> Ref temporarily disabled due to OCIO crash (see ColorDialog::ColorSpaceChanged for more info) - display_tab_->setEnabled(false); - layout->addWidget(tabs); } } diff --git a/app/widget/filefield/filefield.cpp b/app/widget/filefield/filefield.cpp index 1f02099c0..9a66f980a 100644 --- a/app/widget/filefield/filefield.cpp +++ b/app/widget/filefield/filefield.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include "ui/icons/icons.h" @@ -56,11 +57,30 @@ void FileField::BrowseBtnClicked() { QString s; - if (directory_mode_) { - s = QFileDialog::getExistingDirectory(this, tr("Open Directory")); + if (sidebar_urls_.isEmpty()) { + if (directory_mode_) { + s = QFileDialog::getExistingDirectory(this, tr("Open Directory")); + } else { + s = QFileDialog::getOpenFileName(this, tr("Open File"), QString(), + name_filter_); + } } else { - s = QFileDialog::getOpenFileName(this, tr("Open File"), QString(), - name_filter_); + // Sidebar URLs require the non-static dialog API + QFileDialog dialog(this, tr("Open File")); + dialog.setFileMode(directory_mode_ ? QFileDialog::Directory : + QFileDialog::ExistingFile); + dialog.setAcceptMode(QFileDialog::AcceptOpen); + if (!directory_mode_ && !name_filter_.isEmpty()) { + dialog.setNameFilter(name_filter_); + } + dialog.setSidebarUrls(sidebar_urls_); + if (directory_mode_) { + dialog.setOption(QFileDialog::ShowDirsOnly, true); + } + + if (dialog.exec() == QDialog::Accepted && !dialog.selectedFiles().isEmpty()) { + s = dialog.selectedFiles().first(); + } } if (!s.isEmpty()) { diff --git a/app/widget/filefield/filefield.h b/app/widget/filefield/filefield.h index 472f8944f..a5dea1910 100644 --- a/app/widget/filefield/filefield.h +++ b/app/widget/filefield/filefield.h @@ -58,6 +58,17 @@ public: name_filter_ = filter; } + /** + * @brief Sets extra sidebar shortcuts (e.g. a library directory) for the + * browse dialog + * + * Note: setting sidebar URLs requires Qt's non-native file dialog. + */ + void SetSidebarUrls(const QList &urls) + { + sidebar_urls_ = urls; + } + signals: void FilenameChanged(const QString &filename); @@ -70,6 +81,8 @@ private: QString name_filter_; + QList sidebar_urls_; + private slots: void BrowseBtnClicked(); diff --git a/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp b/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp index adb395323..f573bb971 100644 --- a/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp +++ b/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -36,6 +37,7 @@ #include "node/project/sequence/sequence.h" #include "nodeparamviewarraywidget.h" #include "nodeparamviewtextedit.h" +#include "render/lutlibrary.h" #include "undo/undostack.h" #include "widget/bezier/bezierwidget.h" #include "widget/colorbutton/colorbutton.h" @@ -938,6 +940,16 @@ void NodeParamViewWidgetBridge::SetProperty(const QString &key, ff->SetDirectoryMode(value.toBool()); } else if (key == QStringLiteral("filter")) { ff->SetNameFilter(value.toString()); + } else if (key == QStringLiteral("lut_library") && value.toBool()) { + // Offer the global LUT library directories as sidebar shortcuts in + // the browse dialog + QList sidebar_urls; + for (const QString &dir : LUTLibrary::GetDirectories()) { + sidebar_urls.append(QUrl::fromLocalFile(dir)); + } + if (!sidebar_urls.isEmpty()) { + ff->SetSidebarUrls(sidebar_urls); + } } } diff --git a/tests/gtest/color_lut_test.cpp b/tests/gtest/color_lut_test.cpp index d086f6e77..1a163a14c 100644 --- a/tests/gtest/color_lut_test.cpp +++ b/tests/gtest/color_lut_test.cpp @@ -8,13 +8,17 @@ #include +#include "config/config.h" #include "node/color/ociolut/ociolut.h" +#include "node/color/colormanager/colormanager.h" +#include "node/color/ociogradingtransformlinear/ociogradingtransformlinear.h" #include "node/color/threewaycolor/threewaycolor.h" #include "node/factory.h" #include "node/generator/solid/solid.h" #include "node/project.h" #include "node/traverser.h" #include "render/colorprocessor.h" +#include "render/lutlibrary.h" namespace OCIO = OCIO_NAMESPACE; @@ -748,3 +752,355 @@ TEST(ColorLutNode, SwitchingBackToOriginalFileRestoresOriginalPixels) EXPECT_NEAR(restored_out.green(), invert_out.green(), 0.02f); EXPECT_NEAR(restored_out.blue(), invert_out.blue(), 0.02f); } + +// ----------------------------------------------------------------------------- +// Error reporting: silent passthrough states must be observable. +// ----------------------------------------------------------------------------- + +TEST(ColorLutNode, MissingFileSetsLastError) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + auto *lut = new olive::OCIOLutNode(); + lut->setParent(&project); + lut->SetStandardValue(olive::OCIOLutNode::kFileInput, + QStringLiteral("/nonexistent/path/lut.cube")); + + EXPECT_FALSE(lut->last_error().isEmpty()); +} + +TEST(ColorLutNode, UnsupportedExtensionSetsLastError) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = QDir(dir.path()).filePath(QStringLiteral("lut.txt")); + QFile file(path); + ASSERT_TRUE(file.open(QIODevice::WriteOnly)); + file.close(); + + auto *lut = new olive::OCIOLutNode(); + lut->setParent(&project); + lut->SetStandardValue(olive::OCIOLutNode::kFileInput, path); + + EXPECT_FALSE(lut->last_error().isEmpty()); +} + +TEST(ColorLutNode, ValidFileClearsLastError) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = WriteTestCubeLut(&dir, "invert", 0.0f, 1.0f); + ASSERT_FALSE(path.isEmpty()); + + auto *lut = new olive::OCIOLutNode(); + lut->setParent(&project); + + lut->SetStandardValue(olive::OCIOLutNode::kFileInput, + QStringLiteral("/nonexistent/path/lut.cube")); + EXPECT_FALSE(lut->last_error().isEmpty()); + + lut->SetStandardValue(olive::OCIOLutNode::kFileInput, path); + EXPECT_TRUE(lut->last_error().isEmpty()); +} + +TEST(ColorLutNode, EmptyPathClearsLastError) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + auto *lut = new olive::OCIOLutNode(); + lut->setParent(&project); + + lut->SetStandardValue(olive::OCIOLutNode::kFileInput, + QStringLiteral("/nonexistent/path/lut.cube")); + EXPECT_FALSE(lut->last_error().isEmpty()); + + lut->SetStandardValue(olive::OCIOLutNode::kFileInput, QString()); + EXPECT_TRUE(lut->last_error().isEmpty()); +} + +// ----------------------------------------------------------------------------- +// Global LUT library +// ----------------------------------------------------------------------------- + +TEST(LUTLibrary, SupportsCubeAnd3dlExtensions) +{ + EXPECT_TRUE(olive::LUTLibrary::IsSupportedExtension(QStringLiteral("cube"))); + EXPECT_TRUE( + olive::LUTLibrary::IsSupportedExtension(QStringLiteral(".cube"))); + EXPECT_TRUE(olive::LUTLibrary::IsSupportedExtension(QStringLiteral("CUBE"))); + EXPECT_TRUE(olive::LUTLibrary::IsSupportedExtension(QStringLiteral("3dl"))); + EXPECT_TRUE(olive::LUTLibrary::IsSupportedExtension(QStringLiteral(".3dl"))); + EXPECT_FALSE(olive::LUTLibrary::IsSupportedExtension(QStringLiteral("txt"))); + EXPECT_FALSE(olive::LUTLibrary::IsSupportedExtension(QString())); +} + +TEST(LUTLibrary, DirectoryRoundTripCleansAndDeduplicates) +{ + const QString previous = + olive::Config::Current()[QStringLiteral("LUTLibraryPaths")].toString(); + + olive::LUTLibrary::SetDirectories( + { QStringLiteral("/a/luts"), QStringLiteral(" /a/luts "), + QStringLiteral("/b/luts"), QString() }); + + EXPECT_EQ(olive::LUTLibrary::GetDirectories(), + (QStringList{ QStringLiteral("/a/luts"), + QStringLiteral("/b/luts") })); + + olive::Config::Current()[QStringLiteral("LUTLibraryPaths")] = previous; +} + +TEST(LUTLibrary, ScansDirectoriesRecursivelyForSupportedLuts) +{ + const QString previous = + olive::Config::Current()[QStringLiteral("LUTLibraryPaths")].toString(); + + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + + const QString cube_path = + QDir(dir.path()).filePath(QStringLiteral("top.cube")); + const QString sub_dir = QDir(dir.path()).filePath(QStringLiteral("sub")); + const QString three_dl_path = + QDir(sub_dir).filePath(QStringLiteral("nested.3dl")); + const QString text_path = + QDir(dir.path()).filePath(QStringLiteral("skip.txt")); + + ASSERT_TRUE(QDir().mkpath(sub_dir)); + for (const QString &p : { cube_path, three_dl_path, text_path }) { + QFile file(p); + ASSERT_TRUE(file.open(QIODevice::WriteOnly)); + file.close(); + } + + olive::LUTLibrary::SetDirectories({ dir.path() }); + + const QStringList files = olive::LUTLibrary::GetLutFiles(); + EXPECT_EQ(files.size(), 2); + EXPECT_TRUE(files.contains(cube_path)); + EXPECT_TRUE(files.contains(three_dl_path)); + EXPECT_FALSE(files.contains(text_path)); + + olive::Config::Current()[QStringLiteral("LUTLibraryPaths")] = previous; +} + +// ----------------------------------------------------------------------------- +// Display -> reference inverse transform (previously disabled over an OCIO +// crash; covered here so the ColorDialog conversion can stay enabled). +// ----------------------------------------------------------------------------- + +TEST(ColorProcessor, InverseDisplayTransformRoundTripsColor) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + olive::ColorManager *color_manager = project.color_manager(); + ASSERT_NE(color_manager, nullptr); + + const QString display = color_manager->GetDefaultDisplay(); + const QString view = color_manager->GetDefaultView(display); + ASSERT_FALSE(display.isEmpty()); + ASSERT_FALSE(view.isEmpty()); + + const olive::ColorTransform output_transform(display, view, QString()); + + olive::ColorProcessorPtr ref_to_display = olive::ColorProcessor::Create( + color_manager, color_manager->GetReferenceColorSpace(), + output_transform); + ASSERT_NE(ref_to_display, nullptr); + ASSERT_NE(ref_to_display->GetProcessor(), nullptr); + + olive::ColorProcessorPtr display_to_ref = olive::ColorProcessor::Create( + color_manager, color_manager->GetReferenceColorSpace(), + output_transform, olive::ColorProcessor::kInverse); + ASSERT_NE(display_to_ref, nullptr); + ASSERT_NE(display_to_ref->GetProcessor(), nullptr); + + const olive::Color reference(0.2f, 0.4f, 0.6f, 1.0f); + const olive::Color display_color = ref_to_display->ConvertColor(reference); + const olive::Color round_trip = display_to_ref->ConvertColor(display_color); + + EXPECT_NEAR(round_trip.red(), reference.red(), 0.001f); + EXPECT_NEAR(round_trip.green(), reference.green(), 0.001f); + EXPECT_NEAR(round_trip.blue(), reference.blue(), 0.001f); + EXPECT_NEAR(round_trip.alpha(), reference.alpha(), 0.001f); +} + +// ----------------------------------------------------------------------------- +// OCIOGradingTransformLinearNode clamp invariant +// ----------------------------------------------------------------------------- + +namespace +{ + +class ClampCaptureTraverser : public PixelColorTransformTraverser { +public: + bool captured_white_clamp = false; + double white_clamp_value = 0.0; + +protected: + virtual void + ProcessColorTransform(olive::TexturePtr destination, const olive::Node *node, + const olive::ColorTransformJob *job) override + { + const olive::NodeValueRow &values = job->GetValues(); + if (values.contains( + olive::OCIOGradingTransformLinearNode::kClampWhiteInput)) { + white_clamp_value = + values + .value(olive::OCIOGradingTransformLinearNode:: + kClampWhiteInput) + .toDouble(); + captured_white_clamp = true; + } + PixelColorTransformTraverser::ProcessColorTransform(destination, node, + job); + } +}; + +} // namespace + +TEST(ColorGradingLinear, InvalidClampRangeIsCorrectedPerFrame) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + auto *solid = new olive::SolidGenerator(); + solid->setParent(&project); + solid->SetStandardValue( + olive::SolidGenerator::kColorInput, + QVariant::fromValue(olive::Color(0.25f, 0.50f, 0.75f, 1.0f))); + + auto *grading = new olive::OCIOGradingTransformLinearNode(); + grading->setParent(&project); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampBlackEnableInput, true); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampWhiteEnableInput, true); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampBlackInput, 0.5); + // Invalid: white clamp below black clamp + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampWhiteInput, 0.0); + + olive::Node::ConnectEdge( + solid, + olive::NodeInput( + grading, + olive::OCIOGradingTransformLinearNode::kTextureInput)); + + const olive::VideoParams params(16, 16, olive::core::PixelFormat::F32, + olive::VideoParams::kRGBAChannelCount); + + ClampCaptureTraverser traverser; + traverser.SetCacheVideoParams(params); + olive::NodeValueTable table = traverser.GenerateTable( + grading, olive::TimeRange(olive::core::rational(0), + olive::core::rational(1, 30))); + olive::NodeValue tex_val = table.Get(olive::NodeValue::kTexture); + traverser.Resolve(tex_val); + + // The node must have corrected the white clamp to just above the black + // clamp instead of feeding an invalid grading primary to OCIO + ASSERT_TRUE(traverser.captured_white_clamp); + EXPECT_NEAR(traverser.white_clamp_value, 0.500001, 1e-9); + + // And rendering must not crash or drop the frame + ASSERT_TRUE(traverser.output_frame); +} + +TEST(ColorGradingLinear, ValidClampRangeIsLeftUntouched) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + auto *solid = new olive::SolidGenerator(); + solid->setParent(&project); + solid->SetStandardValue( + olive::SolidGenerator::kColorInput, + QVariant::fromValue(olive::Color(0.25f, 0.50f, 0.75f, 1.0f))); + + auto *grading = new olive::OCIOGradingTransformLinearNode(); + grading->setParent(&project); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampBlackEnableInput, true); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampWhiteEnableInput, true); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampBlackInput, 0.1); + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampWhiteInput, 0.9); + + olive::Node::ConnectEdge( + solid, + olive::NodeInput( + grading, + olive::OCIOGradingTransformLinearNode::kTextureInput)); + + const olive::VideoParams params(16, 16, olive::core::PixelFormat::F32, + olive::VideoParams::kRGBAChannelCount); + + ClampCaptureTraverser traverser; + traverser.SetCacheVideoParams(params); + olive::NodeValueTable table = traverser.GenerateTable( + grading, olive::TimeRange(olive::core::rational(0), + olive::core::rational(1, 30))); + olive::NodeValue tex_val = table.Get(olive::NodeValue::kTexture); + traverser.Resolve(tex_val); + + ASSERT_TRUE(traverser.captured_white_clamp); + EXPECT_NEAR(traverser.white_clamp_value, 0.9, 1e-9); + + ASSERT_TRUE(traverser.output_frame); +} + +TEST(ColorGradingLinear, StaticBlackClampConstrainsWhiteMinimum) +{ + olive::ColorManager::SetUpDefaultConfig(); + + olive::Project project; + project.Initialize(); + + auto *grading = new olive::OCIOGradingTransformLinearNode(); + grading->setParent(&project); + + // Default black clamp is 0, so the white minimum starts just above it + EXPECT_NEAR(grading + ->GetInputProperty( + olive::OCIOGradingTransformLinearNode::kClampWhiteInput, + QStringLiteral("min")) + .toDouble(), + 0.000001, 1e-9); + + // Changing the static black clamp updates the white minimum + grading->SetStandardValue( + olive::OCIOGradingTransformLinearNode::kClampBlackInput, 0.25); + EXPECT_NEAR(grading + ->GetInputProperty( + olive::OCIOGradingTransformLinearNode::kClampWhiteInput, + QStringLiteral("min")) + .toDouble(), + 0.250001, 1e-9); +}