From a633081dc4e99fbfea82ff98bcd396e928e51b9d Mon Sep 17 00:00:00 2001 From: Zhou Ying Date: Tue, 18 Aug 2026 17:30:02 +0800 Subject: [PATCH 1/5] feat(rating): add RatingStore and header-scan rating readers (#201) First stage of star ratings: the isolated reading core, with no UI and no writes yet. Ratings stay off the decode pipeline as required -- RatingStore owns its own cache, queue and worker thread, and nothing here is reachable from CImageLoader::ReadMetadata or the thumbnail path. A read costs one 128 KB header scan for a JPEG/TIFF, or 16 KB of a .xmp sidecar for a RAW; never SHGetPropertyStore, never a decode. Parsing lives in RatingMetadata as pure functions over a byte span, which keeps it free of WIC and Win32 and therefore directly unit-testable: the test binary links it without the imaging stack. - JPEG: Exif IFD0 tag 0x4746 (SimpleRating) first, then the XMP packet's xmp:Rating. The XMP fallback matters -- Lightroom rates a JPEG by writing only xmp:Rating, so an Exif-only scan would report those files unrated. - TIFF: the same IFD scan, entered directly since a TIFF carries no APP1. - RAW: xmp:Rating from the same-name sidecar. - Adobe's rejected mark (xmp:Rating="-1") is recognized rather than mistaken for a star count. - Pair resolution follows the ruling on #201: the sidecar wins, and a disagreement is recorded together with the losing value so the full EXIF panel can show it later. 24 unit tests cover both serializations of xmp:Rating, both Exif byte orders, Exif-over-XMP precedence, the XMP fallback, out-of-range and malformed values, pair resolution, and a pass over every prefix of a synthetic JPEG so that a truncated header read cannot read out of bounds. The test images are assembled in memory: a test that depends on a file outside the repository cannot pass on a fresh clone. All file access goes through the wide-char API so that non-ASCII paths open correctly. --- CMakeLists.txt | 4 + QuickView/RatingMetadata.cpp | 214 +++++++++++++++++++++++++++++ QuickView/RatingMetadata.h | 78 +++++++++++ QuickView/RatingStore.cpp | 182 ++++++++++++++++++++++++ QuickView/RatingStore.h | 98 +++++++++++++ tests/RatingMetadataTests.cpp | 252 ++++++++++++++++++++++++++++++++++ 6 files changed, 828 insertions(+) create mode 100644 QuickView/RatingMetadata.cpp create mode 100644 QuickView/RatingMetadata.h create mode 100644 QuickView/RatingStore.cpp create mode 100644 QuickView/RatingStore.h create mode 100644 tests/RatingMetadataTests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 7aad4909..4720a1b2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -78,6 +78,8 @@ set(QUICKVIEW_SOURCES QuickView/GeekIconRenderer.cpp QuickView/GeekIconData.cpp QuickView/exif.cpp + QuickView/RatingMetadata.cpp + QuickView/RatingStore.cpp QuickView/RenderEngine.cpp QuickView/ImageLoader.cpp QuickView/MiniTiff.cpp @@ -392,6 +394,7 @@ add_executable(QuickViewTests tests/SvgNeedsFallbackTests.cpp tests/MetafileCodecTests.cpp tests/GeekWidgetsTests.cpp + tests/RatingMetadataTests.cpp QuickView/GeekWidgets.cpp QuickView/GeekIconLibrary.cpp QuickView/GeekIconData.cpp @@ -406,6 +409,7 @@ add_executable(QuickViewTests QuickView/FileNavigator.cpp QuickView/ArchiveVFS.cpp QuickView/exif.cpp + QuickView/RatingMetadata.cpp QuickView/QuickViewETW.cpp QuickView/pch.cpp ) diff --git a/QuickView/RatingMetadata.cpp b/QuickView/RatingMetadata.cpp new file mode 100644 index 00000000..1e95b949 --- /dev/null +++ b/QuickView/RatingMetadata.cpp @@ -0,0 +1,214 @@ +/* + * QuickView Star Ratings - metadata parsing (pure, no WIC / no Win32) + * Copyright (C) 2026-Present QuickView Contributors + * + * 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 "RatingMetadata.h" + +#include +#include + +namespace QuickView::Rating { + +namespace { + +// --- byte readers (every one bounds-checked: this parses untrusted files) --- + +uint16_t ReadU16(std::span b, size_t off, bool bigEndian) { + const uint16_t hi = b[off], lo = b[off + 1]; + return bigEndian ? (uint16_t)((hi << 8) | lo) : (uint16_t)((lo << 8) | hi); +} + +uint32_t ReadU32(std::span b, size_t off, bool bigEndian) { + const uint32_t b0 = b[off], b1 = b[off + 1], b2 = b[off + 2], b3 = b[off + 3]; + return bigEndian ? (b0 << 24) | (b1 << 16) | (b2 << 8) | b3 + : (b3 << 24) | (b2 << 16) | (b1 << 8) | b0; +} + +bool StartsWith(std::span b, size_t off, const char* literal, size_t len) { + if (off + len > b.size()) return false; + return std::memcmp(b.data() + off, literal, len) == 0; +} + +// TIFF/Exif IFD0 scan for tag 0x4746 (SimpleRating). `tiff` starts at the TIFF +// header ("II"/"MM"), which is how Exif nests inside APP1. +std::optional ParseExifRating(std::span tiff) { + constexpr uint16_t TAG_SIMPLE_RATING = 0x4746; // 18246 + + if (tiff.size() < 8) return std::nullopt; + + bool bigEndian; + if (tiff[0] == 'I' && tiff[1] == 'I') bigEndian = false; + else if (tiff[0] == 'M' && tiff[1] == 'M') bigEndian = true; + else return std::nullopt; + + if (ReadU16(tiff, 2, bigEndian) != 42) return std::nullopt; + + const uint32_t ifdOffset = ReadU32(tiff, 4, bigEndian); + if (ifdOffset < 8 || (size_t)ifdOffset + 2 > tiff.size()) return std::nullopt; + + const uint16_t entryCount = ReadU16(tiff, ifdOffset, bigEndian); + // 12 bytes per entry; reject a count that cannot fit in the buffer. + if (entryCount == 0 || (size_t)ifdOffset + 2 + (size_t)entryCount * 12 > tiff.size()) { + return std::nullopt; + } + + for (uint16_t i = 0; i < entryCount; ++i) { + const size_t entry = (size_t)ifdOffset + 2 + (size_t)i * 12; + if (ReadU16(tiff, entry, bigEndian) != TAG_SIMPLE_RATING) continue; + + const uint16_t type = ReadU16(tiff, entry + 2, bigEndian); + const uint32_t count = ReadU32(tiff, entry + 4, bigEndian); + if (count != 1) return std::nullopt; + + // SHORT is what the policy specifies; BYTE and LONG are accepted + // defensively. All three fit inline in the value field. + int value; + switch (type) { + case 1: value = tiff[entry + 8]; break; // BYTE + case 3: value = ReadU16(tiff, entry + 8, bigEndian); break; // SHORT + case 4: value = (int)ReadU32(tiff, entry + 8, bigEndian); break; // LONG + default: return std::nullopt; + } + return IsValidRating(value) ? std::optional(value) : std::nullopt; + } + return std::nullopt; +} + +} // namespace + +std::optional ParseXmpRating(std::string_view xmp) { + constexpr std::string_view PROPERTY = "xmp:Rating"; + + for (size_t pos = xmp.find(PROPERTY); pos != std::string_view::npos; + pos = xmp.find(PROPERTY, pos + PROPERTY.size())) { + size_t cursor = pos + PROPERTY.size(); + + // Attribute form: xmp:Rating="3" (spaces tolerated around the =) + // Element form: 3 + while (cursor < xmp.size() && (xmp[cursor] == ' ' || xmp[cursor] == '\t')) ++cursor; + if (cursor >= xmp.size()) break; + + if (xmp[cursor] == '=') { + ++cursor; + while (cursor < xmp.size() && (xmp[cursor] == ' ' || xmp[cursor] == '\t')) ++cursor; + const char quote = (cursor < xmp.size()) ? xmp[cursor] : '\0'; + if (quote != '"' && quote != '\'') continue; + ++cursor; + } else if (xmp[cursor] == '>') { + ++cursor; + } else { + continue; // a longer property name that merely starts with xmp:Rating + } + + while (cursor < xmp.size() && (xmp[cursor] == ' ' || xmp[cursor] == '\t' || + xmp[cursor] == '\r' || xmp[cursor] == '\n')) { + ++cursor; + } + + size_t end = cursor; + if (end < xmp.size() && xmp[end] == '-') ++end; // rejected: -1 + while (end < xmp.size() && xmp[end] >= '0' && xmp[end] <= '9') ++end; + if (end == cursor) continue; // no digits + + int value = 0; + const auto result = std::from_chars(xmp.data() + cursor, xmp.data() + end, value); + if (result.ec != std::errc{} || result.ptr != xmp.data() + end) continue; + // A fractional rating (xmp:Rating="3.5") is legal XMP; the integer part + // is what a 0-5 star UI can represent, so it is taken as-is. + if (IsValidRating(value)) return value; + } + return std::nullopt; +} + +std::optional ParseTiffRating(std::span bytes) { + return ParseExifRating(bytes); +} + +std::optional ParseJpegRating(std::span bytes) { + constexpr size_t EXIF_ID_LEN = 6; // "Exif\0\0" + constexpr size_t XMP_ID_LEN = 29; // "http://ns.adobe.com/xap/1.0/\0" + + if (bytes.size() < 4 || bytes[0] != 0xFF || bytes[1] != 0xD8) { + return std::nullopt; // not a JPEG (no SOI) + } + + std::optional fromXmp; + size_t pos = 2; + + while (pos + 4 <= bytes.size()) { + if (bytes[pos] != 0xFF) return fromXmp; // desynchronized: stop, stay safe + const uint8_t marker = bytes[pos + 1]; + + // Fill bytes between segments are legal. + if (marker == 0xFF) { ++pos; continue; } + // SOS starts the entropy-coded scan and EOI ends the file; no metadata + // follows either, so there is nothing left worth reading. + if (marker == 0xDA || marker == 0xD9) break; + // Standalone markers carry no payload. + if (marker == 0x01 || (marker >= 0xD0 && marker <= 0xD7)) { pos += 2; continue; } + + const uint16_t segLength = ReadU16(bytes, pos + 2, /*bigEndian*/ true); + if (segLength < 2) return fromXmp; // malformed + const size_t payload = pos + 4; + const size_t payloadLen = (size_t)segLength - 2; + if (payload + payloadLen > bytes.size()) { + // Truncated by our header-sized read: what we already have is the + // best answer available without reading more of the file. + return fromXmp; + } + + if (marker == 0xE1) { // APP1: Exif or XMP + if (StartsWith(bytes, payload, "Exif\0\0", EXIF_ID_LEN)) { + if (auto exifRating = ParseExifRating( + bytes.subspan(payload + EXIF_ID_LEN, payloadLen - EXIF_ID_LEN))) { + return exifRating; // cheapest and most authoritative source + } + } else if (StartsWith(bytes, payload, "http://ns.adobe.com/xap/1.0/\0", XMP_ID_LEN)) { + if (!fromXmp) { + const char* text = + reinterpret_cast(bytes.data()) + payload + XMP_ID_LEN; + fromXmp = ParseXmpRating(std::string_view(text, payloadLen - XMP_ID_LEN)); + } + } + } + pos = payload + payloadLen; + } + return fromXmp; +} + +Resolved ResolvePairRating(std::optional inFile, std::optional sidecar) { + // Rejected (-1) is a deliberate mark, so it takes part in the resolution; + // it is only flattened to 0 stars for display. + auto toStars = [](int value) { return value < MIN_STARS ? MIN_STARS : value; }; + + Resolved out; + if (sidecar && inFile) { + out.stars = toStars(*sidecar); + out.source = Source::Sidecar; + out.conflict = (*sidecar != *inFile); + out.otherStars = toStars(*inFile); + } else if (sidecar) { + out.stars = toStars(*sidecar); + out.source = Source::Sidecar; + } else if (inFile) { + out.stars = toStars(*inFile); + out.source = Source::InFile; + } + return out; +} + +} // namespace QuickView::Rating diff --git a/QuickView/RatingMetadata.h b/QuickView/RatingMetadata.h new file mode 100644 index 00000000..38a01ae9 --- /dev/null +++ b/QuickView/RatingMetadata.h @@ -0,0 +1,78 @@ +/* + * QuickView Star Ratings - metadata parsing (pure, no WIC / no Win32) + * Copyright (C) 2026-Present QuickView Contributors + * + * 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 . + */ + +#pragma once + +#include +#include +#include +#include + +// Reading ratings must never touch the decode pipeline, so these parsers work +// on a plain byte prefix of a file (a header read) and pull in nothing but the +// standard library -- which also makes them directly unit-testable. +namespace QuickView::Rating { + +inline constexpr int MIN_STARS = 0; +inline constexpr int MAX_STARS = 5; +// Adobe writes xmp:Rating="-1" to mark a photo rejected. QuickView never +// authors that value, but it must be recognized instead of being mistaken for +// a star count. +inline constexpr int REJECTED = -1; + +// True for a value we are willing to surface (-1 rejected, or 0..5 stars). +constexpr bool IsValidRating(int value) { + return value == REJECTED || (value >= MIN_STARS && value <= MAX_STARS); +} + +// Parse an XMP document (a .xmp sidecar, or the XMP packet embedded in a file) +// for xmp:Rating. Both serializations are accepted: +// xmp:Rating="3" (attribute form, what Lightroom writes) +// 3 (element form) +// Returns nothing when the property is absent or malformed. +std::optional ParseXmpRating(std::string_view xmp); + +// Scan the head of a JPEG for a rating, cheapest source first: +// 1. APP1 Exif -> IFD0 tag 0x4746 (SimpleRating, what Explorer writes) +// 2. APP1 XMP -> xmp:Rating (what Lightroom writes; Explorer may not +// have written the Exif tag at all) +// `bytes` may be a prefix of the file; scanning stops at SOS, since no +// metadata follows the compressed scan. Returns nothing when absent. +std::optional ParseJpegRating(std::span bytes); + +// Scan the head of a bare TIFF for a rating. A TIFF stream carries its IFD +// directly (no APP1 wrapper), so it needs its own entry point. +std::optional ParseTiffRating(std::span bytes); + +// Which file of a pair a displayed rating came from. +enum class Source { None, InFile, Sidecar }; + +struct Resolved { + int stars = 0; // 0..5 (a rejected -1 is surfaced as 0 stars) + Source source = Source::None; + bool conflict = false; // both sides carry a rating and they differ + int otherStars = 0; // the losing side's value, for the EXIF panel +}; + +// Merge the two carriers of a folded RAW+rendered pair into the one rating the +// UI shows. Per the maintainer's ruling the sidecar always wins: a JPEG's +// mtime is churned by lossless rotation, cloud sync and Explorer edits, while +// the sidecar is the only reliable carrier of photographer intent (LR/C1). +Resolved ResolvePairRating(std::optional inFile, std::optional sidecar); + +} // namespace QuickView::Rating diff --git a/QuickView/RatingStore.cpp b/QuickView/RatingStore.cpp new file mode 100644 index 00000000..6995eff5 --- /dev/null +++ b/QuickView/RatingStore.cpp @@ -0,0 +1,182 @@ +/* + * QuickView Star Ratings - isolated rating cache and background reader + * Copyright (C) 2026-Present QuickView Contributors + * + * 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 "RatingStore.h" + +#include "SupportedExtensions.h" + +#include + +namespace { + +// A rating sits in the file header, so a small prefix is all that is ever +// read -- the whole point is to stay off the decode pipeline. +constexpr DWORD HEADER_READ_BYTES = 128 * 1024; +constexpr DWORD SIDECAR_READ_BYTES = 16 * 1024; + +// Wide-char file API throughout: a path round-tripped through a narrow code +// page fails to open on non-ASCII names. +std::vector ReadFilePrefix(const std::wstring& path, DWORD maxBytes) { + std::vector buffer; + if (path.empty()) return buffer; + + HANDLE file = CreateFileW(path.c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN, nullptr); + if (file == INVALID_HANDLE_VALUE) return buffer; + + LARGE_INTEGER size{}; + if (GetFileSizeEx(file, &size) && size.QuadPart > 0) { + const DWORD toRead = (size.QuadPart < (LONGLONG)maxBytes) + ? (DWORD)size.QuadPart + : maxBytes; + buffer.resize(toRead); + DWORD read = 0; + if (!ReadFile(file, buffer.data(), toRead, &read, nullptr)) { + buffer.clear(); + } else { + buffer.resize(read); + } + } + CloseHandle(file); + return buffer; +} + +} // namespace + +RatingStore::~RatingStore() { + Shutdown(); +} + +void RatingStore::Initialize(HWND hwnd) { + if (m_running.load()) return; + m_hwnd = hwnd; + m_running = true; + m_worker = std::thread(&RatingStore::WorkerLoop, this); +} + +void RatingStore::Shutdown() { + if (!m_running.exchange(false)) return; + { + std::lock_guard lock(m_queueMutex); + m_queue.clear(); + m_pending.clear(); + } + m_cv.notify_all(); + if (m_worker.joinable()) m_worker.join(); +} + +std::wstring RatingStore::SidecarPathFor(const std::wstring& path) { + const std::wstring_view ext = QuickView::ExtensionOf(path); + if (ext.empty()) return std::wstring(); + return path.substr(0, path.size() - ext.size()) + L".xmp"; +} + +std::optional RatingStore::ReadRatingFromSidecar(const std::wstring& sidecarPath) { + const std::vector bytes = ReadFilePrefix(sidecarPath, SIDECAR_READ_BYTES); + if (bytes.empty()) return std::nullopt; + return QuickView::Rating::ParseXmpRating( + std::string_view(reinterpret_cast(bytes.data()), bytes.size())); +} + +std::optional RatingStore::ReadRatingFromFile(const std::wstring& path) { + const std::wstring_view ext = QuickView::ExtensionOf(path); + + // Only formats that actually carry an in-file rating are opened at all. + const bool isJpeg = QuickView::ExtEqualsIgnoreCase(ext, L".jpg") || + QuickView::ExtEqualsIgnoreCase(ext, L".jpeg"); + const bool isTiff = QuickView::ExtEqualsIgnoreCase(ext, L".tif") || + QuickView::ExtEqualsIgnoreCase(ext, L".tiff"); + if (!isJpeg && !isTiff) return std::nullopt; + + const std::vector bytes = ReadFilePrefix(path, HEADER_READ_BYTES); + if (bytes.empty()) return std::nullopt; + + return isJpeg ? QuickView::Rating::ParseJpegRating(bytes) + : QuickView::Rating::ParseTiffRating(bytes); +} + +std::optional RatingStore::TryGet(ImageID id) const { + std::lock_guard lock(m_cacheMutex); + auto it = m_cache.find(id); + if (it == m_cache.end()) return std::nullopt; + return it->second; +} + +void RatingStore::QueueRead(ImageID id, const std::wstring& renderedPath, + const std::wstring& rawPath) { + if (!m_running.load() || renderedPath.empty()) return; + { + std::lock_guard cacheLock(m_cacheMutex); + if (m_cache.find(id) != m_cache.end()) return; // already known + } + { + std::lock_guard lock(m_queueMutex); + if (!m_pending.insert(id).second) return; // already queued + m_queue.push_back(Task{ id, renderedPath, rawPath, m_generation.load() }); + } + m_cv.notify_one(); +} + +void RatingStore::Clear() { + ++m_generation; // invalidates results still in flight + { + std::lock_guard lock(m_queueMutex); + m_queue.clear(); + m_pending.clear(); + } + std::lock_guard cacheLock(m_cacheMutex); + m_cache.clear(); +} + +void RatingStore::WorkerLoop() { + while (true) { + Task task; + { + std::unique_lock lock(m_queueMutex); + m_cv.wait(lock, [this] { return !m_running.load() || !m_queue.empty(); }); + if (!m_running.load()) return; + task = std::move(m_queue.front()); + m_queue.pop_front(); + } + + // A rating can live in the file itself (JPEG/TIFF) and in the sidecar + // of the paired RAW. A standalone RAW has no in-file rating, so only + // its own sidecar is probed. + const std::optional inFile = ReadRatingFromFile(task.renderedPath); + const std::wstring sidecarOwner = task.rawPath.empty() ? task.renderedPath : task.rawPath; + const std::optional sidecar = ReadRatingFromSidecar(SidecarPathFor(sidecarOwner)); + + const auto resolved = QuickView::Rating::ResolvePairRating(inFile, sidecar); + + if (task.generation != m_generation.load()) continue; // folder moved on + + { + std::lock_guard cacheLock(m_cacheMutex); + m_cache[task.id] = resolved; + } + { + std::lock_guard lock(m_queueMutex); + m_pending.erase(task.id); + } + if (m_hwnd) { + PostMessageW(m_hwnd, WM_RATING_READY, (WPARAM)task.id, 0); + } + } +} diff --git a/QuickView/RatingStore.h b/QuickView/RatingStore.h new file mode 100644 index 00000000..2d7e2ce4 --- /dev/null +++ b/QuickView/RatingStore.h @@ -0,0 +1,98 @@ +/* + * QuickView Star Ratings - isolated rating cache and background reader + * Copyright (C) 2026-Present QuickView Contributors + * + * 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 . + */ + +#pragma once + +#include "pch.h" +#include "FileNavigator.h" // ImageID +#include "RatingMetadata.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// Posted (wParam = ImageID) once a background read has filled the cache, so +// the UI can repaint just the affected item. +#define WM_RATING_READY (WM_APP + 103) + +// Ratings live entirely outside the decode pipeline: nothing here may be +// called synchronously from CImageLoader::ReadMetadata or the thumbnail path. +// The UI asks the cache (TryGet, never touches disk) and queues a background +// read when the answer is not there yet. +class RatingStore { +public: + RatingStore() = default; + ~RatingStore(); + RatingStore(const RatingStore&) = delete; + RatingStore& operator=(const RatingStore&) = delete; + + void Initialize(HWND hwnd); + void Shutdown(); + + // Cache-only lookup: no disk access, safe on the UI thread and on any hot + // path. Returns nothing when this item has not been read yet. + std::optional TryGet(ImageID id) const; + + // Queue a background read. `rawPath` is the hidden RAW of a folded pair + // (empty when the item is not paired); a standalone RAW is passed as + // `renderedPath` and resolves through its own sidecar. + void QueueRead(ImageID id, const std::wstring& renderedPath, + const std::wstring& rawPath = std::wstring()); + + // Drop everything and cancel in-flight work (folder changed). + void Clear(); + + // The .xmp sidecar a RAW's rating lives in, i.e. the path with its + // extension replaced. Empty when `path` has no extension. + static std::wstring SidecarPathFor(const std::wstring& path); + + // Read one file's rating straight from disk, no cache. Exposed for the + // writer (which must re-read to confirm) and for tests of the disk path. + static std::optional ReadRatingFromFile(const std::wstring& path); + static std::optional ReadRatingFromSidecar(const std::wstring& sidecarPath); + +private: + struct Task { + ImageID id = 0; + std::wstring renderedPath; + std::wstring rawPath; + uint64_t generation = 0; + }; + + void WorkerLoop(); + + HWND m_hwnd = nullptr; + + mutable std::mutex m_cacheMutex; + std::unordered_map m_cache; + + std::mutex m_queueMutex; + std::condition_variable m_cv; + std::deque m_queue; + std::unordered_set m_pending; + + std::thread m_worker; + std::atomic m_running{ false }; + std::atomic m_generation{ 0 }; +}; diff --git a/tests/RatingMetadataTests.cpp b/tests/RatingMetadataTests.cpp new file mode 100644 index 00000000..316d06a1 --- /dev/null +++ b/tests/RatingMetadataTests.cpp @@ -0,0 +1,252 @@ +#include +#include "RatingMetadata.h" + +#include +#include +#include + +using namespace QuickView::Rating; + +namespace { + +// Test images are assembled in memory on purpose: a unit test that depends on +// a file outside the repository cannot pass on a fresh clone. + +void AppendU16BE(std::vector& out, uint16_t value) { + out.push_back((uint8_t)(value >> 8)); + out.push_back((uint8_t)(value & 0xFF)); +} + +void AppendBytes(std::vector& out, const char* data, size_t len) { + for (size_t i = 0; i < len; ++i) out.push_back((uint8_t)data[i]); +} + +// Minimal TIFF block holding IFD0 with a single tag. +std::vector MakeTiff(uint16_t tag, uint16_t type, uint32_t value, bool bigEndian) { + std::vector t; + auto u16 = [&](uint16_t v) { + if (bigEndian) { t.push_back((uint8_t)(v >> 8)); t.push_back((uint8_t)v); } + else { t.push_back((uint8_t)v); t.push_back((uint8_t)(v >> 8)); } + }; + auto u32 = [&](uint32_t v) { + if (bigEndian) { + t.push_back((uint8_t)(v >> 24)); t.push_back((uint8_t)(v >> 16)); + t.push_back((uint8_t)(v >> 8)); t.push_back((uint8_t)v); + } else { + t.push_back((uint8_t)v); t.push_back((uint8_t)(v >> 8)); + t.push_back((uint8_t)(v >> 16)); t.push_back((uint8_t)(v >> 24)); + } + }; + + t.push_back(bigEndian ? 'M' : 'I'); + t.push_back(bigEndian ? 'M' : 'I'); + u16(42); + u32(8); // IFD0 begins right after the header + u16(1); // one entry + u16(tag); + u16(type); + u32(1); // count + // TIFF stores a value shorter than 4 bytes left-justified in the value + // field, so a SHORT occupies the first two bytes (not a padded u32). + switch (type) { + case 1: t.push_back((uint8_t)value); t.push_back(0); u16(0); break; // BYTE + case 3: u16((uint16_t)value); u16(0); break; // SHORT + default: u32(value); break; // LONG + } + u32(0); // no next IFD + return t; +} + +std::vector MakeJpeg(const std::vector>& app1Payloads) { + std::vector jpeg{ 0xFF, 0xD8 }; // SOI + for (const auto& payload : app1Payloads) { + jpeg.push_back(0xFF); + jpeg.push_back(0xE1); // APP1 + AppendU16BE(jpeg, (uint16_t)(payload.size() + 2)); + jpeg.insert(jpeg.end(), payload.begin(), payload.end()); + } + jpeg.push_back(0xFF); + jpeg.push_back(0xDA); // SOS - scan data would follow + return jpeg; +} + +std::vector ExifPayload(uint16_t tag, uint16_t type, uint32_t value, bool bigEndian) { + std::vector payload; + AppendBytes(payload, "Exif\0\0", 6); + const std::vector tiff = MakeTiff(tag, type, value, bigEndian); + payload.insert(payload.end(), tiff.begin(), tiff.end()); + return payload; +} + +std::vector XmpPayload(const std::string& xmpBody) { + std::vector payload; + AppendBytes(payload, "http://ns.adobe.com/xap/1.0/\0", 29); + AppendBytes(payload, xmpBody.c_str(), xmpBody.size()); + return payload; +} + +constexpr uint16_t TAG_SIMPLE_RATING = 0x4746; +constexpr uint16_t TYPE_SHORT = 3; + +} // namespace + +// --- ParseXmpRating ------------------------------------------------------- + +TEST(RatingMetadataTest, XmpAttributeForm) { + EXPECT_EQ(ParseXmpRating(R"()"), 4); +} + +TEST(RatingMetadataTest, XmpElementForm) { + EXPECT_EQ(ParseXmpRating("2"), 2); +} + +TEST(RatingMetadataTest, XmpSingleQuotesAndSpacing) { + EXPECT_EQ(ParseXmpRating("xmp:Rating = '5'"), 5); +} + +TEST(RatingMetadataTest, XmpElementFormWithWhitespace) { + EXPECT_EQ(ParseXmpRating("\n 3\n"), 3); +} + +TEST(RatingMetadataTest, XmpRejectedValueIsRecognized) { + // Adobe marks a rejected photo with -1; it must not read as a star count. + EXPECT_EQ(ParseXmpRating(R"(xmp:Rating="-1")"), REJECTED); +} + +TEST(RatingMetadataTest, XmpZeroMeansUnrated) { + EXPECT_EQ(ParseXmpRating(R"(xmp:Rating="0")"), 0); +} + +TEST(RatingMetadataTest, XmpOutOfRangeIsIgnored) { + EXPECT_FALSE(ParseXmpRating(R"(xmp:Rating="99")").has_value()); + EXPECT_FALSE(ParseXmpRating(R"(xmp:Rating="-7")").has_value()); +} + +TEST(RatingMetadataTest, XmpAbsentOrMalformed) { + EXPECT_FALSE(ParseXmpRating("").has_value()); + EXPECT_FALSE(ParseXmpRating("").has_value()); + EXPECT_FALSE(ParseXmpRating("xmp:Rating=\"\"").has_value()); + EXPECT_FALSE(ParseXmpRating("xmp:Rating").has_value()); +} + +TEST(RatingMetadataTest, XmpLongerPropertyNameIsNotMistaken) { + // MicrosoftPhoto:Rating is a 0-99 percent value living under another + // namespace; a name that merely starts with xmp:Rating must not match. + EXPECT_FALSE(ParseXmpRating(R"(xmp:RatingPercent="75")").has_value()); +} + +TEST(RatingMetadataTest, XmpSkipsMalformedOccurrenceAndTakesTheNext) { + EXPECT_EQ(ParseXmpRating(R"(xmp:Rating="" ... xmp:Rating="3")"), 3); +} + +// --- ParseJpegRating ------------------------------------------------------ + +TEST(RatingMetadataTest, JpegExifRatingLittleEndian) { + const auto jpeg = MakeJpeg({ ExifPayload(TAG_SIMPLE_RATING, TYPE_SHORT, 4, false) }); + EXPECT_EQ(ParseJpegRating(jpeg), 4); +} + +TEST(RatingMetadataTest, JpegExifRatingBigEndian) { + const auto jpeg = MakeJpeg({ ExifPayload(TAG_SIMPLE_RATING, TYPE_SHORT, 5, true) }); + EXPECT_EQ(ParseJpegRating(jpeg), 5); +} + +TEST(RatingMetadataTest, JpegXmpRatingWhenExifTagAbsent) { + // Lightroom writes only xmp:Rating, so an Exif-only scan would miss it. + const auto jpeg = MakeJpeg({ XmpPayload(R"()") }); + EXPECT_EQ(ParseJpegRating(jpeg), 3); +} + +TEST(RatingMetadataTest, JpegExifWinsOverXmp) { + const auto jpeg = MakeJpeg({ + ExifPayload(TAG_SIMPLE_RATING, TYPE_SHORT, 4, false), + XmpPayload(R"(xmp:Rating="1")"), + }); + EXPECT_EQ(ParseJpegRating(jpeg), 4); +} + +TEST(RatingMetadataTest, JpegXmpFallbackWhenExifSegmentHasNoRating) { + // An Exif block without the rating tag must not stop the XMP fallback. + const auto jpeg = MakeJpeg({ + ExifPayload(0x010F /*Make*/, TYPE_SHORT, 1, false), + XmpPayload(R"(xmp:Rating="2")"), + }); + EXPECT_EQ(ParseJpegRating(jpeg), 2); +} + +TEST(RatingMetadataTest, JpegWithoutRating) { + const auto jpeg = MakeJpeg({}); + EXPECT_FALSE(ParseJpegRating(jpeg).has_value()); +} + +TEST(RatingMetadataTest, NonJpegInputIsRejected) { + const std::vector png{ 0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A }; + EXPECT_FALSE(ParseJpegRating(png).has_value()); + EXPECT_FALSE(ParseJpegRating(std::vector{}).has_value()); +} + +TEST(RatingMetadataTest, TruncatedAndMalformedInputDoesNotCrash) { + // A header-sized read can cut a segment in half; every prefix must be safe. + const auto full = MakeJpeg({ ExifPayload(TAG_SIMPLE_RATING, TYPE_SHORT, 4, false) }); + for (size_t len = 0; len < full.size(); ++len) { + std::vector prefix(full.begin(), full.begin() + len); + (void)ParseJpegRating(prefix); // must not crash or read out of bounds + } + + std::vector garbage{ 0xFF, 0xD8, 0xFF, 0xE1, 0x00, 0x01, 0xFF, 0xFF, 0x00 }; + (void)ParseJpegRating(garbage); + + // Segment length pointing far past the buffer. + std::vector overrun{ 0xFF, 0xD8, 0xFF, 0xE1, 0x7F, 0xFF, 'E', 'x' }; + EXPECT_FALSE(ParseJpegRating(overrun).has_value()); +} + +TEST(RatingMetadataTest, JpegExifRatingOutOfRangeIsIgnored) { + const auto jpeg = MakeJpeg({ ExifPayload(TAG_SIMPLE_RATING, TYPE_SHORT, 42, false) }); + EXPECT_FALSE(ParseJpegRating(jpeg).has_value()); +} + +// --- ResolvePairRating ---------------------------------------------------- + +TEST(RatingMetadataTest, PairSidecarWinsOnConflict) { + // The maintainer's ruling: the sidecar is the authoritative carrier. + const Resolved r = ResolvePairRating(/*inFile*/ 2, /*sidecar*/ 4); + EXPECT_EQ(r.stars, 4); + EXPECT_EQ(r.source, Source::Sidecar); + EXPECT_TRUE(r.conflict); + EXPECT_EQ(r.otherStars, 2); +} + +TEST(RatingMetadataTest, PairAgreeingSidesAreNotAConflict) { + const Resolved r = ResolvePairRating(3, 3); + EXPECT_EQ(r.stars, 3); + EXPECT_EQ(r.source, Source::Sidecar); + EXPECT_FALSE(r.conflict); +} + +TEST(RatingMetadataTest, PairSingleSidedRatings) { + const Resolved onlySidecar = ResolvePairRating(std::nullopt, 5); + EXPECT_EQ(onlySidecar.stars, 5); + EXPECT_EQ(onlySidecar.source, Source::Sidecar); + EXPECT_FALSE(onlySidecar.conflict); + + const Resolved onlyInFile = ResolvePairRating(1, std::nullopt); + EXPECT_EQ(onlyInFile.stars, 1); + EXPECT_EQ(onlyInFile.source, Source::InFile); + EXPECT_FALSE(onlyInFile.conflict); +} + +TEST(RatingMetadataTest, PairWithNoRatingAnywhere) { + const Resolved r = ResolvePairRating(std::nullopt, std::nullopt); + EXPECT_EQ(r.stars, 0); + EXPECT_EQ(r.source, Source::None); + EXPECT_FALSE(r.conflict); +} + +TEST(RatingMetadataTest, PairRejectedIsDisplayedAsZeroStarsButStillConflicts) { + const Resolved r = ResolvePairRating(/*inFile*/ 4, /*sidecar*/ REJECTED); + EXPECT_EQ(r.stars, 0); + EXPECT_EQ(r.source, Source::Sidecar); + EXPECT_TRUE(r.conflict); + EXPECT_EQ(r.otherStars, 4); +} From a56bb17431990b0fcfc8723df4972fbbc51784cf Mon Sep 17 00:00:00 2001 From: Zhou Ying Date: Mon, 24 Aug 2026 16:04:34 +0800 Subject: [PATCH 2/5] feat(rating): show the star rating in the full info panel (#201) Second stage of star ratings: reading is wired up and the rating is shown, still with no way to change it. RatingStore is initialized with the window, queued from StartNavigation (a folded pair is resolved so either face reports the same stars), and its WM_RATING_READY reply repaints. The full info panel gains a star row, showing an unrated photo as empty stars so that enabling the item does not make the row come and go from photo to photo. A disagreement between the two files of a pair is spelled out here and only here -- "(sidecar; JPG has 2)" -- since the gallery and the viewport must stay quiet while culling. Four things were needed to make an asynchronous value survive this UI: - The panel caches its rows behind a state hash, so the rating takes part in that hash; otherwise a rating arriving after the first build would never be picked up. - The panel is drawn on the static layer, so WM_RATING_READY repaints static as well as dynamic. - A read queued before the store is initialized is kept rather than dropped, since the first photo can be navigated to that early. - The directory watcher fires right after a folder is opened, and the cache invalidation it triggers used to discard the read for the photo on screen; the current photo is now queued again immediately. The panel's item list is a whitelist that lives in the ini, so on an existing installation the new row would have been filtered out forever. A one-shot migration adds the item once and records that it did, which leaves a later removal by the user alone. --- QuickView/EditState.h | 4 +-- QuickView/RatingStore.cpp | 5 ++- QuickView/SettingsOverlay.cpp | 4 +-- QuickView/UIRenderer.cpp | 33 +++++++++++++++++ QuickView/main.cpp | 68 ++++++++++++++++++++++++++++++++--- 5 files changed, 105 insertions(+), 9 deletions(-) diff --git a/QuickView/EditState.h b/QuickView/EditState.h index 63a90acb..84697f3f 100644 --- a/QuickView/EditState.h +++ b/QuickView/EditState.h @@ -728,8 +728,8 @@ struct AppConfig { // --- Customizable Info Panel Lite --- std::wstring InfoPanelLiteItemsNormal = L"Zoom,Progress,File,Size,Disk,Format"; std::wstring InfoPanelLiteItemsCompare = L"File,Size,Disk,Sharp,Ent,BPP,Date"; - std::wstring InfoPanelFullItemsNormal = L"Histogram,File,Position,RAW,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,GPS"; - std::wstring InfoPanelFullItemsCompare = L"Histogram,File,RAW,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,Sharp,Ent,BPP,GPS"; + std::wstring InfoPanelFullItemsNormal = L"Histogram,File,Position,RAW,Rating,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,GPS"; + std::wstring InfoPanelFullItemsCompare = L"Histogram,File,RAW,Rating,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,Sharp,Ent,BPP,GPS"; int InfoPanelScale = 0; // 0=Global, 1=100%, 2=125%, 3=150%, 4=175%, 5=200% std::wstring InfoPanelLiteSeparator = L" \u00b7 "; diff --git a/QuickView/RatingStore.cpp b/QuickView/RatingStore.cpp index 6995eff5..64a125c3 100644 --- a/QuickView/RatingStore.cpp +++ b/QuickView/RatingStore.cpp @@ -121,7 +121,10 @@ std::optional RatingStore::TryGet(ImageID id) const void RatingStore::QueueRead(ImageID id, const std::wstring& renderedPath, const std::wstring& rawPath) { - if (!m_running.load() || renderedPath.empty()) return; + // Deliberately not gated on m_running: the first image can be navigated to + // before Initialize runs, and such a request must still be honoured once + // the worker starts rather than being dropped. + if (renderedPath.empty()) return; { std::lock_guard cacheLock(m_cacheMutex); if (m_cache.find(id) != m_cache.end()) return; // already known diff --git a/QuickView/SettingsOverlay.cpp b/QuickView/SettingsOverlay.cpp index cc057436..de12343f 100644 --- a/QuickView/SettingsOverlay.cpp +++ b/QuickView/SettingsOverlay.cpp @@ -1807,7 +1807,7 @@ void SettingsOverlay::BuildMenu() { tagCloudFullNormal.label = AppStrings::Settings_Label_ItemsInNormalMode; tagCloudFullNormal.type = OptionType::TagCloud; tagCloudFullNormal.pStrVal = &g_config.InfoPanelFullItemsNormal; - tagCloudFullNormal.options = { L"Histogram", L"Position", L"File", L"RAW", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"GPS" }; + tagCloudFullNormal.options = { L"Histogram", L"Position", L"File", L"RAW", L"Rating", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"GPS" }; tagCloudFullNormal.tagCloudNoLimit = true; tagCloudFullNormal.tagCloudNoSort = true; tagCloudFullNormal.onChange = []([[maybe_unused]] SettingsOverlay* overlay, [[maybe_unused]] SettingsItem* item) { SaveConfig(); }; @@ -1817,7 +1817,7 @@ void SettingsOverlay::BuildMenu() { tagCloudFullCompare.label = AppStrings::Settings_Label_ItemsInCompareMode; tagCloudFullCompare.type = OptionType::TagCloud; tagCloudFullCompare.pStrVal = &g_config.InfoPanelFullItemsCompare; - tagCloudFullCompare.options = { L"Histogram", L"File", L"RAW", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program" }; + tagCloudFullCompare.options = { L"Histogram", L"File", L"RAW", L"Rating", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program" }; tagCloudFullCompare.tagCloudNoLimit = true; tagCloudFullCompare.tagCloudNoSort = true; tagCloudFullCompare.onChange = []([[maybe_unused]] SettingsOverlay* overlay, [[maybe_unused]] SettingsItem* item) { SaveConfig(); }; diff --git a/QuickView/UIRenderer.cpp b/QuickView/UIRenderer.cpp index 17cce161..ce461bd7 100644 --- a/QuickView/UIRenderer.cpp +++ b/QuickView/UIRenderer.cpp @@ -1,4 +1,5 @@ #include "UIRenderer.h" +#include "RatingStore.h" #include "StringUtils.h" #include "AppStrings.h" #include @@ -45,6 +46,7 @@ extern ImageEngine* g_pImageEngine; // [v3.1] Accessor (renamed from g_imageEngi #include "FileNavigator.h" extern FileNavigator& g_navigator; +extern RatingStore g_ratingStore; // Dialog rendering is handled by DialogController @@ -2928,6 +2930,27 @@ std::vector UIRenderer::BuildGridRows(const CImageLoader::ImageMetadata } } + // [Ratings] Star rating, read off the decode pipeline by RatingStore; the + // row only appears once that background read has landed. Icon: glowing + // star is taken by HDR, so the plain star (U+2B50) marks this row. + // Shown as soon as the read has landed, unrated included, so that enabling + // the item does not make the row appear and vanish from photo to photo. + if (const auto rating = g_ratingStore.TryGet(FileNavigator::PathToImageID(imagePath))) { + std::wstring stars; + for (int i = 0; i < QuickView::Rating::MAX_STARS; ++i) { + stars += (i < rating->stars) ? L"\u2605" : L"\u2606"; + } + // The hot path stays silent about a disagreement between the two files + // of a pair; the full panel is where it is spelled out. + std::wstring detail; + if (rating->conflict) { + detail = L"(sidecar; JPG has " + std::to_wstring(rating->otherStars) + L")"; + } + rows.push_back({L"\u2B50", L"Rating", stars, detail, + stars + (detail.empty() ? L"" : L" " + detail), + TruncateMode::None, false}); + } + // [RAW+JPEG Pairing] Hidden RAW sibling of this photo. Icon: link // (U+1F517) = "file paired with this photo"; the film-frames icon is // already taken by the Format row. @@ -3508,6 +3531,16 @@ void UIRenderer::BuildInfoGrid() { CombineHash(stateHash, g_currentMetadata.HasSharpness); CombineHash(stateHash, g_currentMetadata.HasEntropy); CombineHash(stateHash, hasHistR); + // [Ratings] The background read lands after the panel has already been + // built for this photo, so the rating has to take part in the hash -- + // otherwise the cached rows would never pick the star row up. + if (const auto rating = g_ratingStore.TryGet(FileNavigator::PathToImageID(g_imagePath))) { + CombineHash(stateHash, rating->stars); + CombineHash(stateHash, rating->conflict); + CombineHash(stateHash, (int)rating->source); + } else { + CombineHash(stateHash, -1); // not read yet + } const auto& editState = GetPaneContext(PaneSlot::Primary).editState; CombineHash(stateHash, editState.HasCrop); if (editState.HasCrop) { diff --git a/QuickView/main.cpp b/QuickView/main.cpp index ae22c476..bd509a3c 100644 --- a/QuickView/main.cpp +++ b/QuickView/main.cpp @@ -105,6 +105,7 @@ using namespace Microsoft::WRL; // Globals #include "FileNavigator.h" #include "GalleryOverlay.h" +#include "RatingStore.h" #include "Toolbar.h" #include "SettingsOverlay.h" #include "HelpOverlay.h" @@ -517,6 +518,9 @@ static std::wstring g_pairViewRenderedPath; static bool g_pairCompareSession = false; static void ReturnToPairFaceAfterCompareExit(HWND hwnd); static void ArmPairRawFullDecode(const std::wstring& renderedPath, const std::wstring& rawPath); +// [Ratings] Queue the background rating read for one photo, resolving a folded +// pair so that both of its faces end up with the same answer. +static void QueueRatingRead(const std::wstring& path); // [RAW+JPEG Pairing] Delete handling for a folded pair (three-way choice) and // the shared refresh of the not-per-frame pair indicators (title + toolbar). static void HandlePairedDelete(HWND hwnd, const std::wstring& renderedPath, const std::wstring& rawPath, bool isCurrentViewing); @@ -546,6 +550,8 @@ static void ToggleSlideshowPlayback(HWND hwnd) { ViewState g_preservedViewState; int g_renderExifOrientation = 1; // Exif orientation baked into the bitmap surface static ThumbnailManager g_thumbMgr; +// [Ratings] Isolated from the decode pipeline: its own cache and worker. +RatingStore g_ratingStore; GalleryOverlay g_gallery; // Non-static for extern access from UIRenderer Toolbar g_toolbar; // Non-static for extern access from UIRenderer SettingsOverlay g_settingsOverlay; // Non-static for extern access from UIRenderer @@ -5372,19 +5378,34 @@ void LoadConfig() { g_config.InfoPanelLiteItemsCompare = QuickView::NormalizeCSV(g_config.InfoPanelLiteItemsCompare, allowedCompare, 8); wchar_t bufFullItems[1024]; - GetPrivateProfileStringW(L"Controls", L"InfoPanelFullItemsNormal", L"Histogram,File,Position,RAW,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,GPS", bufFullItems, 1024, iniPath.c_str()); + GetPrivateProfileStringW(L"Controls", L"InfoPanelFullItemsNormal", L"Histogram,File,Position,RAW,Rating,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,GPS", bufFullItems, 1024, iniPath.c_str()); g_config.InfoPanelFullItemsNormal = bufFullItems; - GetPrivateProfileStringW(L"Controls", L"InfoPanelFullItemsCompare", L"Histogram,File,RAW,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,Sharp,Ent,BPP,GPS", bufFullItems, 1024, iniPath.c_str()); + GetPrivateProfileStringW(L"Controls", L"InfoPanelFullItemsCompare", L"Histogram,File,RAW,Rating,Size,Disk,Date,Camera,Exp,Lens,Focal,Profile,HDR,Flash,W.Bal,Meter,Prog,Program,Format,Sharp,Ent,BPP,GPS", bufFullItems, 1024, iniPath.c_str()); g_config.InfoPanelFullItemsCompare = bufFullItems; g_config.InfoPanelScale = std::clamp(static_cast(GetPrivateProfileIntW(L"Controls", L"InfoPanelScale", 0, iniPath.c_str())), 0, 5); - std::vector allowedFullNormal = { L"Histogram", L"File", L"Position", L"RAW", L"Size", L"Disk", L"Date", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"Format", L"Sharp", L"Ent", L"BPP", L"GPS" }; - std::vector allowedFullCompare = { L"Histogram", L"File", L"RAW", L"Size", L"Disk", L"Date", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"Format", L"Sharp", L"Ent", L"BPP" }; + std::vector allowedFullNormal = { L"Histogram", L"File", L"Position", L"RAW", L"Rating", L"Size", L"Disk", L"Date", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"Format", L"Sharp", L"Ent", L"BPP", L"GPS" }; + std::vector allowedFullCompare = { L"Histogram", L"File", L"RAW", L"Rating", L"Size", L"Disk", L"Date", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"Format", L"Sharp", L"Ent", L"BPP" }; g_config.InfoPanelFullItemsNormal = QuickView::NormalizeCSV(g_config.InfoPanelFullItemsNormal, allowedFullNormal, 99); g_config.InfoPanelFullItemsCompare = QuickView::NormalizeCSV(g_config.InfoPanelFullItemsCompare, allowedFullCompare, 99); + // [Ratings] One-shot migration. An ini written before the Rating row + // existed holds a list that predates it, and since the list is a + // whitelist the new row would stay invisible forever. Add it once and + // remember having done so, leaving a later removal by the user alone. + if (GetPrivateProfileIntW(L"Controls", L"RatingItemMigrated", 0, iniPath.c_str()) == 0) { + auto addRatingItem = [](std::wstring& csv) { + if ((L"," + csv + L",").find(L",Rating,") != std::wstring::npos) return; + if (!csv.empty()) csv += L","; + csv += L"Rating"; + }; + addRatingItem(g_config.InfoPanelFullItemsNormal); + addRatingItem(g_config.InfoPanelFullItemsCompare); + WritePrivateProfileStringW(L"Controls", L"RatingItemMigrated", L"1", iniPath.c_str()); + } + wchar_t bufSeparator[64]; GetPrivateProfileStringW(L"Controls", L"InfoPanelLiteSeparator", L"\" \u00b7 \"", bufSeparator, 64, iniPath.c_str()); std::wstring rawSep = bufSeparator; @@ -7555,6 +7576,7 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE, [[maybe_unused]] LPWSTR lpCm // Init Gallery g_thumbMgr.Initialize(hwnd, g_imageLoader.get()); + g_ratingStore.Initialize(hwnd); g_gallery.Initialize(&g_thumbMgr, &GetPaneContext(PaneSlot::Primary).navigator); g_settingsOverlay.Init(g_renderEngine->GetDeviceContext(), hwnd); g_helpOverlay.Init(g_renderEngine->GetDeviceContext(), hwnd); @@ -9039,6 +9061,7 @@ case WM_DESTROY: { g_webContentHost.reset(); } g_thumbMgr.Shutdown(); + g_ratingStore.Shutdown(); QuickView::WebViewThumbService::Instance().Shutdown(); PostQuitMessage(0); return 0; @@ -10294,11 +10317,27 @@ SKIP_EDGE_NAV:; } return 0; + case WM_RATING_READY: + // [Ratings] A background read filled the cache; refresh what shows it. + // The info panel lives on the static layer, so refreshing only the + // dynamic one would leave the star row unpainted until something else + // happened to dirty it. + RequestRepaint(PaintLayer::Static | PaintLayer::Dynamic); + if (g_gallery.IsVisible()) RequestRepaint(PaintLayer::Gallery); + return 0; + case WM_APP + 4: // WM_DEFERRED_REPAINT ::InvalidateRect(hwnd, nullptr, FALSE); return 0; case WM_NAVIGATOR_DIR_CHANGED: { + // [Ratings] The folder changed on disk, so cached ratings may be stale + // (an external tool can rewrite a sidecar at any time). The watcher + // also fires right after a folder is opened, so the photo on screen + // has to be queued again immediately -- otherwise its rating would + // stay blank until the user navigated somewhere else. + g_ratingStore.Clear(); + QueueRatingRead(GetPaneContext(PaneSlot::Primary).path); auto& nav = GetPaneContext(PaneSlot::Primary).navigator; const size_t oldCount = nav.Count(); const int oldIndex = nav.Index(); @@ -15006,6 +15045,8 @@ void StartNavigation(HWND hwnd, std::wstring path, [[maybe_unused]] bool showOSD || (!g_pairViewRawPath.empty() && path == g_pairViewRawPath); g_toolbar.SetRawState(QuickView::IsRawPath(path) || navHasPairedRaw, g_runtime.ForceRawDecode, isPairedView); + + QueueRatingRead(path); } if (IsCompareModeActive()) { RefreshCompareRawUI(hwnd); @@ -16303,6 +16344,25 @@ static void ReturnToPairFaceAfterCompareExit(HWND hwnd) { // the current primary image. (The gallery badge, info panel and EXIF row are // per-frame and only need a repaint.) Compare mode routes through the existing // RefreshCompareRawUI instead. +static void QueueRatingRead(const std::wstring& path) { + if (path.empty()) return; + const auto& nav = GetPaneContext(PaneSlot::Primary).navigator; + + // A folded pair is read as one photo: the rendered file carries the + // in-file rating and the RAW's sidecar the authoritative one, so whichever + // face is on screen resolves to the same stars. + std::wstring rendered = path; + std::wstring raw; + if (const auto* pairedRaw = nav.GetPairedRaw(FileNavigator::PathToImageID(path))) { + raw = pairedRaw->path; + } else if (!g_pairViewRawPath.empty() && path == g_pairViewRawPath && + !g_pairViewRenderedPath.empty()) { + rendered = g_pairViewRenderedPath; // showing the RAW face of a pair + raw = path; + } + g_ratingStore.QueueRead(FileNavigator::PathToImageID(path), rendered, raw); +} + static void RefreshCurrentPairIndicators(HWND hwnd) { auto& pane = GetPaneContext(PaneSlot::Primary); const std::wstring cur = pane.path; From 06831c8057dc5c7d557ba7e849a10405e2d45fd3 Mon Sep 17 00:00:00 2001 From: Zhou Ying Date: Mon, 24 Aug 2026 16:59:49 +0800 Subject: [PATCH 3/5] feat(rating): show ratings in the gallery and the info panel lite (#201) Completes the display side of star ratings. The gallery draws the filled stars on a dark chip in the bottom-left corner, opposite the RAW badge and styled exactly like it, because a coloured badge would shout across a wall of thumbnails. An unrated photo gets no chip at all, so a folder nobody has rated looks exactly as it did before. The reads ride along with the thumbnail visibility pass -- the cache is consulted first and only a miss is queued, and nothing is queued at all while the columns are being zoomed -- so opening a folder never turns into a scan of every file in it. The compact info panel gains the same field, available but off by default: it is a single terse line with room for eight items, so an existing layout is left as its owner arranged it. Like the gallery it stays silent for an unrated photo, while the full panel keeps showing empty stars. The compact panel caches its text behind a state hash of its own, so the rating joins that hash for the same reason it joined the full panel's: a value that arrives after the first build would otherwise never be shown. Also fixes the migration added in the previous commit. It recorded that it had run before the migrated lists were persisted, which left them to the next SaveConfig -- so a process that was killed in between kept the flag and lost the item, hiding the row for good. The lists are now written together with the flag. --- QuickView/GalleryOverlay.cpp | 38 +++++++++++++++++++++++++++++++++++ QuickView/SettingsOverlay.cpp | 4 ++-- QuickView/UIRenderer.cpp | 18 +++++++++++++++++ QuickView/main.cpp | 12 +++++++++-- 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/QuickView/GalleryOverlay.cpp b/QuickView/GalleryOverlay.cpp index 1a2e96c8..c1aac6b0 100644 --- a/QuickView/GalleryOverlay.cpp +++ b/QuickView/GalleryOverlay.cpp @@ -1,6 +1,7 @@ #include "pch.h" #include "AppStrings.h" #include "GalleryOverlay.h" +#include "RatingStore.h" #include "Toolbar.h" #include "ThumbnailManager.h" #include "ImageTypes.h" @@ -16,6 +17,7 @@ extern AppConfig g_config; extern HWND g_mainHwnd; extern bool IsLightThemeActive(); extern float g_uiScale; +extern RatingStore g_ratingStore; extern Toolbar g_toolbar; extern RuntimeConfig g_runtime; extern std::wstring& g_imagePath; @@ -798,6 +800,42 @@ void GalleryOverlay::Render(ID2D1DeviceContext *pDC, const D2D1_SIZE_F &size, m_pThumbMgr->QueueRequest(imgId, path.c_str(), prio); } } + // [Ratings] Filled stars in the opposite corner from the RAW badge. + // The read is queued lazily, riding the same visibility pass as the + // thumbnails; an unrated photo gets no chip, so a folder nobody has + // rated stays exactly as quiet as before. + { + const auto rating = g_ratingStore.TryGet(imgId); + if (!rating && !m_isZooming) { + const FileNavigator::PairedRaw* ratingRaw = m_pNav->GetPairedRaw(imgId); + g_ratingStore.QueueRead(imgId, path, ratingRaw ? ratingRaw->path : std::wstring()); + } + if (rating && rating->stars > 0) { + std::wstring stars; + for (int sIdx = 0; sIdx < rating->stars; ++sIdx) stars += L"\u2605"; + + const float bw = (8.0f + 7.5f * (float)stars.length()) * g_uiScale; + const float bh = 15.0f * g_uiScale; + const float bm = 5.0f * g_uiScale; + const float br = 3.5f * g_uiScale; + D2D1_RECT_F badge = D2D1::RectF(cellRect.left + bm, cellRect.bottom - bm - bh, + cellRect.left + bm + bw, cellRect.bottom - bm); + m_brushBg->SetColor(D2D1::ColorF(0.0f, 0.0f, 0.0f, 0.55f)); + m_brushBg->SetOpacity(m_transitionProgress); + pDC->FillRoundedRectangle(D2D1::RoundedRect(badge, br, br), m_brushBg.Get()); + + D2D1_COLOR_F prevStarTxt = m_brushText->GetColor(); + // Same white-on-dark chip as the RAW badge: the star shape + // already carries the meaning, so a coloured one would only + // shout across a wall of thumbnails. + m_brushText->SetColor(D2D1::ColorF(D2D1::ColorF::White)); + m_brushText->SetOpacity(m_transitionProgress * 0.95f); + pDC->DrawText(stars.c_str(), (UINT32)stars.length(), m_textFormatBadge.Get(), badge, m_brushText.Get()); + m_brushText->SetColor(prevStarTxt); + m_brushText->SetOpacity(1.0f); + } + } + // [RAW+JPEG Pairing] "+CR3"-style badge: this item carries a hidden // RAW. Theme-independent dark chip so it reads on any photo content. if (const FileNavigator::PairedRaw* pairedRaw = m_pNav->GetPairedRaw(imgId)) { diff --git a/QuickView/SettingsOverlay.cpp b/QuickView/SettingsOverlay.cpp index de12343f..6150eaf8 100644 --- a/QuickView/SettingsOverlay.cpp +++ b/QuickView/SettingsOverlay.cpp @@ -1770,7 +1770,7 @@ void SettingsOverlay::BuildMenu() { tagCloudNormal.label = AppStrings::Settings_Label_ItemsInNormalMode; tagCloudNormal.type = OptionType::TagCloud; tagCloudNormal.pStrVal = &g_config.InfoPanelLiteItemsNormal; - tagCloudNormal.options = { L"Zoom", L"Progress", L"File", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"GPS" }; + tagCloudNormal.options = { L"Zoom", L"Progress", L"File", L"Rating", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"GPS" }; tagCloudNormal.onChange = []([[maybe_unused]] SettingsOverlay* overlay, [[maybe_unused]] SettingsItem* item) { SaveConfig(); }; tabVisuals.items.push_back(tagCloudNormal); @@ -1778,7 +1778,7 @@ void SettingsOverlay::BuildMenu() { tagCloudCompare.label = AppStrings::Settings_Label_ItemsInCompareMode; tagCloudCompare.type = OptionType::TagCloud; tagCloudCompare.pStrVal = &g_config.InfoPanelLiteItemsCompare; - tagCloudCompare.options = { L"Zoom", L"Progress", L"File", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program" }; + tagCloudCompare.options = { L"Zoom", L"Progress", L"File", L"Rating", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program" }; tagCloudCompare.onChange = []([[maybe_unused]] SettingsOverlay* overlay, [[maybe_unused]] SettingsItem* item) { SaveConfig(); }; tabVisuals.items.push_back(tagCloudCompare); diff --git a/QuickView/UIRenderer.cpp b/QuickView/UIRenderer.cpp index ce461bd7..1b8f1ea6 100644 --- a/QuickView/UIRenderer.cpp +++ b/QuickView/UIRenderer.cpp @@ -2578,6 +2578,17 @@ namespace { } return std::nullopt; } + else if (key == L"Rating") { + // Only a rated photo takes up room in the compact strip; the + // detailed panel is where an unrated one still shows its row. + const std::wstring p = path.empty() ? meta.SourcePath : path; + if (p.empty()) return std::nullopt; + const auto rating = g_ratingStore.TryGet(FileNavigator::PathToImageID(p)); + if (!rating || rating->stars <= 0) return std::nullopt; + std::wstring stars; + for (int i = 0; i < rating->stars; ++i) stars += L"\u2605"; + return stars; + } else if (key == L"Format") { std::wstring fmtStr; if (!meta.FormatDetails.empty()) { @@ -2837,6 +2848,13 @@ std::wstring UIRenderer::BuildCompactInfoText(float maxFileW) const { CombineHash(stateHash, currentZoom); CombineHash(stateHash, g_imagePath); CombineHash(stateHash, g_config.InfoPanelLiteItemsNormal); + // [Ratings] Same reason as the full panel: the read lands asynchronously, + // so it must be part of the cache key or the strip would never update. + if (const auto rating = g_ratingStore.TryGet(FileNavigator::PathToImageID(g_imagePath))) { + CombineHash(stateHash, rating->stars); + } else { + CombineHash(stateHash, -1); + } CombineHash(stateHash, maxFileW); CombineHash(stateHash, g_currentMetadata.IsFullMetadataLoaded); CombineHash(stateHash, g_currentMetadata.HasSharpness); diff --git a/QuickView/main.cpp b/QuickView/main.cpp index bd509a3c..1d4feda0 100644 --- a/QuickView/main.cpp +++ b/QuickView/main.cpp @@ -5372,8 +5372,8 @@ void LoadConfig() { g_config.InfoPanelLiteItemsCompare = bufLiteItems; // Normalize loaded CSV configs (de-duplicate, check against allowed tags, limit to kInfoPanelLiteMaxItems (8)) - std::vector allowedNormal = { L"Zoom", L"Progress", L"File", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"GPS" }; - std::vector allowedCompare = { L"Zoom", L"Progress", L"File", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program" }; + std::vector allowedNormal = { L"Zoom", L"Progress", L"File", L"Rating", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program", L"GPS" }; + std::vector allowedCompare = { L"Zoom", L"Progress", L"File", L"Rating", L"Size", L"Disk", L"Date", L"Format", L"Sharp", L"Ent", L"BPP", L"Camera", L"Exp", L"Lens", L"Focal", L"Profile", L"HDR", L"Flash", L"W.Bal", L"Meter", L"Prog", L"Program" }; g_config.InfoPanelLiteItemsNormal = QuickView::NormalizeCSV(g_config.InfoPanelLiteItemsNormal, allowedNormal, 8); g_config.InfoPanelLiteItemsCompare = QuickView::NormalizeCSV(g_config.InfoPanelLiteItemsCompare, allowedCompare, 8); @@ -5403,6 +5403,14 @@ void LoadConfig() { }; addRatingItem(g_config.InfoPanelFullItemsNormal); addRatingItem(g_config.InfoPanelFullItemsCompare); + // The lists are persisted here rather than left to the next SaveConfig: + // if the process were killed before that ran, the "migrated" flag would + // already be set while the lists still lacked the item, and the row + // would then stay hidden for good. + WritePrivateProfileStringW(L"Controls", L"InfoPanelFullItemsNormal", + g_config.InfoPanelFullItemsNormal.c_str(), iniPath.c_str()); + WritePrivateProfileStringW(L"Controls", L"InfoPanelFullItemsCompare", + g_config.InfoPanelFullItemsCompare.c_str(), iniPath.c_str()); WritePrivateProfileStringW(L"Controls", L"RatingItemMigrated", L"1", iniPath.c_str()); } From 27bee238a5bf0a6f1e3a993b3a9318acdc94c372 Mon Sep 17 00:00:00 2001 From: Zhou Ying Date: Wed, 26 Aug 2026 12:33:19 +0800 Subject: [PATCH 4/5] feat(rating): rate photos with the numpad and store it in the file (#201) Ratings become editable. Rate0..Rate5 are ordinary rebindable hotkeys, defaulting to the numeric keypad with 0 clearing the rating, and they have their own heading in Settings > Shortcuts. A keypress updates the in-memory rating and repaints immediately, while the disk write follows on its own thread behind a 400 ms debounce, so running 1-3-5 through a photo leaves one write rather than three. The debounce never costs a rating: navigating away commits it at once (the photo is no longer held open, which is the moment a rebuild becomes safe), and anything still pending is written on shutdown regardless of its deadline. A photo that cannot carry a rating (an archive entry, an unsupported format, a read-only file) says so through the OSD instead of swallowing the keystroke, and nothing here releases image resources, so the viewport neither flickers nor reloads. Writing goes through the fast metadata encoder first, which patches the value into the padding the file already has and leaves every pixel byte alone. When there is no room the file is rebuilt with WriteSource over the decoded frame, copying the compressed data verbatim -- checked by comparing ~11k sampled pixels before and after, with zero differences. Clearing removes the property instead of storing a zero, so the file goes back to carrying no rating at all. Two details the rebuild path needs to be correct: - The source is decoded from a copy in memory, because a decoder opened on the path keeps the file open even after every interface has been released, and the swap then fails with a sharing violation. - The metadata query writer is released before the swap as well, since it holds the frame encoder, which holds the temp file's stream, and ReplaceFileW needs the replacement to itself. Because the rebuild replaces the file, which is not safe while it is mapped for display, a write for the photo on screen waits for the user to navigate away rather than retrying on a timer, and the write thread sleeps until the next entry is actually due instead of on a fixed tick. It also initializes its own COM apartment, since WIC is COM and this is not the UI thread. As requested on #201, the hardcoded 1 and 0 shortcuts for 100% and Fit are gone; Z and F remain as the bindings, and the help overlay and the context menu labels are updated to match. The context menu's own zoom entries used to be routed by faking those keypresses, so they now call the actions directly. --- CMakeLists.txt | 1 + QuickView/AppStrings.cpp | 60 ++++++--- QuickView/EditState.h | 18 +++ QuickView/HelpOverlay.cpp | 5 +- QuickView/RatingStore.cpp | 171 ++++++++++++++++++++++++ QuickView/RatingStore.h | 46 +++++++ QuickView/RatingWriter.cpp | 236 ++++++++++++++++++++++++++++++++++ QuickView/RatingWriter.h | 43 +++++++ QuickView/SettingsOverlay.cpp | 2 + QuickView/main.cpp | 92 +++++++++++-- 10 files changed, 647 insertions(+), 27 deletions(-) create mode 100644 QuickView/RatingWriter.cpp create mode 100644 QuickView/RatingWriter.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 4720a1b2..dd4281c5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -80,6 +80,7 @@ set(QUICKVIEW_SOURCES QuickView/exif.cpp QuickView/RatingMetadata.cpp QuickView/RatingStore.cpp + QuickView/RatingWriter.cpp QuickView/RenderEngine.cpp QuickView/ImageLoader.cpp QuickView/MiniTiff.cpp diff --git a/QuickView/AppStrings.cpp b/QuickView/AppStrings.cpp index e86214f2..99ea2e8b 100644 --- a/QuickView/AppStrings.cpp +++ b/QuickView/AppStrings.cpp @@ -1273,8 +1273,8 @@ static const LanguageTable Table_EN = { L"Flip Horizontal\tH", // Context_FlipH L"Flip Vertical\tV", // Context_FlipV L"Transform", // Context_Transform - L"Actual Size (100%)\t1 / Z", // Context_ActualSize - L"Fit to Screen\t0 / F", // Context_FitToScreen + L"Actual Size (100%)\tZ", // Context_ActualSize + L"Fit to Screen\tF", // Context_FitToScreen L"Fit Window", // Context_FitWindow L"Fill Window", // Context_FillWindow L"Zoom In\t+ / Ctrl +", // Context_ZoomIn @@ -1849,8 +1849,8 @@ static const LanguageTable Table_CN = { L"水平翻转\tH", // Context_FlipH L"垂直翻转\tV", // Context_FlipV L"变换", // Context_Transform - L"实际大小 (100%)\t1 / Z", // Context_ActualSize - L"适应屏幕\t0 / F", // Context_FitToScreen + L"实际大小 (100%)\tZ", // Context_ActualSize + L"适应屏幕\tF", // Context_FitToScreen L"适应窗口", // Context_FitWindow L"填充窗口", // Context_FillWindow L"放大\t+ / Ctrl +", // Context_ZoomIn @@ -2425,8 +2425,8 @@ static const LanguageTable Table_TW = { L"水平翻轉\tH", // Context_FlipH L"垂直翻轉\tV", // Context_FlipV L"變換", // Context_Transform - L"實際大小 (100%)\t1 / Z", // Context_ActualSize - L"適應螢幕\t0 / F", // Context_FitToScreen + L"實際大小 (100%)\tZ", // Context_ActualSize + L"適應螢幕\tF", // Context_FitToScreen L"適應視窗", // Context_FitWindow L"填滿視窗", // Context_FillWindow L"放大\t+ / Ctrl +", // Context_ZoomIn @@ -3001,8 +3001,8 @@ static const LanguageTable Table_JA = { L"左右反転\tH", // Context_FlipH L"上下反転\tV", // Context_FlipV L"変形", // Context_Transform - L"原寸大 (100%)\t1 / Z", // Context_ActualSize - L"画面に合わせる\t0 / F", // Context_FitToScreen + L"原寸大 (100%)\tZ", // Context_ActualSize + L"画面に合わせる\tF", // Context_FitToScreen L"ウィンドウに合わせる", // Context_FitWindow L"ウィンドウを埋める", // Context_FillWindow L"拡大\t+ / Ctrl +", // Context_ZoomIn @@ -3577,8 +3577,8 @@ static const LanguageTable Table_RU = { L"Отразить по горизонтали\tH", // Context_FlipH L"Отразить по вертикали\tV", // Context_FlipV L"Преобразовать", // Context_Transform - L"Настоящий размер (100%)\t1 / Z", // Context_ActualSize - L"По размеру экрана\t0 / F", // Context_FitToScreen + L"Настоящий размер (100%)\tZ", // Context_ActualSize + L"По размеру экрана\tF", // Context_FitToScreen L"По размеру окна", // Context_FitWindow L"Заполнить окно", // Context_FillWindow L"Увеличить\t+ / Ctrl +", // Context_ZoomIn @@ -4153,8 +4153,8 @@ static const LanguageTable Table_DE = { L"Horizontal spiegeln\tH", // Context_FlipH L"Vertikal spiegeln\tV", // Context_FlipV L"Transformieren", // Context_Transform - L"Originalgröße (100%)\t1 / Z", // Context_ActualSize - L"An Bildschirm anpassen\t0 / F", // Context_FitToScreen + L"Originalgröße (100%)\tZ", // Context_ActualSize + L"An Bildschirm anpassen\tF", // Context_FitToScreen L"An Fenster anpassen", // Context_FitWindow L"Fenster ausfüllen", // Context_FillWindow L"Vergrößern\t+ / Strg +", // Context_ZoomIn @@ -4729,8 +4729,8 @@ static const LanguageTable Table_ES = { L"Voltear horizontal\tH", // Context_FlipH L"Voltear vertical\tV", // Context_FlipV L"Transformar", // Context_Transform - L"Tamaño real (100%)\t1 / Z", // Context_ActualSize - L"Ajustar a pantalla\t0 / F", // Context_FitToScreen + L"Tamaño real (100%)\tZ", // Context_ActualSize + L"Ajustar a pantalla\tF", // Context_FitToScreen L"Ajustar a ventana", // Context_FitWindow L"Rellenar ventana", // Context_FillWindow L"Acercar\t+ / Ctrl +", // Context_ZoomIn @@ -5305,8 +5305,8 @@ static const LanguageTable Table_FR = { L"Flip Horizontal\tH", // Context_FlipH L"Flip Vertical\tV", // Context_FlipV L"Transform", // Context_Transform - L"Actual Size (100%)\t1 / Z", // Context_ActualSize - L"Fit to Screen\t0 / F", // Context_FitToScreen + L"Actual Size (100%)\tZ", // Context_ActualSize + L"Fit to Screen\tF", // Context_FitToScreen L"Fit Window", // Context_FitWindow L"Fill Window", // Context_FillWindow L"Zoom In\t+ / Ctrl +", // Context_ZoomIn @@ -6717,6 +6717,34 @@ std::wstring GetHotkeyActionName(HotkeyAction action) { case HotkeyAction::RenderRaw: raw = AppStrings::Context_RenderRAW; break; + case HotkeyAction::Rate0: + case HotkeyAction::Rate1: + case HotkeyAction::Rate2: + case HotkeyAction::Rate3: + case HotkeyAction::Rate4: + case HotkeyAction::Rate5: { + needsCleaning = false; + const int stars = (int)action - (int)HotkeyAction::Rate0; + // Kept as one string per language rather than a table entry: these + // names appear only in the shortcut list. + static thread_local std::wstring s_rateName; + const wchar_t* pattern; + switch (GetActiveLanguage()) { + case AppStrings::Language::ChineseSimplified: pattern = stars ? L"评分 %d 星" : L"清除评分"; break; + case AppStrings::Language::ChineseTraditional: pattern = stars ? L"評分 %d 星" : L"清除評分"; break; + case AppStrings::Language::Japanese: pattern = stars ? L"評価 %d つ星" : L"評価を消去"; break; + case AppStrings::Language::Russian: pattern = stars ? L"Оценка: %d" : L"Снять оценку"; break; + case AppStrings::Language::German: pattern = stars ? L"%d Sterne" : L"Bewertung löschen"; break; + case AppStrings::Language::Spanish: pattern = stars ? L"%d estrellas" : L"Quitar valoración"; break; + case AppStrings::Language::French: pattern = stars ? L"%d étoiles" : L"Effacer la note"; break; + default: pattern = stars ? L"Rate %d Stars" : L"Clear Rating"; break; + } + wchar_t buf[64]; + swprintf_s(buf, pattern, stars); + s_rateName = buf; + raw = s_rateName.c_str(); + break; + } case HotkeyAction::ComparePair: { needsCleaning = false; switch (GetActiveLanguage()) { diff --git a/QuickView/EditState.h b/QuickView/EditState.h index 84697f3f..df41f0f4 100644 --- a/QuickView/EditState.h +++ b/QuickView/EditState.h @@ -189,6 +189,12 @@ enum class HotkeyAction : uint8_t { ToggleSpan, // Toggle Span Displays ToggleSlideshow, // Toggle Slideshow Mode RenderRaw, // Toggle RAW decode / switch to the paired RAW + Rate0, // Clear the star rating + Rate1, // Rate the photo 1..5 stars + Rate2, + Rate3, + Rate4, + Rate5, OpenFile, // Open File Dialog EditFile, // Edit with External Editor RenameFile, // Rename File Dialog @@ -262,6 +268,12 @@ inline std::wstring_view HotkeyActionToString(HotkeyAction action) noexcept { case HotkeyAction::ShowInExplorer: return L"ShowInExplorer"; case HotkeyAction::ToggleCompare: return L"ToggleCompare"; case HotkeyAction::ComparePair: return L"ComparePair"; + case HotkeyAction::Rate0: return L"Rate0"; + case HotkeyAction::Rate1: return L"Rate1"; + case HotkeyAction::Rate2: return L"Rate2"; + case HotkeyAction::Rate3: return L"Rate3"; + case HotkeyAction::Rate4: return L"Rate4"; + case HotkeyAction::Rate5: return L"Rate5"; case HotkeyAction::AlwaysOnTop: return L"AlwaysOnTop"; case HotkeyAction::ToggleDebugHud: return L"ToggleDebugHud"; case HotkeyAction::Print: return L"Print"; @@ -323,6 +335,12 @@ inline HotkeyAction StringToHotkeyAction(std::wstring_view sv) noexcept { if (sv == L"ShowInExplorer") return HotkeyAction::ShowInExplorer; if (sv == L"ToggleCompare") return HotkeyAction::ToggleCompare; if (sv == L"ComparePair") return HotkeyAction::ComparePair; + if (sv == L"Rate0") return HotkeyAction::Rate0; + if (sv == L"Rate1") return HotkeyAction::Rate1; + if (sv == L"Rate2") return HotkeyAction::Rate2; + if (sv == L"Rate3") return HotkeyAction::Rate3; + if (sv == L"Rate4") return HotkeyAction::Rate4; + if (sv == L"Rate5") return HotkeyAction::Rate5; if (sv == L"AlwaysOnTop") return HotkeyAction::AlwaysOnTop; if (sv == L"ToggleDebugHud") return HotkeyAction::ToggleDebugHud; if (sv == L"Print") return HotkeyAction::Print; diff --git a/QuickView/HelpOverlay.cpp b/QuickView/HelpOverlay.cpp index a4f93135..78e0b07c 100644 --- a/QuickView/HelpOverlay.cpp +++ b/QuickView/HelpOverlay.cpp @@ -138,12 +138,13 @@ void HelpOverlay::RebuildList() { m_items.push_back({ false, L"F12", L"Debug HUD (Enable in Settings)" }); m_items.push_back({ false, L"T / Ctrl+T", m_strHudTopCombined.c_str() }); - m_items.push_back({ false, L"1 / Z", AppStrings::OSD_Zoom100 }); - m_items.push_back({ false, L"0 / F", AppStrings::OSD_ZoomFit }); + m_items.push_back({ false, L"Z", AppStrings::OSD_Zoom100 }); + m_items.push_back({ false, L"F", AppStrings::OSD_ZoomFit }); m_items.push_back({ false, L"+ (\x2191) / - (\x2193)", L"Zoom (+/- 10%)" }); m_items.push_back({ false, L"Ctrl + (+/-)", L"Zoom (+/- 1%)" }); m_items.push_back({ false, L"I / Tab", L"Info Panel (Full / Lite)" }); + m_items.push_back({ false, L"Numpad 0-5", L"Star Rating (0 clears)" }); m_items.push_back({ false, L"C", AppStrings::Help_Item_Compare }); m_items.push_back({ false, L"Ctrl + F11", AppStrings::Settings_Label_SpanDisplays }); diff --git a/QuickView/RatingStore.cpp b/QuickView/RatingStore.cpp index 64a125c3..7a3dc837 100644 --- a/QuickView/RatingStore.cpp +++ b/QuickView/RatingStore.cpp @@ -18,8 +18,10 @@ #include "RatingStore.h" +#include "RatingWriter.h" #include "SupportedExtensions.h" +#include #include namespace { @@ -29,6 +31,10 @@ namespace { constexpr DWORD HEADER_READ_BYTES = 128 * 1024; constexpr DWORD SIDECAR_READ_BYTES = 16 * 1024; +// Long enough that holding a digit or running 1-3-5 lands one write, +// short enough that the file is up to date by the time the user looks. +constexpr auto WRITE_DEBOUNCE = std::chrono::milliseconds(400); + // Wide-char file API throughout: a path round-tripped through a narrow code // page fails to open on non-ASCII names. std::vector ReadFilePrefix(const std::wstring& path, DWORD maxBytes) { @@ -69,6 +75,7 @@ void RatingStore::Initialize(HWND hwnd) { m_hwnd = hwnd; m_running = true; m_worker = std::thread(&RatingStore::WorkerLoop, this); + m_writeWorker = std::thread(&RatingStore::WriteLoop, this); } void RatingStore::Shutdown() { @@ -80,6 +87,13 @@ void RatingStore::Shutdown() { } m_cv.notify_all(); if (m_worker.joinable()) m_worker.join(); + + m_writeCv.notify_all(); + if (m_writeWorker.joinable()) m_writeWorker.join(); + // Ratings the user set moments before closing must still reach the disk, + // including any whose rebuild was waiting for the photo to leave the + // screen -- which it now has. + FlushPendingWrites(); } std::wstring RatingStore::SidecarPathFor(const std::wstring& path) { @@ -112,6 +126,163 @@ std::optional RatingStore::ReadRatingFromFile(const std::wstring& path) { : QuickView::Rating::ParseTiffRating(bytes); } +RatingStore::Writability RatingStore::GetWritability(const std::wstring& renderedPath, + const std::wstring& rawPath) { + // A RAW is rated through its sidecar, so the RAW itself never has to be + // writable -- only the folder does, which the write attempt will report. + if (!rawPath.empty()) return Writability::Writable; + + if (renderedPath.find(L'|') != std::wstring::npos) { + return Writability::UnsupportedFormat; // entry inside an archive + } + + const std::wstring_view ext = QuickView::ExtensionOf(renderedPath); + const bool inFileRatable = QuickView::ExtEqualsIgnoreCase(ext, L".jpg") || + QuickView::ExtEqualsIgnoreCase(ext, L".jpeg") || + QuickView::ExtEqualsIgnoreCase(ext, L".tif") || + QuickView::ExtEqualsIgnoreCase(ext, L".tiff"); + // A standalone RAW still resolves through a sidecar of its own. + if (!inFileRatable && !QuickView::IsRawPath(renderedPath)) { + return Writability::UnsupportedFormat; + } + + const DWORD attrs = GetFileAttributesW(renderedPath.c_str()); + if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_READONLY)) { + return Writability::ReadOnlyFile; + } + return Writability::Writable; +} + +QuickView::Rating::Resolved RatingStore::ApplyRatingOptimistic( + ImageID id, int stars, const std::wstring& renderedPath, const std::wstring& rawPath, + bool isResident) { + QuickView::Rating::Resolved resolved; + resolved.stars = stars; + // A rating set here is the user's, so it is authoritative on both carriers + // and there is no disagreement left to report. + resolved.source = rawPath.empty() ? QuickView::Rating::Source::InFile + : QuickView::Rating::Source::Sidecar; + if (stars == 0) resolved.source = QuickView::Rating::Source::None; + + { + std::lock_guard lock(m_cacheMutex); + m_cache[id] = resolved; + } + + { + // Replacing the entry is what makes a burst of keypresses collapse + // into one write: only the last value survives to reach the disk. + std::lock_guard lock(m_writeMutex); + PendingWrite& pending = m_pendingWrites[id]; + pending.stars = stars; + pending.renderedPath = renderedPath; + pending.rawPath = rawPath; + pending.resident = isResident; + pending.due = std::chrono::steady_clock::now() + WRITE_DEBOUNCE; + } + m_writeCv.notify_one(); + return resolved; +} + +void RatingStore::ReleaseResident(const std::wstring& nowResidentPath) { + bool woke = false; + { + std::lock_guard lock(m_writeMutex); + for (auto& [id, pending] : m_pendingWrites) { + if (pending.resident && pending.renderedPath != nowResidentPath) { + pending.resident = false; // free to rebuild the file now + pending.due = std::chrono::steady_clock::now(); + woke = true; + } + } + } + if (woke) m_writeCv.notify_one(); +} + +void RatingStore::FlushPendingWrites() { + std::vector due; + { + std::lock_guard lock(m_writeMutex); + for (auto& [id, pending] : m_pendingWrites) { + pending.resident = false; // nothing is on screen any more + due.push_back(pending); + } + m_pendingWrites.clear(); + } + for (const auto& write : due) PerformWrite(write); +} + +void RatingStore::PerformWrite(const PendingWrite& write) { + if (write.renderedPath.empty()) return; + + // A folded pair keeps its rating in the RAW's sidecar as well; that half + // arrives with the sidecar stage. + if (QuickView::IsRawPath(write.renderedPath)) return; + + const auto status = QuickView::Rating::WriteRatingToImage( + write.renderedPath, write.stars, /*allowTranscode*/ !write.resident); + + if (status == QuickView::Rating::WriteStatus::NeedsTranscode) { + // The file has no room for an in-place patch and is still on screen. + // Put it back with no deadline of its own: retrying on a timer would + // reopen the file every debounce for as long as the photo is shown, + // and the only thing that can actually unblock it is the photo + // leaving the screen, which ReleaseResident reports. + std::lock_guard lock(m_writeMutex); + auto& pending = m_pendingWrites[FileNavigator::PathToImageID(write.renderedPath)]; + pending = write; + pending.resident = true; + pending.due = std::chrono::steady_clock::time_point::max(); + } +} + +void RatingStore::WriteLoop() { + // WIC is COM, and this thread owns its own apartment: without this every + // write would fail at CoCreateInstance. + const HRESULT comInit = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + const bool ownsCom = SUCCEEDED(comInit); + + struct ComScope { + bool owns; + ~ComScope() { if (owns) CoUninitialize(); } + } comScope{ ownsCom }; + + while (true) { + std::vector due; + { + std::unique_lock lock(m_writeMutex); + if (m_pendingWrites.empty()) { + m_writeCv.wait(lock, [this] { return !m_running.load() || !m_pendingWrites.empty(); }); + } else { + // Sleep exactly until the next entry is due rather than on a + // fixed tick; an entry waiting for its photo to leave the + // screen has no deadline and must not cause a wakeup at all. + auto earliest = std::chrono::steady_clock::time_point::max(); + for (const auto& [id, pending] : m_pendingWrites) { + earliest = (std::min)(earliest, pending.due); + } + if (earliest == std::chrono::steady_clock::time_point::max()) { + m_writeCv.wait(lock); + } else { + m_writeCv.wait_until(lock, earliest); + } + } + if (!m_running.load()) return; + + const auto now = std::chrono::steady_clock::now(); + for (auto it = m_pendingWrites.begin(); it != m_pendingWrites.end();) { + if (it->second.due <= now) { + due.push_back(it->second); + it = m_pendingWrites.erase(it); + } else { + ++it; + } + } + } + for (const auto& write : due) PerformWrite(write); + } +} + std::optional RatingStore::TryGet(ImageID id) const { std::lock_guard lock(m_cacheMutex); auto it = m_cache.find(id); diff --git a/QuickView/RatingStore.h b/QuickView/RatingStore.h index 2d7e2ce4..f921db49 100644 --- a/QuickView/RatingStore.h +++ b/QuickView/RatingStore.h @@ -23,6 +23,7 @@ #include "RatingMetadata.h" #include +#include #include #include #include @@ -63,6 +64,33 @@ class RatingStore { // Drop everything and cancel in-flight work (folder changed). void Clear(); + // Why a photo cannot be rated, so the UI can say so instead of doing + // nothing when a key is pressed. + enum class Writability { + Writable, + UnsupportedFormat, // no place to put a rating (HEIC, PNG, archive entry...) + ReadOnlyFile, + }; + static Writability GetWritability(const std::wstring& renderedPath, + const std::wstring& rawPath); + + // Apply a rating immediately in memory so the UI can repaint at once, and + // queue the disk write behind a short debounce. Returns the value now on + // display. `isResident` marks the photo currently held open for display: + // such a file is never rebuilt underneath itself, the write waits until + // it is no longer on screen. + QuickView::Rating::Resolved ApplyRatingOptimistic(ImageID id, int stars, + const std::wstring& renderedPath, + const std::wstring& rawPath, + bool isResident); + + // A photo left the screen, so a write that was postponed for it can go + // ahead. Pass the path now on display (empty when there is none). + void ReleaseResident(const std::wstring& nowResidentPath); + + // Write everything still pending, blocking until done (shutdown). + void FlushPendingWrites(); + // The .xmp sidecar a RAW's rating lives in, i.e. the path with its // extension replaced. Empty when `path` has no extension. static std::wstring SidecarPathFor(const std::wstring& path); @@ -80,7 +108,19 @@ class RatingStore { uint64_t generation = 0; }; + // One outstanding rating change per photo: a burst of keypresses replaces + // this entry rather than queuing several writes. + struct PendingWrite { + int stars = 0; + std::wstring renderedPath; + std::wstring rawPath; + std::chrono::steady_clock::time_point due; + bool resident = false; // held open for display: do not rebuild it yet + }; + void WorkerLoop(); + void WriteLoop(); + void PerformWrite(const PendingWrite& write); HWND m_hwnd = nullptr; @@ -95,4 +135,10 @@ class RatingStore { std::thread m_worker; std::atomic m_running{ false }; std::atomic m_generation{ 0 }; + + // Writes live on their own thread so a slow disk cannot hold up reads. + std::thread m_writeWorker; + std::mutex m_writeMutex; + std::condition_variable m_writeCv; + std::unordered_map m_pendingWrites; }; diff --git a/QuickView/RatingWriter.cpp b/QuickView/RatingWriter.cpp new file mode 100644 index 00000000..9bc6b47d --- /dev/null +++ b/QuickView/RatingWriter.cpp @@ -0,0 +1,236 @@ +/* + * QuickView Star Ratings - writing a rating into an image file + * Copyright (C) 2026-Present QuickView Contributors + * + * 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 "RatingWriter.h" + +#include "pch.h" + +#include +#include "SupportedExtensions.h" + +#include + +#include + +using Microsoft::WRL::ComPtr; + +namespace QuickView::Rating { + +namespace { + +// A JPEG nests Exif under APP1; a TIFF carries the IFD directly. Both take +// xmp:Rating, which is the property Lightroom and Bridge read. +struct RatingQueries { + const wchar_t* exif; + const wchar_t* xmp; +}; + +RatingQueries QueriesFor(const std::wstring& path) { + const std::wstring_view ext = QuickView::ExtensionOf(path); + const bool isTiff = QuickView::ExtEqualsIgnoreCase(ext, L".tif") || + QuickView::ExtEqualsIgnoreCase(ext, L".tiff"); + return isTiff ? RatingQueries{ L"/ifd/{ushort=18246}", L"/ifd/xmp/xmp:Rating" } + : RatingQueries{ L"/app1/ifd/{ushort=18246}", L"/xmp/xmp:Rating" }; +} + +// Writing SimpleRating (0..5) is enough for both ecosystems: Explorer and +// Photos derive System.Rating (0..99) from it, and Adobe reads xmp:Rating. +HRESULT ApplyRating(IWICMetadataQueryWriter* writer, const RatingQueries& q, int stars) { + if (stars <= 0) { + // Clearing means removing the property, not storing a zero, so the + // file goes back to carrying no rating at all. A property that was + // not there is not an error. + writer->RemoveMetadataByName(q.exif); + writer->RemoveMetadataByName(q.xmp); + return S_OK; + } + + PROPVARIANT var; + PropVariantInit(&var); + var.vt = VT_UI2; + var.uiVal = (USHORT)stars; + HRESULT hr = writer->SetMetadataByName(q.exif, &var); + PropVariantClear(&var); + if (FAILED(hr)) return hr; + + const std::wstring text = std::to_wstring(stars); + PropVariantInit(&var); + var.vt = VT_LPWSTR; + var.pwszVal = (WCHAR*)CoTaskMemAlloc((text.size() + 1) * sizeof(WCHAR)); + if (!var.pwszVal) return E_OUTOFMEMORY; + wcscpy_s(var.pwszVal, text.size() + 1, text.c_str()); + hr = writer->SetMetadataByName(q.xmp, &var); + PropVariantClear(&var); + // An XMP packet cannot always be added in place; the Exif tag alone still + // satisfies Explorer, so this is not treated as a failure. + return S_OK; +} + +// Patch the value into the padding the file already has: no re-encode, no +// rewrite, every pixel byte untouched. +HRESULT TryInPlace(IWICImagingFactory* factory, const std::wstring& path, int stars) { + ComPtr decoder; + HRESULT hr = factory->CreateDecoderFromFilename(path.c_str(), nullptr, + GENERIC_READ | GENERIC_WRITE, + WICDecodeMetadataCacheOnDemand, &decoder); + if (FAILED(hr)) return hr; + + ComPtr encoder; + hr = factory->CreateFastMetadataEncoderFromDecoder(decoder.Get(), &encoder); + if (FAILED(hr)) return hr; + + ComPtr writer; + hr = encoder->GetMetadataQueryWriter(&writer); + if (FAILED(hr)) return hr; + + hr = ApplyRating(writer.Get(), QueriesFor(path), stars); + if (FAILED(hr)) return hr; + + return encoder->Commit(); +} + +// Rebuild the file when there was no room to patch. WriteSource with a null +// rectangle copies the compressed frame as it is, so the pixels come through +// bit for bit; only the metadata block is new. +HRESULT Transcode(IWICImagingFactory* factory, const std::wstring& path, int stars) { + // Decode from a copy held in memory rather than from the file. A decoder + // opened on the path keeps it open even after every interface has been + // released, and the swap at the end then fails with a sharing violation. + std::vector sourceBytes; + { + HANDLE file = CreateFileW(path.c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN, nullptr); + if (file == INVALID_HANDLE_VALUE) return HRESULT_FROM_WIN32(GetLastError()); + LARGE_INTEGER size{}; + if (!GetFileSizeEx(file, &size) || size.QuadPart <= 0 || size.QuadPart > MAXDWORD) { + CloseHandle(file); + return E_FAIL; + } + sourceBytes.resize((size_t)size.QuadPart); + DWORD read = 0; + const BOOL ok = ReadFile(file, sourceBytes.data(), (DWORD)sourceBytes.size(), &read, nullptr); + CloseHandle(file); + if (!ok || read != sourceBytes.size()) return E_FAIL; + } + + ComPtr sourceStream; + HRESULT hr = factory->CreateStream(&sourceStream); + if (FAILED(hr)) return hr; + hr = sourceStream->InitializeFromMemory(sourceBytes.data(), (DWORD)sourceBytes.size()); + if (FAILED(hr)) return hr; + + ComPtr decoder; + hr = factory->CreateDecoderFromStream(sourceStream.Get(), nullptr, + WICDecodeMetadataCacheOnDemand, &decoder); + if (FAILED(hr)) return hr; + + GUID containerFormat{}; + hr = decoder->GetContainerFormat(&containerFormat); + if (FAILED(hr)) return hr; + + ComPtr frameDecode; + hr = decoder->GetFrame(0, &frameDecode); + if (FAILED(hr)) return hr; + + const std::wstring tempPath = path + L".qvrating.tmp"; + ComPtr stream; + hr = factory->CreateStream(&stream); + if (FAILED(hr)) return hr; + hr = stream->InitializeFromFilename(tempPath.c_str(), GENERIC_WRITE); + if (FAILED(hr)) return hr; + + auto dropTemp = [&tempPath] { DeleteFileW(tempPath.c_str()); }; + + ComPtr encoder; + hr = factory->CreateEncoder(containerFormat, nullptr, &encoder); + if (FAILED(hr)) { dropTemp(); return hr; } + hr = encoder->Initialize(stream.Get(), WICBitmapEncoderNoCache); + if (FAILED(hr)) { dropTemp(); return hr; } + + ComPtr frameEncode; + hr = encoder->CreateNewFrame(&frameEncode, nullptr); + if (FAILED(hr)) { dropTemp(); return hr; } + hr = frameEncode->Initialize(nullptr); + if (FAILED(hr)) { dropTemp(); return hr; } + + // Carry the existing metadata over before adding the rating to it. + ComPtr blockReader; + if (SUCCEEDED(frameDecode.As(&blockReader))) { + ComPtr blockWriter; + if (SUCCEEDED(frameEncode.As(&blockWriter))) { + blockWriter->InitializeFromBlockReader(blockReader.Get()); + } + } + + ComPtr writer; + if (SUCCEEDED(frameEncode->GetMetadataQueryWriter(&writer))) { + ApplyRating(writer.Get(), QueriesFor(path), stars); + } + + hr = frameEncode->WriteSource(frameDecode.Get(), nullptr); + if (FAILED(hr)) { dropTemp(); return hr; } + hr = frameEncode->Commit(); + if (FAILED(hr)) { dropTemp(); return hr; } + hr = encoder->Commit(); + if (FAILED(hr)) { dropTemp(); return hr; } + + // Release everything that keeps the temp file's stream alive before the + // swap -- the query writer counts, since it holds the frame encoder, which + // holds the stream, and ReplaceFileW needs the replacement to itself. + // (The source file needs no such care: it was decoded from memory, so the + // original was only ever open for the moment it took to read it.) + writer.Reset(); + stream.Reset(); + frameEncode.Reset(); + encoder.Reset(); + frameDecode.Reset(); + decoder.Reset(); + sourceStream.Reset(); + + if (!ReplaceFileW(path.c_str(), tempPath.c_str(), nullptr, REPLACEFILE_IGNORE_MERGE_ERRORS, + nullptr, nullptr)) { + const DWORD err = GetLastError(); + dropTemp(); + return HRESULT_FROM_WIN32(err); + } + return S_OK; +} + +} // namespace + +WriteStatus WriteRatingToImage(const std::wstring& path, int stars, bool allowTranscode) { + if (path.empty()) return WriteStatus::Failed; + + ComPtr factory; + if (FAILED(CoCreateInstance(CLSID_WICImagingFactory, nullptr, CLSCTX_INPROC_SERVER, + IID_PPV_ARGS(&factory)))) { + return WriteStatus::Failed; + } + + const HRESULT hrInPlace = TryInPlace(factory.Get(), path, stars); + if (SUCCEEDED(hrInPlace)) { + return WriteStatus::WrittenInPlace; + } + if (!allowTranscode) return WriteStatus::NeedsTranscode; + + const HRESULT hrTrans = Transcode(factory.Get(), path, stars); + return SUCCEEDED(hrTrans) ? WriteStatus::WrittenTranscode : WriteStatus::Failed; +} + +} // namespace QuickView::Rating diff --git a/QuickView/RatingWriter.h b/QuickView/RatingWriter.h new file mode 100644 index 00000000..3e19b1e0 --- /dev/null +++ b/QuickView/RatingWriter.h @@ -0,0 +1,43 @@ +/* + * QuickView Star Ratings - writing a rating into an image file + * Copyright (C) 2026-Present QuickView Contributors + * + * 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 . + */ + +#pragma once + +#include + +namespace QuickView::Rating { + +enum class WriteStatus { + WrittenInPlace, // patched into existing metadata padding; pixels untouched + WrittenTranscode, // rebuilt losslessly because there was no room to patch + NeedsTranscode, // only a rebuild would do, and the caller asked not to + Failed, +}; + +// Store `stars` (0..5) in an image file. 0 removes the rating rather than +// writing a zero, so that clearing restores the file to never-rated. +// +// The fast path patches the value into the metadata padding already in the +// file, which leaves every pixel byte alone. When there is no room, the file +// has to be rebuilt; that rebuild copies the compressed frame verbatim, so it +// is still lossless, but it does replace the file. `allowTranscode` == false +// makes the function report NeedsTranscode instead, which lets the caller +// postpone the rebuild while the file is memory-mapped for display. +WriteStatus WriteRatingToImage(const std::wstring& path, int stars, bool allowTranscode); + +} // namespace QuickView::Rating diff --git a/QuickView/SettingsOverlay.cpp b/QuickView/SettingsOverlay.cpp index 6150eaf8..9cd73e8a 100644 --- a/QuickView/SettingsOverlay.cpp +++ b/QuickView/SettingsOverlay.cpp @@ -2101,6 +2101,8 @@ void SettingsOverlay::BuildMenu() { tabKeys.items.push_back({ isChinese ? L"动画控制" : L"Animation Control", OptionType::Header }); } else if (action == HotkeyAction::ToggleGallery) { tabKeys.items.push_back({ isChinese ? L"视图模式" : L"View Modes", OptionType::Header }); + } else if (action == HotkeyAction::Rate0) { + tabKeys.items.push_back({ isChinese ? L"星级评分" : L"Rating", OptionType::Header }); } else if (action == HotkeyAction::OpenFile) { tabKeys.items.push_back({ isChinese ? L"文件操作" : L"File Operations", OptionType::Header }); } else if (action == HotkeyAction::ToggleOverlay) { diff --git a/QuickView/main.cpp b/QuickView/main.cpp index 1d4feda0..5de26ed4 100644 --- a/QuickView/main.cpp +++ b/QuickView/main.cpp @@ -342,6 +342,13 @@ std::array(HotkeyAction::Count)> g_hotkeys = HotkeyBinding{ HotkeyAction::ToggleSpan, KeyCombo{ VK_F11, 1 }, KeyCombo{ VK_F11, 1 } }, // Ctrl + F11 HotkeyBinding{ HotkeyAction::ToggleSlideshow, KeyCombo{ VK_F10, 0 }, KeyCombo{ VK_F10, 0 } }, HotkeyBinding{ HotkeyAction::RenderRaw, KeyCombo{ 'D', 0 }, KeyCombo{ 'D', 0 } }, // Decode RAW / switch to paired RAW + // Ratings: the numeric keypad, 0 clears + HotkeyBinding{ HotkeyAction::Rate0, KeyCombo{ VK_NUMPAD0, 0 }, KeyCombo{ VK_NUMPAD0, 0 } }, + HotkeyBinding{ HotkeyAction::Rate1, KeyCombo{ VK_NUMPAD1, 0 }, KeyCombo{ VK_NUMPAD1, 0 } }, + HotkeyBinding{ HotkeyAction::Rate2, KeyCombo{ VK_NUMPAD2, 0 }, KeyCombo{ VK_NUMPAD2, 0 } }, + HotkeyBinding{ HotkeyAction::Rate3, KeyCombo{ VK_NUMPAD3, 0 }, KeyCombo{ VK_NUMPAD3, 0 } }, + HotkeyBinding{ HotkeyAction::Rate4, KeyCombo{ VK_NUMPAD4, 0 }, KeyCombo{ VK_NUMPAD4, 0 } }, + HotkeyBinding{ HotkeyAction::Rate5, KeyCombo{ VK_NUMPAD5, 0 }, KeyCombo{ VK_NUMPAD5, 0 } }, HotkeyBinding{ HotkeyAction::OpenFile, KeyCombo{ 'O', 0 }, KeyCombo{ 'O', 0 } }, HotkeyBinding{ HotkeyAction::EditFile, KeyCombo{ 'E', 0 }, KeyCombo{ 'E', 0 } }, HotkeyBinding{ HotkeyAction::RenameFile, KeyCombo{ VK_F2, 0 }, KeyCombo{ VK_F2, 0 } }, @@ -521,6 +528,7 @@ static void ArmPairRawFullDecode(const std::wstring& renderedPath, const std::ws // [Ratings] Queue the background rating read for one photo, resolving a folded // pair so that both of its faces end up with the same answer. static void QueueRatingRead(const std::wstring& path); +static void ApplyRatingToCurrentImage(HWND hwnd, int stars); // [RAW+JPEG Pairing] Delete handling for a folded pair (three-way choice) and // the shared refresh of the not-per-frame pair indicators (title + toolbar). static void HandlePairedDelete(HWND hwnd, const std::wstring& renderedPath, const std::wstring& rawPath, bool isCurrentViewing); @@ -12127,13 +12135,6 @@ SKIP_EDGE_NAV:; if (HandleHotkeyAction(hwnd, HotkeyAction::ZoomOut)) return 0; } } - if (!ctrl && !shift && !alt) { - if (wParam == '1' || wParam == VK_NUMPAD1) { - if (HandleHotkeyAction(hwnd, HotkeyAction::Zoom100)) return 0; - } else if (wParam == '0' || wParam == VK_NUMPAD0) { - if (HandleHotkeyAction(hwnd, HotkeyAction::ZoomFit)) return 0; - } - } } if (message == WM_SYSKEYDOWN) { @@ -13142,8 +13143,10 @@ SKIP_EDGE_NAV:; RequestRepaint(PaintLayer::Static); break; - case IDM_ZOOM_100: SendMessage(hwnd, WM_KEYDOWN, '1', 0); break; - case IDM_ZOOM_FIT: SendMessage(hwnd, WM_KEYDOWN, '0', 0); break; + // Invoke the actions directly: these used to be routed by faking a + // keypress, which only worked through the hardcoded 0/1 fallback. + case IDM_ZOOM_100: HandleHotkeyAction(hwnd, HotkeyAction::Zoom100); break; + case IDM_ZOOM_FIT: HandleHotkeyAction(hwnd, HotkeyAction::ZoomFit); break; case IDM_ZOOM_FIT_WINDOW: HandleHotkeyAction(hwnd, HotkeyAction::ZoomFitWindow); break; case IDM_ZOOM_FILL: HandleHotkeyAction(hwnd, HotkeyAction::ZoomFill); break; case IDM_ZOOM_IN: SendMessage(hwnd, WM_KEYDOWN, VK_ADD, 0); break; @@ -15055,6 +15058,8 @@ void StartNavigation(HWND hwnd, std::wstring path, [[maybe_unused]] bool showOSD g_runtime.ForceRawDecode, isPairedView); QueueRatingRead(path); + // Whatever left the screen can now be rewritten safely. + g_ratingStore.ReleaseResident(path); } if (IsCompareModeActive()) { RefreshCompareRawUI(hwnd); @@ -16352,6 +16357,64 @@ static void ReturnToPairFaceAfterCompareExit(HWND hwnd) { // the current primary image. (The gallery badge, info panel and EXIF row are // per-frame and only need a repaint.) Compare mode routes through the existing // RefreshCompareRawUI instead. +// [Ratings] Resolve which files carry the rating of the photo on screen: the +// rendered file, plus the RAW of a folded pair (empty when there is none). +static void ResolveRatingCarriers(const std::wstring& path, std::wstring& outRendered, + std::wstring& outRaw) { + const auto& nav = GetPaneContext(PaneSlot::Primary).navigator; + outRendered = path; + outRaw.clear(); + if (const auto* pairedRaw = nav.GetPairedRaw(FileNavigator::PathToImageID(path))) { + outRaw = pairedRaw->path; + } else if (!g_pairViewRawPath.empty() && path == g_pairViewRawPath && + !g_pairViewRenderedPath.empty()) { + outRendered = g_pairViewRenderedPath; // showing the RAW face of a pair + outRaw = path; + } +} + +// [Ratings] Rate the photo on screen. The cache and the UI are updated at +// once -- the disk write is deferred to its own stage -- and a photo that +// cannot carry a rating says so rather than swallowing the keystroke. +static void ApplyRatingToCurrentImage(HWND hwnd, int stars) { + const std::wstring path = GetPaneContext(PaneSlot::Primary).path; + if (path.empty()) return; + + std::wstring rendered, raw; + ResolveRatingCarriers(path, rendered, raw); + + switch (RatingStore::GetWritability(rendered, raw)) { + case RatingStore::Writability::UnsupportedFormat: + g_osd.Show(hwnd, L"This format cannot store a rating", false); + return; + case RatingStore::Writability::ReadOnlyFile: + g_osd.Show(hwnd, L"File is read-only", false); + return; + case RatingStore::Writability::Writable: + break; + } + + // The photo on screen is held open for display, so a rebuild of it waits + // until the user navigates away (see RatingStore::ReleaseResident). + g_ratingStore.ApplyRatingOptimistic(FileNavigator::PathToImageID(path), stars, rendered, raw, + /*isResident*/ true); + + wchar_t osd[32]; + if (stars > 0) { + std::wstring bar; + for (int i = 0; i < QuickView::Rating::MAX_STARS; ++i) { + bar += (i < stars) ? L"\u2605" : L"\u2606"; + } + swprintf_s(osd, L"%s", bar.c_str()); + } else { + swprintf_s(osd, L"%s", L"\u2606\u2606\u2606\u2606\u2606"); + } + g_osd.Show(hwnd, osd, false); + + RequestRepaint(PaintLayer::Static | PaintLayer::Dynamic); + if (g_gallery.IsVisible()) RequestRepaint(PaintLayer::Gallery); +} + static void QueueRatingRead(const std::wstring& path) { if (path.empty()) return; const auto& nav = GetPaneContext(PaneSlot::Primary).navigator; @@ -17172,6 +17235,17 @@ bool HandleHotkeyAction(HWND hwnd, HotkeyAction action) { return true; } + case HotkeyAction::Rate0: + case HotkeyAction::Rate1: + case HotkeyAction::Rate2: + case HotkeyAction::Rate3: + case HotkeyAction::Rate4: + case HotkeyAction::Rate5: { + const int stars = (int)action - (int)HotkeyAction::Rate0; + ApplyRatingToCurrentImage(hwnd, stars); + return true; + } + case HotkeyAction::AlwaysOnTop: SendMessage(hwnd, WM_COMMAND, IDM_ALWAYS_ON_TOP, 0); return true; From d1bf8135c024aa5f8aca94272d6b0d8da2e60d46 Mon Sep 17 00:00:00 2001 From: Zhou Ying Date: Thu, 27 Aug 2026 11:19:15 +0800 Subject: [PATCH 5/5] feat(rating): store RAW ratings in an XMP sidecar (#201) Completes star ratings. A RAW keeps its rating in the same-name .xmp sidecar that Lightroom, Bridge and Capture One read, since the native stack cannot write a proprietary RAW. An existing sidecar carries that photo's develop settings, so the update is surgical: the xmp:Rating property is rewritten where it stands and everything else is copied through byte for byte -- a test pins this down by asserting that rating a Lightroom sidecar changes exactly one character. Both serializations are handled, clearing removes the property rather than storing a zero, and a document that is not shaped as expected is refused outright, because someone's develop settings are worth more than one rating. A sidecar too large to read whole is refused for the same reason: writing back a truncated document would destroy the rest of it. Only a RAW has one created for it; for a JPEG or TIFF an existing sidecar is updated but never brought into being, since a rating belongs inside those files. Which file owns the sidecar mirrors how reading resolves it. Reading probes the sidecar of whichever file is the rating's carrier, so writing must reach the same file -- a sidecar that is read but not written would keep overruling the rating the user just set, which is exactly what happened before this was made symmetric. For a folded pair the sidecar is written first and the in-file half second. The sidecar is the side that wins when the two disagree, so this order means a failure of the second write still leaves the user looking at the rating they set, rather than the old one coming back; a failure of the first leaves both files untouched. Sidecars are written through a temp file and swapped into place, so a failure cannot leave half a document where the original was. Since that creates a file in the watched folder, the watcher would otherwise rescan the directory for our own write; a notification carries no file name, so the navigator asks an injected predicate whether we wrote something a moment ago and skips the scan when we did. The predicate is injected rather than called directly to keep the navigator, and the test binary, free of the rating subsystem. --- QuickView/FileNavigator.cpp | 10 +++ QuickView/FileNavigator.h | 9 +++ QuickView/RatingMetadata.cpp | 97 +++++++++++++++++++++++++++ QuickView/RatingMetadata.h | 15 +++++ QuickView/RatingStore.cpp | 123 +++++++++++++++++++++++++++++++++- QuickView/RatingStore.h | 10 +++ QuickView/main.cpp | 2 + tests/RatingMetadataTests.cpp | 100 +++++++++++++++++++++++++++ 8 files changed, 364 insertions(+), 2 deletions(-) diff --git a/QuickView/FileNavigator.cpp b/QuickView/FileNavigator.cpp index f3a96c06..285c6182 100644 --- a/QuickView/FileNavigator.cpp +++ b/QuickView/FileNavigator.cpp @@ -1785,6 +1785,16 @@ void FileNavigator::WatcherThreadProc() { } if (cancelled) break; + // [Ratings] Writing a sidecar creates a file in this very folder, and + // rescanning because of our own write would be pure waste: an .xmp is + // not in the playlist, so neither the list nor the pairing can change. + // The notification carries no file name, so the only usable signal is + // that we wrote something a moment ago. + if (s_selfWriteProbe && s_selfWriteProbe()) { + if (!FindNextChangeNotification(hNotify)) break; + continue; + } + // Materialized playlists rescan; Explorer cursor just refreshes ItemCount. if (m_playlistReady.load() || m_needInitialScan.load()) { publishScan(); diff --git a/QuickView/FileNavigator.h b/QuickView/FileNavigator.h index 70e9e1ee..7b1dc0c0 100644 --- a/QuickView/FileNavigator.h +++ b/QuickView/FileNavigator.h @@ -198,6 +198,14 @@ class FileNavigator { // only) cannot parse: RAW via LibRaw, HEIF etc. via WIC. Injected at // startup because the unit-test binary links FileNavigator without // LibRaw/WIC. Returns 0 when unavailable. + // [Ratings] Injected predicate: true when QuickView itself wrote a file + // in the watched folder a moment ago, so a directory-change notification + // is its own echo and rescanning would be waste. Injected rather than + // called directly to keep the navigator (and the test binary) free of the + // rating subsystem. + using SelfWriteProbe = bool (*)(); + static void SetSelfWriteProbe(SelfWriteProbe probe) { s_selfWriteProbe = probe; } + using CaptureTimeFallbackReader = int64_t (*)(const wchar_t* path); static void SetCaptureTimeFallbackReader(CaptureTimeFallbackReader reader) { s_captureTimeFallback = reader; } @@ -247,6 +255,7 @@ class FileNavigator { std::wstring m_verifyDir; std::atomic m_verifyGeneration{ 0 }; // cancels superseded runs inline static CaptureTimeFallbackReader s_captureTimeFallback = nullptr; + inline static SelfWriteProbe s_selfWriteProbe = nullptr; int m_currentIndex = -1; bool m_hitEnd = false; std::wstring m_crossFolderMessage; diff --git a/QuickView/RatingMetadata.cpp b/QuickView/RatingMetadata.cpp index 1e95b949..4605a66a 100644 --- a/QuickView/RatingMetadata.cpp +++ b/QuickView/RatingMetadata.cpp @@ -190,6 +190,103 @@ std::optional ParseJpegRating(std::span bytes) { return fromXmp; } +namespace { + +// Locate an existing xmp:Rating and report the exact span to replace, which is +// what makes the update surgical: everything outside [start, end) is copied +// through untouched. +struct PropertySpan { + size_t start = 0; // first character of the whole property + size_t end = 0; // one past its last character + bool attributeForm = false; +}; + +std::optional FindRatingProperty(std::string_view xmp) { + constexpr std::string_view NAME = "xmp:Rating"; + + for (size_t pos = xmp.find(NAME); pos != std::string_view::npos; + pos = xmp.find(NAME, pos + NAME.size())) { + size_t cursor = pos + NAME.size(); + while (cursor < xmp.size() && (xmp[cursor] == ' ' || xmp[cursor] == '\t')) ++cursor; + if (cursor >= xmp.size()) break; + + if (xmp[cursor] == '=') { + // xmp:Rating="3" + ++cursor; + while (cursor < xmp.size() && (xmp[cursor] == ' ' || xmp[cursor] == '\t')) ++cursor; + if (cursor >= xmp.size()) break; + const char quote = xmp[cursor]; + if (quote != '"' && quote != '\'') continue; + const size_t close = xmp.find(quote, cursor + 1); + if (close == std::string_view::npos) continue; + return PropertySpan{ pos, close + 1, true }; + } + if (xmp[cursor] == '>') { + // 3: the span starts at the opening '<' + constexpr std::string_view CLOSE_TAG = ""; + const size_t close = xmp.find(CLOSE_TAG, cursor); + if (close == std::string_view::npos) continue; + const size_t openTag = xmp.rfind('<', pos); + if (openTag == std::string_view::npos) continue; + return PropertySpan{ openTag, close + CLOSE_TAG.size(), false }; + } + } + return std::nullopt; +} + +} // namespace + +std::optional UpdateXmpRating(std::string_view xmp, int stars) { + if (xmp.empty()) return std::nullopt; + + const std::string value = std::to_string(stars); + + if (const auto span = FindRatingProperty(xmp)) { + std::string out; + out.reserve(xmp.size() + 16); + out.append(xmp.substr(0, span->start)); + if (stars > MIN_STARS) { + out.append(span->attributeForm ? "xmp:Rating=\"" + value + "\"" + : "" + value + ""); + } + // stars == 0 drops the property entirely, which is what clearing means. + out.append(xmp.substr(span->end)); + return out; + } + + if (stars <= MIN_STARS) { + return std::string(xmp); // nothing to clear, leave the document as it is + } + + // Absent: add it as an attribute of the first rdf:Description, which is + // where Lightroom and Capture One keep it too. + constexpr std::string_view DESCRIPTION = " MIN_STARS ? stars : MIN_STARS); + return + "\n" + "\n" + " \n" + " \n" + " \n" + "\n" + "\n"; +} + Resolved ResolvePairRating(std::optional inFile, std::optional sidecar) { // Rejected (-1) is a deliberate mark, so it takes part in the resolution; // it is only flattened to 0 stars for display. diff --git a/QuickView/RatingMetadata.h b/QuickView/RatingMetadata.h index 38a01ae9..4ba164d3 100644 --- a/QuickView/RatingMetadata.h +++ b/QuickView/RatingMetadata.h @@ -21,6 +21,7 @@ #include #include #include +#include #include // Reading ratings must never touch the decode pipeline, so these parsers work @@ -59,6 +60,20 @@ std::optional ParseJpegRating(std::span bytes); // directly (no APP1 wrapper), so it needs its own entry point. std::optional ParseTiffRating(std::span bytes); +// Replace xmp:Rating inside an existing XMP document, touching nothing else. +// A sidecar written by Lightroom or Capture One carries the develop settings +// for that photo, so the update is a surgical edit of that one property and +// never a regeneration of the document. +// - the property is rewritten in place when present, in either serialization +// - it is inserted into the first rdf:Description when absent +// - `stars` == 0 removes it, leaving the rest of the document intact +// Returns nothing when the document is not shaped as expected, which the +// caller must treat as "refuse to write" rather than overwriting the file. +std::optional UpdateXmpRating(std::string_view xmp, int stars); + +// A minimal sidecar for a photo that has none yet. +std::string BuildMinimalXmp(int stars); + // Which file of a pair a displayed rating came from. enum class Source { None, InFile, Sidecar }; diff --git a/QuickView/RatingStore.cpp b/QuickView/RatingStore.cpp index 7a3dc837..d9834385 100644 --- a/QuickView/RatingStore.cpp +++ b/QuickView/RatingStore.cpp @@ -35,6 +35,26 @@ constexpr DWORD SIDECAR_READ_BYTES = 16 * 1024; // short enough that the file is up to date by the time the user looks. constexpr auto WRITE_DEBOUNCE = std::chrono::milliseconds(400); +// An update has to preserve the whole sidecar, so writing reads all of it, +// unlike the prefix that display needs. Anything larger than this is not a +// rating sidecar and is left alone. +constexpr DWORD MAX_SIDECAR_BYTES = 4 * 1024 * 1024; + +// How long after writing a file a directory-change notification is assumed to +// be the echo of that write. Long enough to cover the watcher's own debounce. +constexpr auto SELF_WRITE_ECHO = std::chrono::milliseconds(1500); +std::atomic g_lastSelfWriteTick{ 0 }; + +LARGE_INTEGER FileSizeOf(const std::wstring& path) { + LARGE_INTEGER size{}; + WIN32_FILE_ATTRIBUTE_DATA data{}; + if (GetFileAttributesExW(path.c_str(), GetFileExInfoStandard, &data)) { + size.HighPart = (LONG)data.nFileSizeHigh; + size.LowPart = data.nFileSizeLow; + } + return size; +} + // Wide-char file API throughout: a path round-tripped through a narrow code // page fails to open on non-ASCII names. std::vector ReadFilePrefix(const std::wstring& path, DWORD maxBytes) { @@ -212,11 +232,110 @@ void RatingStore::FlushPendingWrites() { for (const auto& write : due) PerformWrite(write); } +void RatingStore::NotifySelfWrite() { + g_lastSelfWriteTick.store( + std::chrono::steady_clock::now().time_since_epoch().count(), std::memory_order_relaxed); +} + +bool RatingStore::WasSelfWriteJustNow() { + const int64_t ticks = g_lastSelfWriteTick.load(std::memory_order_relaxed); + if (ticks == 0) return false; + const auto last = std::chrono::steady_clock::time_point( + std::chrono::steady_clock::duration(ticks)); + return (std::chrono::steady_clock::now() - last) < SELF_WRITE_ECHO; +} + +bool RatingStore::WriteSidecar(const std::wstring& ownerPath, int stars, bool allowCreate) { + const std::wstring sidecarPath = SidecarPathFor(ownerPath); + if (sidecarPath.empty()) return false; + + // The whole document is read, not just the prefix used for display: an + // update has to copy through everything it is not changing. A file too + // large to read whole would come back truncated, and writing that back + // would destroy the rest of it, so the write is refused instead. + std::string existing; + { + const LARGE_INTEGER size = FileSizeOf(sidecarPath); + if (size.QuadPart > (LONGLONG)MAX_SIDECAR_BYTES) return false; + const std::vector bytes = ReadFilePrefix(sidecarPath, MAX_SIDECAR_BYTES); + existing.assign(reinterpret_cast(bytes.data()), bytes.size()); + } + + std::string updated; + if (existing.empty()) { + if (!allowCreate) return true; // update-only owner + if (stars <= QuickView::Rating::MIN_STARS) return true; // nothing to write + updated = QuickView::Rating::BuildMinimalXmp(stars); + } else { + const auto edited = QuickView::Rating::UpdateXmpRating(existing, stars); + if (!edited) { + // The document is not shaped as expected. Someone else's develop + // settings are worth more than this rating, so leave it alone. + return false; + } + if (*edited == existing) return true; // already says what we want + updated = *edited; + } + + // Write through a temp file so a failure cannot leave a half-written + // sidecar where the original was. + const std::wstring tempPath = sidecarPath + L".qvtmp"; + { + HANDLE file = CreateFileW(tempPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, nullptr); + if (file == INVALID_HANDLE_VALUE) return false; + DWORD written = 0; + const BOOL ok = WriteFile(file, updated.data(), (DWORD)updated.size(), &written, nullptr); + FlushFileBuffers(file); + CloseHandle(file); + if (!ok || written != updated.size()) { + DeleteFileW(tempPath.c_str()); + return false; + } + } + + NotifySelfWrite(); // our own change; the watcher should not rescan for it + + if (existing.empty()) { + if (!MoveFileExW(tempPath.c_str(), sidecarPath.c_str(), MOVEFILE_REPLACE_EXISTING)) { + DeleteFileW(tempPath.c_str()); + return false; + } + return true; + } + if (!ReplaceFileW(sidecarPath.c_str(), tempPath.c_str(), nullptr, + REPLACEFILE_IGNORE_MERGE_ERRORS, nullptr, nullptr)) { + DeleteFileW(tempPath.c_str()); + return false; + } + return true; +} + void RatingStore::PerformWrite(const PendingWrite& write) { if (write.renderedPath.empty()) return; - // A folded pair keeps its rating in the RAW's sidecar as well; that half - // arrives with the sidecar stage. + // The sidecar goes first, because it is the side that wins when the two + // disagree: if the in-file write then fails, what the user sees is still + // the rating they set, rather than the old one coming back. + // + // Which file owns the sidecar mirrors how reading resolves it, and it has + // to: a sidecar that is read but not written would keep overruling the + // rating the user just set. A RAW may have one created for it, since that + // is the only place its rating can live; for anything else an existing + // sidecar is updated but never brought into being, because a rating + // belongs inside a JPEG or TIFF. + const std::wstring sidecarOwner = + write.rawPath.empty() ? write.renderedPath : write.rawPath; + const bool ownerIsRaw = QuickView::IsRawPath(sidecarOwner); + const bool sidecarExists = + GetFileAttributesW(SidecarPathFor(sidecarOwner).c_str()) != INVALID_FILE_ATTRIBUTES; + if (ownerIsRaw || sidecarExists) { + if (!WriteSidecar(sidecarOwner, write.stars, /*allowCreate*/ ownerIsRaw)) { + return; // refused: leave the in-file half alone as well + } + } + + // A standalone RAW has no in-file half to write. if (QuickView::IsRawPath(write.renderedPath)) return; const auto status = QuickView::Rating::WriteRatingToImage( diff --git a/QuickView/RatingStore.h b/QuickView/RatingStore.h index f921db49..79da8381 100644 --- a/QuickView/RatingStore.h +++ b/QuickView/RatingStore.h @@ -91,6 +91,12 @@ class RatingStore { // Write everything still pending, blocking until done (shutdown). void FlushPendingWrites(); + // Record that we just wrote a file ourselves, and ask whether that was + // recent enough that a directory-change notification is most likely our + // own doing. The watcher uses this to skip a rescan it does not need. + static void NotifySelfWrite(); + static bool WasSelfWriteJustNow(); + // The .xmp sidecar a RAW's rating lives in, i.e. the path with its // extension replaced. Empty when `path` has no extension. static std::wstring SidecarPathFor(const std::wstring& path); @@ -121,6 +127,10 @@ class RatingStore { void WorkerLoop(); void WriteLoop(); void PerformWrite(const PendingWrite& write); + // Update (or create) the .xmp sidecar carrying `ownerPath`'s rating. + // False means the write was refused or failed, and the caller must not + // treat the rating as stored. + static bool WriteSidecar(const std::wstring& ownerPath, int stars, bool allowCreate); HWND m_hwnd = nullptr; diff --git a/QuickView/main.cpp b/QuickView/main.cpp index 5de26ed4..9ce994c4 100644 --- a/QuickView/main.cpp +++ b/QuickView/main.cpp @@ -7593,6 +7593,8 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE, [[maybe_unused]] LPWSTR lpCm // Init Gallery g_thumbMgr.Initialize(hwnd, g_imageLoader.get()); g_ratingStore.Initialize(hwnd); + // Let the directory watcher recognise the echo of our own sidecar writes. + FileNavigator::SetSelfWriteProbe(&RatingStore::WasSelfWriteJustNow); g_gallery.Initialize(&g_thumbMgr, &GetPaneContext(PaneSlot::Primary).navigator); g_settingsOverlay.Init(g_renderEngine->GetDeviceContext(), hwnd); g_helpOverlay.Init(g_renderEngine->GetDeviceContext(), hwnd); diff --git a/tests/RatingMetadataTests.cpp b/tests/RatingMetadataTests.cpp index 316d06a1..9695cf45 100644 --- a/tests/RatingMetadataTests.cpp +++ b/tests/RatingMetadataTests.cpp @@ -250,3 +250,103 @@ TEST(RatingMetadataTest, PairRejectedIsDisplayedAsZeroStarsButStillConflicts) { EXPECT_TRUE(r.conflict); EXPECT_EQ(r.otherStars, 4); } + +// --- UpdateXmpRating ------------------------------------------------------- + +namespace { + +// Shaped like a real Lightroom sidecar: the rating sits among develop +// settings that an update must not disturb. +const char* const LIGHTROOM_SIDECAR = + "\n" + "\n" + " \n" + " \n" + " \n" + "\n" + "\n"; + +} // namespace + +TEST(RatingMetadataTest, XmpUpdateKeepsDevelopSettings) { + const auto out = UpdateXmpRating(LIGHTROOM_SIDECAR, 2); + ASSERT_TRUE(out.has_value()); + EXPECT_EQ(ParseXmpRating(*out), 2); + // Everything the photographer actually cares about must survive verbatim. + EXPECT_NE(out->find("crs:Exposure2012=\"+0.35\""), std::string::npos); + EXPECT_NE(out->find("crs:Contrast2012=\"+12\""), std::string::npos); + EXPECT_NE(out->find("xmlns:crs="), std::string::npos); + EXPECT_NE(out->find(""), std::string::npos); +} + +TEST(RatingMetadataTest, XmpUpdateRewritesOnlyTheRating) { + const auto out = UpdateXmpRating(LIGHTROOM_SIDECAR, 5); + ASSERT_TRUE(out.has_value()); + const std::string before(LIGHTROOM_SIDECAR); + // The documents differ by one character: the rating digit. + EXPECT_EQ(out->size(), before.size()); + size_t differing = 0; + for (size_t i = 0; i < before.size(); ++i) { + if ((*out)[i] != before[i]) ++differing; + } + EXPECT_EQ(differing, 1u); +} + +TEST(RatingMetadataTest, XmpUpdateClearingRemovesTheProperty) { + const auto out = UpdateXmpRating(LIGHTROOM_SIDECAR, 0); + ASSERT_TRUE(out.has_value()); + EXPECT_FALSE(ParseXmpRating(*out).has_value()); + EXPECT_EQ(out->find("xmp:Rating"), std::string::npos); + EXPECT_NE(out->find("crs:Exposure2012=\"+0.35\""), std::string::npos); +} + +TEST(RatingMetadataTest, XmpUpdateInsertsWhenAbsent) { + const std::string noRating = + "\n" + " \n" + " \n" + " \n" + "\n"; + const auto out = UpdateXmpRating(noRating, 3); + ASSERT_TRUE(out.has_value()); + EXPECT_EQ(ParseXmpRating(*out), 3); + EXPECT_NE(out->find("crs:Contrast2012=\"+12\""), std::string::npos); +} + +TEST(RatingMetadataTest, XmpUpdateHandlesElementForm) { + const std::string elementForm = + "\n" + " 1\n" + " keep me\n" + "\n"; + const auto out = UpdateXmpRating(elementForm, 4); + ASSERT_TRUE(out.has_value()); + EXPECT_EQ(ParseXmpRating(*out), 4); + EXPECT_NE(out->find("keep me"), std::string::npos); +} + +TEST(RatingMetadataTest, XmpUpdateRefusesUnfamiliarDocument) { + // No rdf:Description to attach to: refusing is the only safe answer, since + // the alternative is overwriting a file we do not understand. + EXPECT_FALSE(UpdateXmpRating("just some text", 3).has_value()); + EXPECT_FALSE(UpdateXmpRating("", 3).has_value()); +} + +TEST(RatingMetadataTest, XmpUpdateClearingAnUnratedDocumentIsANoOp) { + const std::string doc = ""; + const auto out = UpdateXmpRating(doc, 0); + ASSERT_TRUE(out.has_value()); + EXPECT_EQ(*out, doc); +} + +TEST(RatingMetadataTest, MinimalSidecarRoundTrips) { + for (int stars = 1; stars <= MAX_STARS; ++stars) { + const std::string xmp = BuildMinimalXmp(stars); + EXPECT_EQ(ParseXmpRating(xmp), stars); + } +}