From 187a0318d3ff6578c3a282892ecacc5558c50e99 Mon Sep 17 00:00:00 2001 From: Lawrence Lee <45837045+Lawrence37@users.noreply.github.com> Date: Sat, 18 Jan 2025 18:01:30 -0800 Subject: [PATCH 1/3] Fix pp3 deletion when clearing processing params Fix comparison of original vs. current rank. The rank value can be -1, which should be interpreted as 0. This allows proper assessment if the rank has been changed. The pp3 is deleted if and only if the rank, color label, and trash status are unchanged. pp3s were not getting deleted because of the mismatch between the original rank (-1) and current rank (0). Also consider the rank and color label in the metadata and XMP sidecars. If the rank and color label have not been edited in the current session and the current values match the ones in the metadata and XMP sidecars, the pp3 can be deleted. --- rtgui/thumbnail.cc | 178 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 157 insertions(+), 21 deletions(-) diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc index 549c861bb..26c96546e 100644 --- a/rtgui/thumbnail.cc +++ b/rtgui/thumbnail.cc @@ -145,6 +145,133 @@ const std::map defaultColors = { auto defaultColorMapper = ColorMapper(defaultColors); +/** + * Gets the rank from the image metadata, if it exists. + * + * @param cfs The cached image data. + * @param fname The image's file name. + * @param rank Where the rank will be stored. If there is no rank in the + * metadata, the value will not be changed. + * @returns If the rank is in the metadata. + */ +bool getRankFromMetadata( + const CacheImageData &cfs, const Glib::ustring &fname, int &rank) +{ + if (cfs.exifValid) { + rank = rtengine::LIM(cfs.getRating(), 0, 5); + return true; + } + const std::unique_ptr md(rtengine::FramesMetaData::fromFile(fname)); + if (md && md->hasExif()) { + rank = rtengine::LIM(md->getRating(), 0, 5); + return true; + } + return false; +} + +/** + * Gets the rank from the XMP. + * + * @param xmp The XMP data. + * @param rank Where the rank will be stored. If there is no rank in the XMP, + * the value will not be changed. + * @returns If the rank is in the XMP. + */ +bool getRankFromXmp(const Exiv2::XmpData &xmp, int &rank) +{ + auto pos = xmp.findKey(Exiv2::XmpKey("Xmp.xmp.Rating")); + if (pos != xmp.end()) { + int r = rtengine::to_long(pos); + rank = rtengine::LIM(r, 0, 5); + return true; + } + return false; +} + +/** + * Gets the rank from the XMP or image metadata. + * + * The priority is to load from the XMP. The XMP will only be used if the + * option's thumbnail rank/color mode is set to XMP. If no rank is retrieved + * from the XMP, an attempt to get the rank from the metadata will be made. + * + * @param options Options. + * @param xmp The XMP data. + * @param cfs The cached image data. + * @param fname The image's file name. + * @param rank Where the rank will be stored. If there is no rank retrieved from + * the XMP and there is no rank in the metadata, the value will not be changed. + * @returns If a rank was retrieved. + */ +bool getRankFromXmpOrMetadata( + const Options &options, + const Exiv2::XmpData &xmp, + const CacheImageData &cfs, + const Glib::ustring &fname, + int &rank) +{ + bool got_rank_from_xmp = false; + if (options.thumbnailRankColorMode == Options::ThumbnailPropertyMode::XMP) { + try { + got_rank_from_xmp = getRankFromXmp(xmp, rank); + } catch (std::exception &exc) { + std::cerr << "ERROR loading rank from " + << rtengine::Exiv2Metadata::xmpSidecarPath(fname) + << ": " << exc.what() << std::endl; + } + } + return got_rank_from_xmp || getRankFromMetadata(cfs, fname, rank); +} + +/** + * Gets the color label from the XMP. + * + * @param xmp The XMP data. + * @param color Where the color will be stored. If there is no color in the XMP, + * the value will not be changed. + * @returns If the color is in the XMP. + */ +bool getColorFromXmp(const Exiv2::XmpData &xmp, int &color) +{ + auto pos = xmp.findKey(Exiv2::XmpKey("Xmp.xmp.Label")); + if (pos != xmp.end()) { + color = defaultColorMapper.index(pos->toString()); + return true; + } + return false; +} + +/** + * Gets the color label from the XMP. + * + * The XMP will only be used if the option's thumbnail rank/color mode is set to + * XMP. + * + * @param options Options. + * @param xmp The XMP data. + * @param fname The image's file name. + * @param color Where the color will be stored. If there is no color in the XMP, + * the value will not be changed. + * @returns If the color is in the XMP. + */ +bool getColorFromXmpOrNone( + const Options &options, + const Exiv2::XmpData &xmp, + const Glib::ustring &fname, + int &color) +{ + if (options.thumbnailRankColorMode == Options::ThumbnailPropertyMode::XMP) { + try { + return getColorFromXmp(xmp, color); + } catch (std::exception &exc) { + std::cerr << "ERROR loading color label from " + << rtengine::Exiv2Metadata::xmpSidecarPath(fname) + << ": " << exc.what() << std::endl; + } + } + return false; +} + } // namespace using namespace rtengine::procparams; @@ -1252,14 +1379,7 @@ void Thumbnail::loadProperties() properties = Properties(); // get initial rank from cache or image metadata - if (cfs.exifValid) { - properties.rank.value = rtengine::LIM(cfs.getRating(), 0, 5); - } else { - const std::unique_ptr md(rtengine::FramesMetaData::fromFile(fname)); - if (md && md->hasExif()) { - properties.rank.value = rtengine::LIM(md->getRating(), 0, 5); - } - } + getRankFromMetadata(cfs, fname, properties.rank.value); // update rank and color from procparams or xmp sidecar // load trash from procparams @@ -1277,16 +1397,8 @@ void Thumbnail::loadProperties() if (options.thumbnailRankColorMode == Options::ThumbnailPropertyMode::XMP) { try { auto xmp = rtengine::Exiv2Metadata::getXmpSidecar(fname); - auto pos = xmp.findKey(Exiv2::XmpKey("Xmp.xmp.Rating")); - if (pos != xmp.end()) { - int r = rtengine::to_long(pos); - properties.rank.value = rtengine::LIM(r, 0, 5); - } - - pos = xmp.findKey(Exiv2::XmpKey("Xmp.xmp.Label")); - if (pos != xmp.end()) { - properties.color.value = defaultColorMapper.index(pos->toString()); - } + getRankFromXmp(xmp, properties.rank.value); + getColorFromXmp(xmp, properties.color.value); } catch (std::exception &exc) { std::cerr << "ERROR loading thumbnail properties data from " << rtengine::Exiv2Metadata::xmpSidecarPath(fname) @@ -1306,16 +1418,40 @@ void Thumbnail::updateProcParamsProperties(bool forceUpdate) pparamsValid = true; } + const auto getXmpSidecar = [this]() { + static auto xmp = rtengine::Exiv2Metadata::getXmpSidecar(fname); + return xmp; + }; + // save procparams rank and color also when options.thumbnailRankColorMode == Options::ThumbnailPropertyMode::XMP // so they'll be kept in sync - if ((properties.rank.edited || forceUpdate) && properties.rank != pparams->rank) { + // Rank can be -1 to prioritize the rank in the metadata. If the metadata + // rank doesn't exist, it is interpreted as 0. + if ((properties.rank.edited || forceUpdate) && + rtengine::LIM(properties.rank.value, 0, 5) != rtengine::LIM(pparams->rank, 0, 5)) { pparams->rank = properties.rank; - pparamsValid = true; + pparamsValid |= properties.rank.edited; + if (!pparamsValid && forceUpdate) { + // When force-updating, the processing parameters' rank needs not be + // used if the embedded rank is the same. + int initial_rank = 0; + bool has_initial_rank = getRankFromXmpOrMetadata( + options, getXmpSidecar(), cfs, fname, initial_rank); + pparamsValid |= !(has_initial_rank && properties.rank == initial_rank); + } } if ((properties.color.edited || forceUpdate) && properties.color != pparams->colorlabel) { pparams->colorlabel = properties.color; - pparamsValid = true; + pparamsValid |= properties.color.edited; + if (!pparamsValid && forceUpdate) { + // When force-updating, the processing parameters' color label needs + // not be used if the embedded color label is the same. + int initial_color = 0; + bool has_initial_color = getColorFromXmpOrNone( + options, getXmpSidecar(), fname, initial_color); + pparamsValid |= !(has_initial_color && properties.color == initial_color); + } } } From 367041e2e65660c8f31706bb380bc667d22188ac Mon Sep 17 00:00:00 2001 From: Lawrence Lee <45837045+Lawrence37@users.noreply.github.com> Date: Mon, 20 Jan 2025 12:49:36 -0800 Subject: [PATCH 2/3] Fix processing params clear when XMP sync enabled Fix caching of XMP when clearing processing parameters so that it knows if the pp3 can be deleted without affecting the rank and color labels. --- rtengine/utils.h | 32 ++++++++++++++++++++++++++++++++ rtgui/thumbnail.cc | 8 ++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/rtengine/utils.h b/rtengine/utils.h index 5dec93f51..f57163d1e 100644 --- a/rtengine/utils.h +++ b/rtengine/utils.h @@ -18,6 +18,7 @@ */ #pragma once +#include #include #include @@ -26,6 +27,37 @@ namespace rtengine { +/** + * A function object that supplies a value and returns the same value for + * subsequent calls. + */ +template +struct MemoizingSupplier { + using Supplier = std::function; + + /** + * @param supplier The delegate supplier. + */ + explicit MemoizingSupplier(const Supplier &supplier) : + supplier(supplier) + { + } + + T operator()() const + { + if (!is_cached) { + value = supplier(); + is_cached = true; + } + return value; + } + +private: + const Supplier supplier; + mutable T value; + mutable bool is_cached{false}; +}; + // Update a point of a Cairo::Surface by accessing the raw data void poke255_uc(unsigned char*& dest, unsigned char r, unsigned char g, unsigned char b); // Update a point of a Cairo::Surface by accessing the raw data diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc index 26c96546e..b7f634b0e 100644 --- a/rtgui/thumbnail.cc +++ b/rtgui/thumbnail.cc @@ -37,6 +37,7 @@ #include "rtengine/metadata.h" #include "rtengine/profilestore.h" #include "rtengine/settings.h" +#include "rtengine/utils.h" #include "guiutils.h" #include "batchqueue.h" #include "extprog.h" @@ -1418,10 +1419,9 @@ void Thumbnail::updateProcParamsProperties(bool forceUpdate) pparamsValid = true; } - const auto getXmpSidecar = [this]() { - static auto xmp = rtengine::Exiv2Metadata::getXmpSidecar(fname); - return xmp; - }; + const rtengine::MemoizingSupplier getXmpSidecar([this]() { + return rtengine::Exiv2Metadata::getXmpSidecar(fname); + }); // save procparams rank and color also when options.thumbnailRankColorMode == Options::ThumbnailPropertyMode::XMP // so they'll be kept in sync From 1885f5d4894281369eb3d24d8cf7c6d2887b83d1 Mon Sep 17 00:00:00 2001 From: Lawrence Lee <45837045+Lawrence37@users.noreply.github.com> Date: Mon, 20 Jan 2025 12:53:07 -0800 Subject: [PATCH 3/3] Improve proc params clear when XMP sync enabled Always check the rank and color label in the XMP if XMP sync is enabled when clearing the processing parameters. Allows the pp3 to be deleted whenever possible, even if the rank or color label has been edited. --- rtgui/thumbnail.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc index b7f634b0e..e7f117101 100644 --- a/rtgui/thumbnail.cc +++ b/rtgui/thumbnail.cc @@ -1430,8 +1430,10 @@ void Thumbnail::updateProcParamsProperties(bool forceUpdate) if ((properties.rank.edited || forceUpdate) && rtengine::LIM(properties.rank.value, 0, 5) != rtengine::LIM(pparams->rank, 0, 5)) { pparams->rank = properties.rank; - pparamsValid |= properties.rank.edited; - if (!pparamsValid && forceUpdate) { + if (!forceUpdate) { + pparamsValid |= properties.rank.edited; + } + else if (!pparamsValid && forceUpdate) { // When force-updating, the processing parameters' rank needs not be // used if the embedded rank is the same. int initial_rank = 0; @@ -1443,8 +1445,10 @@ void Thumbnail::updateProcParamsProperties(bool forceUpdate) if ((properties.color.edited || forceUpdate) && properties.color != pparams->colorlabel) { pparams->colorlabel = properties.color; - pparamsValid |= properties.color.edited; - if (!pparamsValid && forceUpdate) { + if (!forceUpdate) { + pparamsValid |= properties.color.edited; + } + else if (!pparamsValid && forceUpdate) { // When force-updating, the processing parameters' color label needs // not be used if the embedded color label is the same. int initial_color = 0;