From 8b460642b566438713b61a777fe87b68454a1436 Mon Sep 17 00:00:00 2001 From: Fernando Date: Sun, 30 Aug 2026 22:36:46 -0400 Subject: [PATCH 1/7] =?UTF-8?q?feat(#551):=20Slice=20H=20part=201=20?= =?UTF-8?q?=E2=80=94=20colour=20palette=20library=20(pure=20data)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First piece of Paint v2 Slice H. Ogre-free core for colour swatches, modelled closely on GradientRamp (#544): same /paint// layout, same JSON + safe-stem + hashed-filename conventions, bundled entries defined in C++. - 6 bundled palettes (Material Design, Pantone Classics, Skin Tones, Foliage Greens, Sky Blues, Earth Tones) defined in code rather than shipped as data files, so they cannot go missing from an install. - JSON persistence to /paint/palettes/, with a hashed filename suffix so two names that sanitise to the same stem cannot overwrite each other, and a truncated write is removed rather than left behind. - `allPalettes()` lets a custom palette override a bundled one of the same name — the precedence BrushAssetLibrary::resolvePath already uses for stamps. - `pushRecent()` for the recent-colours ring: re-picking a colour PROMOTES it instead of appending a duplicate, so the ring stays 12 distinct colours. - `extractFromImage()` for "save palette from texture": coarse colour-cube quantisation (5 bits/channel) so a photographic texture yields ten visibly different swatches rather than ten near-identical ones, averaging the real pixels in each bucket so every swatch is a colour that actually occurs. Fully transparent pixels are skipped — otherwise a mostly-empty texture reports "transparent black" as its dominant colour. Two deliberate choices worth noting: - `Swatch` carries no alpha. A palette curates hues; paint alpha is a separate brush property, and baking it into swatches would silently override the user's setting on every pick. - Malformed hex is REJECTED rather than defaulting to black, so a corrupt file drops the bad entry instead of rendering a row of invisible swatches. One bad entry does not discard the palette's other swatches. Registered in BOTH src/CMakeLists.txt and tests/CMakeLists.txt. The tests library keeps its own explicit source list, and registering in only one of them is what broke CI on the previous slice. Tests: 16/16. Filesystem cases isolate via QStandardPaths::setTestModeEnabled and clean up in TearDown, so they never touch (or read stale state from) a real install. Mutation-checked the two guarantees most likely to rot: dropping the transparent-pixel skip and dropping the recent-ring de-dup each fail their test. Tablet support (the third part of #551) is deliberately NOT included — the project is desktop-only for now, so pressure/tilt would be unverifiable. Co-Authored-By: Claude Opus 5 (1M context) --- src/CMakeLists.txt | 2 + src/ColorPaletteLibrary.cpp | 344 +++++++++++++++++++++++++++++++ src/ColorPaletteLibrary.h | 88 ++++++++ src/ColorPaletteLibrary_test.cpp | 246 ++++++++++++++++++++++ tests/CMakeLists.txt | 1 + 5 files changed, 681 insertions(+) create mode 100644 src/ColorPaletteLibrary.cpp create mode 100644 src/ColorPaletteLibrary.h create mode 100644 src/ColorPaletteLibrary_test.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index b0f5867e..be59e2f7 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -158,6 +158,7 @@ PaintLayerBlend.cpp PaintLayerStack.cpp PaintSelectionMask.cpp GradientRamp.cpp +ColorPaletteLibrary.cpp BrushEngine.cpp BrushFootprint.cpp BrushAssetLibrary.cpp @@ -390,6 +391,7 @@ PaintChannel.h SymmetryMirrorMap.h PaintSelectionMask.h GradientRamp.h +ColorPaletteLibrary.h BrushEngine.h TexturePaintBuffer.h UpdateVersion.h diff --git a/src/ColorPaletteLibrary.cpp b/src/ColorPaletteLibrary.cpp new file mode 100644 index 00000000..fbe7f604 --- /dev/null +++ b/src/ColorPaletteLibrary.cpp @@ -0,0 +1,344 @@ +#include "ColorPaletteLibrary.h" + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace ColorPaletteLibrary { + +namespace { + +Swatch rgb(int r, int g, int b) +{ + Swatch s; + s.r = static_cast(std::clamp(r, 0, 255)); + s.g = static_cast(std::clamp(g, 0, 255)); + s.b = static_cast(std::clamp(b, 0, 255)); + return s; +} + +int hexDigit(char c) +{ + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return 10 + (c - 'a'); + if (c >= 'A' && c <= 'F') return 10 + (c - 'A'); + return -1; +} + +/// Custom palettes get a hashed suffix so two names that sanitise to the same +/// stem ("Sky Blues" / "sky/blues") cannot overwrite each other. +std::string customFileStem(const std::string& name) +{ + const size_t h = std::hash{}(name); + char buf[16]; + std::snprintf(buf, sizeof(buf), "_%08x", static_cast(h & 0xffffffffu)); + return safeFileStem(name) + buf; +} + +} // namespace + +bool swatchFromHex(const std::string& hex, Swatch& out) +{ + std::string h = hex; + if (!h.empty() && h.front() == '#') h.erase(h.begin()); + if (h.size() != 6) return false; + int v[6]; + for (int i = 0; i < 6; ++i) { + v[i] = hexDigit(h[static_cast(i)]); + if (v[i] < 0) return false; + } + out.r = static_cast(v[0] * 16 + v[1]); + out.g = static_cast(v[2] * 16 + v[3]); + out.b = static_cast(v[4] * 16 + v[5]); + return true; +} + +std::string swatchToHex(const Swatch& s) +{ + char buf[8]; + std::snprintf(buf, sizeof(buf), "#%02x%02x%02x", s.r, s.g, s.b); + return std::string(buf); +} + +std::vector bundledPalettes() +{ + std::vector out; + + // All six are CC0 / factual colour values (Material Design's published + // palette, common Pantone-classic hues, and observational skin/foliage/sky/ + // earth ranges) — no third-party asset files ship with them. + out.push_back({"Material Design", { + rgb(0xF4, 0x43, 0x36), rgb(0xE9, 0x1E, 0x63), rgb(0x9C, 0x27, 0xB0), + rgb(0x67, 0x3A, 0xB7), rgb(0x3F, 0x51, 0xB5), rgb(0x21, 0x96, 0xF3), + rgb(0x03, 0xA9, 0xF4), rgb(0x00, 0xBC, 0xD4), rgb(0x00, 0x96, 0x88), + rgb(0x4C, 0xAF, 0x50), rgb(0x8B, 0xC3, 0x4A), rgb(0xCD, 0xDC, 0x39), + rgb(0xFF, 0xEB, 0x3B), rgb(0xFF, 0xC1, 0x07), rgb(0xFF, 0x98, 0x00), + rgb(0xFF, 0x57, 0x22), rgb(0x79, 0x55, 0x48), rgb(0x9E, 0x9E, 0x9E), + rgb(0x60, 0x7D, 0x8B), rgb(0x21, 0x21, 0x21), + }}); + + out.push_back({"Pantone Classics", { + rgb(0xC7, 0x40, 0x75), rgb(0x93, 0x9A, 0xC0), rgb(0xFF, 0x6F, 0x61), + rgb(0x5F, 0x4B, 0x8B), rgb(0x88, 0xB0, 0x4B), rgb(0x0F, 0x4C, 0x81), + rgb(0x93, 0x4F, 0x5E), rgb(0xF5, 0xDF, 0x4D), rgb(0x93, 0x93, 0x96), + rgb(0xBB, 0x25, 0x28), + }}); + + out.push_back({"Skin Tones", { + rgb(0xFF, 0xE0, 0xC9), rgb(0xF6, 0xC9, 0xA8), rgb(0xE8, 0xB0, 0x8D), + rgb(0xD9, 0x99, 0x77), rgb(0xC1, 0x7F, 0x5E), rgb(0xA3, 0x66, 0x47), + rgb(0x84, 0x51, 0x36), rgb(0x63, 0x3C, 0x28), rgb(0x45, 0x29, 0x1B), + rgb(0x2B, 0x19, 0x10), + }}); + + out.push_back({"Foliage Greens", { + rgb(0xE4, 0xF0, 0xC2), rgb(0xC4, 0xDE, 0x8E), rgb(0x9C, 0xC7, 0x5B), + rgb(0x76, 0xA9, 0x3A), rgb(0x55, 0x8B, 0x2F), rgb(0x3E, 0x6E, 0x24), + rgb(0x2C, 0x54, 0x1B), rgb(0x1D, 0x3D, 0x14), rgb(0x6B, 0x7F, 0x3A), + rgb(0x8F, 0x9E, 0x4C), + }}); + + out.push_back({"Sky Blues", { + rgb(0xEA, 0xF6, 0xFF), rgb(0xC7, 0xE7, 0xFB), rgb(0x9E, 0xD2, 0xF6), + rgb(0x73, 0xB9, 0xEC), rgb(0x4A, 0x9D, 0xDE), rgb(0x2E, 0x7F, 0xC4), + rgb(0x1F, 0x62, 0xA1), rgb(0x16, 0x47, 0x78), rgb(0xB5, 0xC7, 0xD8), + rgb(0xF7, 0xC9, 0x9B), + }}); + + out.push_back({"Earth Tones", { + rgb(0xE8, 0xD9, 0xC0), rgb(0xD3, 0xBC, 0x9A), rgb(0xBB, 0x9E, 0x77), + rgb(0xA1, 0x82, 0x5C), rgb(0x84, 0x67, 0x45), rgb(0x68, 0x4F, 0x34), + rgb(0x4D, 0x39, 0x25), rgb(0x8C, 0x6B, 0x52), rgb(0xA9, 0x7C, 0x50), + rgb(0x6E, 0x5A, 0x47), + }}); + + return out; +} + +const Palette* findBundled(const std::string& name) +{ + // Function-local static: stable addresses for the returned pointer, built + // once. Same pattern as GradientRamp::findBundled. + static const std::vector kBundled = bundledPalettes(); + for (const auto& p : kBundled) + if (p.name == name) return &p; + return nullptr; +} + +bool isBundled(const std::string& name) +{ + return findBundled(name) != nullptr; +} + +std::string toJson(const Palette& p) +{ + QJsonObject root; + root["name"] = QString::fromStdString(p.name); + QJsonArray arr; + for (const auto& s : p.swatches) + arr.append(QString::fromStdString(swatchToHex(s))); + root["swatches"] = arr; + return QJsonDocument(root).toJson(QJsonDocument::Compact).toStdString(); +} + +bool fromJson(const std::string& json, Palette& out) +{ + QJsonParseError err{}; + const QJsonDocument doc = + QJsonDocument::fromJson(QByteArray::fromStdString(json), &err); + if (err.error != QJsonParseError::NoError || !doc.isObject()) return false; + const QJsonObject root = doc.object(); + + Palette p; + p.name = root.value("name").toString().toStdString(); + if (p.name.empty()) return false; + const QJsonArray arr = root.value("swatches").toArray(); + for (const auto& v : arr) { + Swatch s; + // Skip malformed entries rather than rejecting the whole palette: one + // bad hex string should not lose the user's other swatches. + if (swatchFromHex(v.toString().toStdString(), s)) p.swatches.push_back(s); + } + if (p.swatches.empty()) return false; + out = std::move(p); + return true; +} + +std::string palettesDirectory() +{ + // QStandardPaths needs an application instance for AppDataLocation. + if (!QCoreApplication::instance()) return {}; + const QString base = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + if (base.isEmpty()) return {}; + const QString dir = QDir(base).filePath(QStringLiteral("paint/palettes")); + QDir().mkpath(dir); + return dir.toStdString(); +} + +std::string safeFileStem(const std::string& name) +{ + std::string out; + out.reserve(name.size()); + for (const char c : name) { + if (std::isalnum(static_cast(c)) || c == '-' || c == '_') + out.push_back(c); + else if (c == ' ') + out.push_back('_'); + } + if (out.empty()) out = "palette"; + return out; +} + +std::string saveCustom(const Palette& p) +{ + if (!p.isValid()) return {}; + const std::string dir = palettesDirectory(); + if (dir.empty()) return {}; + const QString path = QDir(QString::fromStdString(dir)) + .filePath(QString::fromStdString(customFileStem(p.name)) + + QStringLiteral(".json")); + QFile f(path); + if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return {}; + const std::string json = toJson(p); + if (f.write(json.data(), static_cast(json.size())) + != static_cast(json.size())) { + f.close(); + QFile::remove(path); // never leave a truncated palette behind + return {}; + } + f.close(); + return path.toStdString(); +} + +std::vector loadCustomPalettes() +{ + std::vector out; + const std::string dir = palettesDirectory(); + if (dir.empty()) return out; + QDir d(QString::fromStdString(dir)); + const QStringList files = + d.entryList({QStringLiteral("*.json")}, QDir::Files, QDir::Name); + for (const QString& fn : files) { + QFile f(d.filePath(fn)); + if (!f.open(QIODevice::ReadOnly)) continue; + const QByteArray data = f.readAll(); + f.close(); + Palette p; + if (fromJson(data.toStdString(), p)) out.push_back(std::move(p)); + } + return out; +} + +bool deleteCustom(const std::string& name) +{ + const std::string dir = palettesDirectory(); + if (dir.empty()) return false; + QDir d(QString::fromStdString(dir)); + + // Preferred: the hashed stem this library writes. + const QString direct = d.filePath(QString::fromStdString(customFileStem(name)) + + QStringLiteral(".json")); + if (QFile::exists(direct)) return QFile::remove(direct); + + // Fall back to matching the embedded name, so hand-authored or legacy files + // (written before the hashed stem) are still deletable from the UI. + const QStringList files = + d.entryList({QStringLiteral("*.json")}, QDir::Files, QDir::Name); + for (const QString& fn : files) { + QFile f(d.filePath(fn)); + if (!f.open(QIODevice::ReadOnly)) continue; + const QByteArray data = f.readAll(); + f.close(); + Palette p; + if (fromJson(data.toStdString(), p) && p.name == name) + return QFile::remove(d.filePath(fn)); + } + return false; +} + +std::vector allPalettes() +{ + std::vector out = bundledPalettes(); + for (auto& custom : loadCustomPalettes()) { + // Custom overrides a bundled palette of the same name — the same + // precedence BrushAssetLibrary::resolvePath applies to stamps, so a user + // can tweak a bundled palette without losing the ability to reset. + auto it = std::find_if(out.begin(), out.end(), + [&](const Palette& p) { return p.name == custom.name; }); + if (it != out.end()) *it = std::move(custom); + else out.push_back(std::move(custom)); + } + return out; +} + +void pushRecent(std::vector& recent, const Swatch& s, size_t maxCount) +{ + if (maxCount == 0) { recent.clear(); return; } + // Remove an existing copy first so re-picking a colour promotes it to the + // front instead of filling the ring with duplicates. + recent.erase(std::remove(recent.begin(), recent.end(), s), recent.end()); + recent.insert(recent.begin(), s); + if (recent.size() > maxCount) recent.resize(maxCount); +} + +std::vector extractFromImage(const uint8_t* rgba, int width, int height, + int maxColours) +{ + std::vector out; + if (!rgba || width <= 0 || height <= 0 || maxColours <= 0) return out; + + // Quantise into a coarse colour cube and count occupancy. 5 bits/channel + // (32 levels) groups near-identical shades — a photographic texture has + // thousands of unique RGB values, so counting exact colours would return + // ten imperceptibly different swatches. + constexpr int kShift = 3; // 8 -> 5 bits + std::map counts; + std::map> sums; + const size_t n = static_cast(width) * static_cast(height); + for (size_t i = 0; i < n; ++i) { + const uint8_t* px = rgba + i * 4; + if (px[3] == 0) continue; // fully transparent carries no colour + const uint32_t key = (uint32_t(px[0] >> kShift) << 10) + | (uint32_t(px[1] >> kShift) << 5) + | (uint32_t(px[2] >> kShift)); + ++counts[key]; + auto& acc = sums[key]; + acc[0] += px[0]; acc[1] += px[1]; acc[2] += px[2]; + } + if (counts.empty()) return out; + + std::vector> ordered(counts.begin(), counts.end()); + std::sort(ordered.begin(), ordered.end(), + [](const auto& a, const auto& b) { + // Most frequent first; tie-break on the key so the result is + // deterministic rather than dependent on map iteration. + if (a.second != b.second) return a.second > b.second; + return a.first < b.first; + }); + + const size_t take = std::min(ordered.size(), static_cast(maxColours)); + out.reserve(take); + for (size_t i = 0; i < take; ++i) { + const uint32_t key = ordered[i].first; + const uint32_t c = ordered[i].second; + const auto& acc = sums[key]; + // Average the real pixels in the bucket rather than the bucket centre, + // so the swatch is a colour that actually occurs in the image. + out.push_back(rgb(static_cast(acc[0] / c), + static_cast(acc[1] / c), + static_cast(acc[2] / c))); + } + return out; +} + +} // namespace ColorPaletteLibrary diff --git a/src/ColorPaletteLibrary.h b/src/ColorPaletteLibrary.h new file mode 100644 index 00000000..4adaa284 --- /dev/null +++ b/src/ColorPaletteLibrary.h @@ -0,0 +1,88 @@ +#ifndef COLOR_PALETTE_LIBRARY_H +#define COLOR_PALETTE_LIBRARY_H + +#include +#include +#include + +/** + * @brief Paint v2 Slice H (#551) — colour swatches / palettes (Ogre-free). + * + * A palette is a named, ordered list of RGB swatches for curated reuse, plus a + * "recent colours" ring the painter feeds as the user picks colours. + * + * Bundled palettes are defined in C++ (not shipped as data files) so they + * cannot go missing from an install; custom palettes serialize as JSON into + * `/paint/palettes/`. This mirrors GradientRamp (#544) exactly — see + * that file for the same directory/JSON/safe-stem conventions. + * + * Pure data. Qt is used only in the .cpp for JSON I/O and AppData paths, so the + * colour maths stays unit-testable headlessly. + */ +namespace ColorPaletteLibrary { + +/// 8-bit RGB swatch. Alpha is deliberately absent: a palette curates HUES, and +/// the paint alpha is a brush property the user sets independently — baking +/// alpha into swatches would silently override it on every pick. +struct Swatch { + uint8_t r = 0; + uint8_t g = 0; + uint8_t b = 0; + + bool operator==(const Swatch& o) const { return r == o.r && g == o.g && b == o.b; } +}; + +struct Palette { + std::string name; + std::vector swatches; + + bool isValid() const { return !name.empty() && !swatches.empty(); } +}; + +/// Parse "#rrggbb" (or "rrggbb"). Returns false on malformed input rather than +/// silently yielding black, so a corrupt file is a skipped entry, not a row of +/// invisible swatches. +bool swatchFromHex(const std::string& hex, Swatch& out); +/// Lower-case "#rrggbb". +std::string swatchToHex(const Swatch& s); + +/// The six bundled CC0 palettes required by #551. +std::vector bundledPalettes(); +/// Look up a bundled palette by exact name; nullptr when unknown. +const Palette* findBundled(const std::string& name); +/// True when `name` matches a bundled palette (drives the rename/delete gate). +bool isBundled(const std::string& name); + +/// JSON schema: `{ "name": "...", "swatches": ["#rrggbb", ...] }`. +std::string toJson(const Palette& p); +bool fromJson(const std::string& json, Palette& out); + +/// `/paint/palettes`, created on demand. Empty when unavailable. +std::string palettesDirectory(); +/// Write a custom palette; returns the file path, or empty on failure. +std::string saveCustom(const Palette& p); +/// Every parseable custom palette, name-sorted. Unparseable files are skipped. +std::vector loadCustomPalettes(); +bool deleteCustom(const std::string& name); + +/// Bundled + custom, with custom overriding a bundled palette of the same name +/// (the same precedence BrushAssetLibrary::resolvePath uses for stamps). +std::vector allPalettes(); + +/// Filesystem-safe stem for a palette name. +std::string safeFileStem(const std::string& name); + +/// Push `s` onto the front of `recent`, de-duplicating and capping at `maxCount` +/// (most-recent first). Pure list maths, exposed for unit tests. +void pushRecent(std::vector& recent, const Swatch& s, size_t maxCount = 12); + +/// Extract up to `maxColours` representative swatches from an RGBA8 image +/// (row-major, 4 bytes/px) by uniform colour-cube quantisation, ordered most +/// frequent first. Fully transparent pixels are ignored — they carry no colour +/// and would otherwise dominate a mostly-empty texture. +std::vector extractFromImage(const uint8_t* rgba, int width, int height, + int maxColours = 10); + +} // namespace ColorPaletteLibrary + +#endif // COLOR_PALETTE_LIBRARY_H diff --git a/src/ColorPaletteLibrary_test.cpp b/src/ColorPaletteLibrary_test.cpp new file mode 100644 index 00000000..1c054ac3 --- /dev/null +++ b/src/ColorPaletteLibrary_test.cpp @@ -0,0 +1,246 @@ +/* +----------------------------------------------------------------------------------- +A QtMeshEditor file — ColorPaletteLibrary unit tests (Paint v2 Slice H, issue #551) + +Pure-data: hex parsing, the bundled catalogue, JSON round-trip, the recent-colours +ring, and image colour extraction. Filesystem cases redirect via +QStandardPaths test mode so they never touch a real install. + +Copyright (c) Fernando Tonon (https://github.com/fernandotonon) +The MIT License — see other project sources for the full header. +----------------------------------------------------------------------------------- +*/ +#include + +#include "ColorPaletteLibrary.h" + +#include +#include + +#include + +using ColorPaletteLibrary::Palette; +using ColorPaletteLibrary::Swatch; + +namespace { +Swatch mk(int r, int g, int b) +{ + Swatch s; + s.r = static_cast(r); s.g = static_cast(g); s.b = static_cast(b); + return s; +} +} // namespace + +// --- hex ------------------------------------------------------------------- + +TEST(ColorPaletteLibraryTest, HexRoundTrips) { + Swatch s; + ASSERT_TRUE(ColorPaletteLibrary::swatchFromHex("#4CAF50", s)); + EXPECT_EQ(s.r, 0x4C); EXPECT_EQ(s.g, 0xAF); EXPECT_EQ(s.b, 0x50); + EXPECT_EQ(ColorPaletteLibrary::swatchToHex(s), "#4caf50"); + + // The leading '#' is optional and case does not matter. + Swatch t; + ASSERT_TRUE(ColorPaletteLibrary::swatchFromHex("4caf50", t)); + EXPECT_TRUE(s == t); +} + +TEST(ColorPaletteLibraryTest, MalformedHexIsRejectedNotSilentlyBlack) { + // Returning false (rather than black) is what lets a corrupt file drop the + // bad entry instead of showing a row of invisible swatches. + Swatch s; + for (const char* bad : {"", "#", "#12345", "#1234567", "#gg0000", "zzzzzz", "#12 34 56"}) + EXPECT_FALSE(ColorPaletteLibrary::swatchFromHex(bad, s)) << bad; +} + +// --- bundled catalogue ----------------------------------------------------- + +TEST(ColorPaletteLibraryTest, ShipsAtLeastSixBundledPalettes) { + const auto all = ColorPaletteLibrary::bundledPalettes(); + EXPECT_GE(all.size(), 6u) << "#551 requires at least 6 bundled palettes"; + for (const auto& p : all) { + EXPECT_TRUE(p.isValid()) << p.name; + EXPECT_FALSE(p.swatches.empty()) << p.name; + } +} + +TEST(ColorPaletteLibraryTest, BundledNamesAreUniqueAndFindable) { + const auto all = ColorPaletteLibrary::bundledPalettes(); + for (const auto& p : all) { + const Palette* found = ColorPaletteLibrary::findBundled(p.name); + ASSERT_NE(found, nullptr) << p.name; + EXPECT_EQ(found->swatches.size(), p.swatches.size()) << p.name; + EXPECT_TRUE(ColorPaletteLibrary::isBundled(p.name)) << p.name; + } + // Names double as the custom-override key, so duplicates would make one + // bundled palette unreachable. + for (size_t i = 0; i < all.size(); ++i) + for (size_t j = i + 1; j < all.size(); ++j) + EXPECT_NE(all[i].name, all[j].name); +} + +TEST(ColorPaletteLibraryTest, UnknownNameIsNotBundled) { + EXPECT_EQ(ColorPaletteLibrary::findBundled("No Such Palette"), nullptr); + EXPECT_FALSE(ColorPaletteLibrary::isBundled("No Such Palette")); +} + +// --- JSON ------------------------------------------------------------------ + +TEST(ColorPaletteLibraryTest, JsonRoundTripPreservesSwatches) { + Palette p; + p.name = "Test Palette"; + p.swatches = {mk(255, 0, 0), mk(0, 128, 64), mk(1, 2, 3)}; + + Palette back; + ASSERT_TRUE(ColorPaletteLibrary::fromJson(ColorPaletteLibrary::toJson(p), back)); + EXPECT_EQ(back.name, p.name); + ASSERT_EQ(back.swatches.size(), p.swatches.size()); + for (size_t i = 0; i < p.swatches.size(); ++i) + EXPECT_TRUE(back.swatches[i] == p.swatches[i]) << i; +} + +TEST(ColorPaletteLibraryTest, JsonRejectsUnusableInputButKeepsGoodSwatches) { + Palette out; + EXPECT_FALSE(ColorPaletteLibrary::fromJson("not json at all", out)); + EXPECT_FALSE(ColorPaletteLibrary::fromJson("[]", out)); + EXPECT_FALSE(ColorPaletteLibrary::fromJson(R"({"swatches":["#ff0000"]})", out)) + << "a palette with no name is unusable"; + EXPECT_FALSE(ColorPaletteLibrary::fromJson(R"({"name":"Empty","swatches":[]})", out)) + << "a palette with no swatches is unusable"; + + // One bad entry must not lose the user's other swatches. + ASSERT_TRUE(ColorPaletteLibrary::fromJson( + R"({"name":"Mixed","swatches":["#ff0000","nope","#0000ff"]})", out)); + EXPECT_EQ(out.swatches.size(), 2u); +} + +// --- recent colours -------------------------------------------------------- + +TEST(ColorPaletteLibraryTest, RecentPushesFrontAndCaps) { + std::vector recent; + for (int i = 0; i < 20; ++i) + ColorPaletteLibrary::pushRecent(recent, mk(i, i, i), 12); + ASSERT_EQ(recent.size(), 12u) << "must cap at maxCount"; + EXPECT_TRUE(recent.front() == mk(19, 19, 19)) << "most recent first"; + EXPECT_TRUE(recent.back() == mk(8, 8, 8)); +} + +TEST(ColorPaletteLibraryTest, RecentPromotesInsteadOfDuplicating) { + std::vector recent; + ColorPaletteLibrary::pushRecent(recent, mk(1, 1, 1)); + ColorPaletteLibrary::pushRecent(recent, mk(2, 2, 2)); + ColorPaletteLibrary::pushRecent(recent, mk(1, 1, 1)); // re-pick + ASSERT_EQ(recent.size(), 2u) << "re-picking must promote, not duplicate"; + EXPECT_TRUE(recent[0] == mk(1, 1, 1)); + EXPECT_TRUE(recent[1] == mk(2, 2, 2)); +} + +TEST(ColorPaletteLibraryTest, RecentWithZeroCapClears) { + std::vector recent{mk(9, 9, 9)}; + ColorPaletteLibrary::pushRecent(recent, mk(1, 1, 1), 0); + EXPECT_TRUE(recent.empty()); +} + +// --- extraction ------------------------------------------------------------ + +TEST(ColorPaletteLibraryTest, ExtractFindsDominantColoursMostFrequentFirst) { + // 4x1 image: three red pixels, one blue. + const std::vector px = { + 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 0, 0, 255, 255, + }; + const auto got = ColorPaletteLibrary::extractFromImage(px.data(), 4, 1, 10); + ASSERT_GE(got.size(), 2u); + EXPECT_TRUE(got[0] == mk(255, 0, 0)) << "the majority colour must come first"; +} + +TEST(ColorPaletteLibraryTest, ExtractIgnoresFullyTransparentPixels) { + // Mostly transparent with two opaque green pixels — a texture with a large + // empty region must not yield "transparent black" as its main colour. + std::vector px(4 * 10, 0); // 10 px, all alpha 0 + px[0] = 0; px[1] = 200; px[2] = 0; px[3] = 255; // one opaque green + px[4] = 0; px[5] = 200; px[6] = 0; px[7] = 255; // another + const auto got = ColorPaletteLibrary::extractFromImage(px.data(), 10, 1, 5); + ASSERT_EQ(got.size(), 1u); + EXPECT_TRUE(got[0] == mk(0, 200, 0)); +} + +TEST(ColorPaletteLibraryTest, ExtractRespectsMaxAndHandlesDegenerateInput) { + std::vector px(4 * 64, 255); + for (int i = 0; i < 64; ++i) { // 64 distinct-ish colours + px[i * 4 + 0] = static_cast(i * 4); + px[i * 4 + 1] = static_cast(255 - i * 4); + px[i * 4 + 2] = 128; + px[i * 4 + 3] = 255; + } + EXPECT_LE(ColorPaletteLibrary::extractFromImage(px.data(), 64, 1, 5).size(), 5u); + EXPECT_TRUE(ColorPaletteLibrary::extractFromImage(nullptr, 4, 4, 5).empty()); + EXPECT_TRUE(ColorPaletteLibrary::extractFromImage(px.data(), 0, 0, 5).empty()); + EXPECT_TRUE(ColorPaletteLibrary::extractFromImage(px.data(), 64, 1, 0).empty()); +} + +// --- persistence (isolated from the real ) ------------------------ + +class ColorPaletteLibraryFileTest : public ::testing::Test +{ +protected: + void SetUp() override { QStandardPaths::setTestModeEnabled(true); } + void TearDown() override + { + // Remove anything the test wrote before leaving test mode, so runs stay + // independent (a leftover palette would make a later "must not reload" + // assertion fail for the wrong reason). + const std::string dir = ColorPaletteLibrary::palettesDirectory(); + if (!dir.empty()) QDir(QString::fromStdString(dir)).removeRecursively(); + QStandardPaths::setTestModeEnabled(false); + } +}; + +TEST_F(ColorPaletteLibraryFileTest, SaveLoadDeleteCustomRoundTrip) { + Palette p; + p.name = "My Custom Palette"; + p.swatches = {mk(10, 20, 30), mk(40, 50, 60)}; + + const std::string path = ColorPaletteLibrary::saveCustom(p); + ASSERT_FALSE(path.empty()); + + const auto loaded = ColorPaletteLibrary::loadCustomPalettes(); + bool found = false; + for (const auto& q : loaded) + if (q.name == p.name && q.swatches.size() == 2u) found = true; + EXPECT_TRUE(found) << "a saved palette must load back"; + + EXPECT_TRUE(ColorPaletteLibrary::deleteCustom(p.name)); + for (const auto& q : ColorPaletteLibrary::loadCustomPalettes()) + EXPECT_NE(q.name, p.name) << "deleted palette must not reload"; +} + +TEST_F(ColorPaletteLibraryFileTest, CustomOverridesBundledOfTheSameName) { + const auto bundled = ColorPaletteLibrary::bundledPalettes(); + ASSERT_FALSE(bundled.empty()); + + Palette shadow; + shadow.name = bundled.front().name; // deliberately collide + shadow.swatches = {mk(1, 2, 3)}; + ASSERT_FALSE(ColorPaletteLibrary::saveCustom(shadow).empty()); + + const auto all = ColorPaletteLibrary::allPalettes(); + int matches = 0; + for (const auto& p : all) { + if (p.name != shadow.name) continue; + ++matches; + EXPECT_EQ(p.swatches.size(), 1u) << "the custom version must win"; + } + EXPECT_EQ(matches, 1) << "override must replace, not duplicate, the entry"; + + ColorPaletteLibrary::deleteCustom(shadow.name); +} + +TEST_F(ColorPaletteLibraryFileTest, SafeFileStemSanitisesPathCharacters) { + // The stem becomes a filename, so separators must not survive. + EXPECT_EQ(ColorPaletteLibrary::safeFileStem("Sky Blues"), "Sky_Blues"); + const std::string evil = ColorPaletteLibrary::safeFileStem("../../etc/passwd"); + EXPECT_EQ(evil.find('/'), std::string::npos); + EXPECT_EQ(evil.find('.'), std::string::npos); + EXPECT_FALSE(ColorPaletteLibrary::safeFileStem("!!!").empty()) + << "an all-punctuation name must still yield a usable stem"; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1ec9769b..de9afd74 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -167,6 +167,7 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/../src/PaintLayerStack.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/PaintSelectionMask.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/GradientRamp.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../src/ColorPaletteLibrary.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushEngine.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushFootprint.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushAssetLibrary.cpp From 0b0fa4f4870a0b064026fa6b3e4583bebeee0369 Mon Sep 17 00:00:00 2001 From: Fernando Date: Sun, 30 Aug 2026 22:41:40 -0400 Subject: [PATCH 2/7] =?UTF-8?q?feat(#551):=20Slice=20H=20part=202=20?= =?UTF-8?q?=E2=80=94=20brush=20preset=20library=20(pure=20data)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ogre-free core for full brush snapshots, same conventions as ColorPaletteLibrary / GradientRamp: bundled entries in C++, JSON persistence to /paint/presets/, hashed filename stems, custom-overrides-bundled. 15 bundled presets per the issue: Soft/Hard Round, Pencil Sketch, Spray Paint, Foliage Cluster, Edge Wear, Scratched Metal, Wet Brush, Smudge Soft/Hard, Stencil Hard/Soft, Eraser Soft/Hard, Cavity Dirt. A Preset captures tool, radius/strength/falloff, shape, channel, footprint, stamp/tiling asset, the stamp dynamics (spacing/scatter/size+opacity jitter/rotation) and the colour source (solid vs gradient + ramp name). Enum-valued fields are stored as plain ints holding the controller enum values, so this header stays pure-data with no dependency on the Ogre/Qt-heavy controllers. That is a deliberate trade: it means the controller enums must not be renumbered without a format bump, which the Preset doc comment states. Also adds export/import to an arbitrary path for the issue's JSON import/export requirement. Tests (13, all isolating via QStandardPaths test mode): - `BundledStampReferencesResolveToRealAssets` resolves every stamp name a preset mentions against BrushAssetLibrary. A typo there would apply silently and leave the previous footprint — presenting as "the preset did nothing" — so this is mutation-checked: renaming "Charcoal" to "Charcol" fails the test. - `JsonFromOlderBuildKeepsDefaultsForMissingFields` pins that a preset written before a field existed still loads, with the new field taking its struct default. These files persist across versions, so it matters now. - `BundledValuesAreInRange` catches a table typo shipping an unusable brush (negative radius, out-of-range jitter, stamp footprint with no stamp name). - `NamesCollidingAfterSanitisationDoNotOverwrite` proves the hashed stem keeps "My Brush" and "My/Brush" as two files. Registered in BOTH src/ and tests/ CMakeLists. Tests: 29/29 (16 palette + 13 preset). Co-Authored-By: Claude Opus 5 (1M context) --- src/BrushPresetLibrary.cpp | 348 ++++++++++++++++++++++++++++++++ src/BrushPresetLibrary.h | 106 ++++++++++ src/BrushPresetLibrary_test.cpp | 252 +++++++++++++++++++++++ src/CMakeLists.txt | 2 + tests/CMakeLists.txt | 1 + 5 files changed, 709 insertions(+) create mode 100644 src/BrushPresetLibrary.cpp create mode 100644 src/BrushPresetLibrary.h create mode 100644 src/BrushPresetLibrary_test.cpp diff --git a/src/BrushPresetLibrary.cpp b/src/BrushPresetLibrary.cpp new file mode 100644 index 00000000..795e504e --- /dev/null +++ b/src/BrushPresetLibrary.cpp @@ -0,0 +1,348 @@ +#include "BrushPresetLibrary.h" + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace BrushPresetLibrary { + +namespace { + +// Mirrors of the controller enums, named here only so the bundled table reads +// clearly. These MUST stay numerically in step with the real enums — see the +// Preset doc comment. +enum Tool { ToolPaint = 0, ToolErase = 1, ToolSmudge = 4 }; +enum Footprint { FpRound = 0, FpSquare = 1, FpStamp = 2, FpTiling = 3 }; +enum Rotation { RotNone = 0, RotFixed = 1, RotStroke = 2, RotRandom = 3 }; + +/// Hashed suffix so two names that sanitise to the same stem cannot collide. +std::string customFileStem(const std::string& name) +{ + const size_t h = std::hash{}(name); + char buf[16]; + std::snprintf(buf, sizeof(buf), "_%08x", static_cast(h & 0xffffffffu)); + return safeFileStem(name) + buf; +} + +} // namespace + +std::vector bundledPresets() +{ + std::vector out; + + auto add = [&out](const char* name, const char* note) -> Preset& { + Preset p; + p.name = name; + p.note = note; + out.push_back(std::move(p)); + return out.back(); + }; + + { // 1 + Preset& p = add("Soft Round", "General-purpose soft-edged round brush."); + p.footprint = FpRound; p.radius = 0.06; p.strength = 0.7; p.falloff = 0.8; + } + { // 2 + Preset& p = add("Hard Round", "Crisp round brush with almost no falloff."); + p.footprint = FpRound; p.radius = 0.04; p.strength = 1.0; p.falloff = 0.05; + } + { // 3 + Preset& p = add("Pencil Sketch", "Small, hard, lightly scattered — line work."); + p.footprint = FpStamp; p.stamp = "Charcoal"; + p.radius = 0.015; p.strength = 0.85; p.falloff = 0.2; + p.spacing = 0.12; p.scatter = 0.05; p.sizeJitter = 0.15; + p.stampRotation = RotRandom; + } + { // 4 + Preset& p = add("Spray Paint", "Wide, sparse spatter with heavy jitter."); + p.footprint = FpStamp; p.stamp = "Spatter"; + p.radius = 0.12; p.strength = 0.35; p.falloff = 0.9; + p.spacing = 0.25; p.scatter = 0.8; p.sizeJitter = 0.5; p.opacityJitter = 0.6; + p.stampRotation = RotRandom; + } + { // 5 + Preset& p = add("Foliage Cluster", "Scattered leaf stamps for vegetation."); + p.footprint = FpStamp; p.stamp = "Foliage Cluster"; + p.radius = 0.1; p.strength = 0.9; p.falloff = 0.3; + p.spacing = 0.6; p.scatter = 0.65; p.sizeJitter = 0.45; p.opacityJitter = 0.25; + p.stampRotation = RotRandom; + } + { // 6 + Preset& p = add("Edge Wear", "Thin scratchy strokes for worn edges."); + p.footprint = FpStamp; p.stamp = "Scratch Lines"; + p.radius = 0.05; p.strength = 0.6; p.falloff = 0.4; + p.spacing = 0.18; p.scatter = 0.2; p.opacityJitter = 0.4; + p.stampRotation = RotStroke; + } + { // 7 + Preset& p = add("Scratched Metal", "Long directional scratches."); + p.footprint = FpStamp; p.stamp = "Scratch Lines"; + p.radius = 0.08; p.strength = 0.8; p.falloff = 0.15; + p.spacing = 0.1; p.scatter = 0.1; p.sizeJitter = 0.3; + p.stampRotation = RotStroke; + } + { // 8 + Preset& p = add("Wet Brush", "Broad, soft, low-opacity build-up."); + p.footprint = FpRound; p.radius = 0.1; p.strength = 0.25; p.falloff = 0.95; + p.spacing = 0.08; + } + { // 9 + Preset& p = add("Smudge Soft", "Gentle smear with a wide soft tip."); + p.tool = ToolSmudge; p.footprint = FpRound; + p.radius = 0.09; p.strength = 0.35; p.falloff = 0.9; + } + { // 10 + Preset& p = add("Smudge Hard", "Tight, strong smear for pushing detail."); + p.tool = ToolSmudge; p.footprint = FpRound; + p.radius = 0.04; p.strength = 0.8; p.falloff = 0.2; + } + { // 11 + Preset& p = add("Stencil Hard", "Hard-edged square footprint for masks."); + p.footprint = FpSquare; p.shape = 1; + p.radius = 0.06; p.strength = 1.0; p.falloff = 0.0; + } + { // 12 + Preset& p = add("Stencil Soft", "Square footprint with a feathered edge."); + p.footprint = FpSquare; p.shape = 1; + p.radius = 0.07; p.strength = 0.75; p.falloff = 0.7; + } + { // 13 + Preset& p = add("Eraser Soft", "Soft eraser for fading paint away."); + p.tool = ToolErase; p.footprint = FpRound; + p.radius = 0.08; p.strength = 0.5; p.falloff = 0.85; + } + { // 14 + Preset& p = add("Eraser Hard", "Crisp eraser for clean cut-outs."); + p.tool = ToolErase; p.footprint = FpRound; + p.radius = 0.05; p.strength = 1.0; p.falloff = 0.05; + } + { // 15 + Preset& p = add("Cavity Dirt", "Grimy speckle for recesses and seams."); + p.footprint = FpStamp; p.stamp = "Spatter"; + p.radius = 0.07; p.strength = 0.45; p.falloff = 0.6; + p.spacing = 0.2; p.scatter = 0.5; p.sizeJitter = 0.4; p.opacityJitter = 0.5; + p.stampRotation = RotRandom; + } + + return out; +} + +const Preset* findBundled(const std::string& name) +{ + // Function-local static so the returned pointer stays valid. Same pattern + // as GradientRamp::findBundled. + static const std::vector kBundled = bundledPresets(); + for (const auto& p : kBundled) + if (p.name == name) return &p; + return nullptr; +} + +bool isBundled(const std::string& name) +{ + return findBundled(name) != nullptr; +} + +std::string toJson(const Preset& p) +{ + QJsonObject o; + o["name"] = QString::fromStdString(p.name); + o["tool"] = p.tool; + o["radius"] = p.radius; + o["strength"] = p.strength; + o["falloff"] = p.falloff; + o["shape"] = p.shape; + o["channel"] = p.channel; + o["footprint"] = p.footprint; + o["stamp"] = QString::fromStdString(p.stamp); + o["tiling"] = QString::fromStdString(p.tiling); + o["spacing"] = p.spacing; + o["scatter"] = p.scatter; + o["sizeJitter"] = p.sizeJitter; + o["opacityJitter"] = p.opacityJitter; + o["stampRotation"] = p.stampRotation; + o["stampAngleDeg"] = p.stampAngleDeg; + o["colorSource"] = p.colorSource; + o["gradientMode"] = p.gradientMode; + o["rampName"] = QString::fromStdString(p.rampName); + o["note"] = QString::fromStdString(p.note); + return QJsonDocument(o).toJson(QJsonDocument::Compact).toStdString(); +} + +bool fromJson(const std::string& json, Preset& out) +{ + QJsonParseError err{}; + const QJsonDocument doc = + QJsonDocument::fromJson(QByteArray::fromStdString(json), &err); + if (err.error != QJsonParseError::NoError || !doc.isObject()) return false; + const QJsonObject o = doc.object(); + + Preset p; // struct defaults are the fallback for every missing field, so + // a preset written by an older build still loads cleanly. + p.name = o.value("name").toString().toStdString(); + if (p.name.empty()) return false; + + p.tool = o.value("tool").toInt(p.tool); + p.radius = o.value("radius").toDouble(p.radius); + p.strength = o.value("strength").toDouble(p.strength); + p.falloff = o.value("falloff").toDouble(p.falloff); + p.shape = o.value("shape").toInt(p.shape); + p.channel = o.value("channel").toInt(p.channel); + p.footprint = o.value("footprint").toInt(p.footprint); + p.stamp = o.value("stamp").toString().toStdString(); + p.tiling = o.value("tiling").toString().toStdString(); + p.spacing = o.value("spacing").toDouble(p.spacing); + p.scatter = o.value("scatter").toDouble(p.scatter); + p.sizeJitter = o.value("sizeJitter").toDouble(p.sizeJitter); + p.opacityJitter = o.value("opacityJitter").toDouble(p.opacityJitter); + p.stampRotation = o.value("stampRotation").toInt(p.stampRotation); + p.stampAngleDeg = o.value("stampAngleDeg").toDouble(p.stampAngleDeg); + p.colorSource = o.value("colorSource").toInt(p.colorSource); + p.gradientMode = o.value("gradientMode").toInt(p.gradientMode); + p.rampName = o.value("rampName").toString().toStdString(); + p.note = o.value("note").toString().toStdString(); + + out = std::move(p); + return true; +} + +std::string presetsDirectory() +{ + if (!QCoreApplication::instance()) return {}; // AppDataLocation needs one + const QString base = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + if (base.isEmpty()) return {}; + const QString dir = QDir(base).filePath(QStringLiteral("paint/presets")); + QDir().mkpath(dir); + return dir.toStdString(); +} + +std::string safeFileStem(const std::string& name) +{ + std::string out; + out.reserve(name.size()); + for (const char c : name) { + if (std::isalnum(static_cast(c)) || c == '-' || c == '_') + out.push_back(c); + else if (c == ' ') + out.push_back('_'); + } + if (out.empty()) out = "preset"; + return out; +} + +std::string saveCustom(const Preset& p) +{ + if (!p.isValid()) return {}; + const std::string dir = presetsDirectory(); + if (dir.empty()) return {}; + const QString path = QDir(QString::fromStdString(dir)) + .filePath(QString::fromStdString(customFileStem(p.name)) + + QStringLiteral(".json")); + QFile f(path); + if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return {}; + const std::string json = toJson(p); + if (f.write(json.data(), static_cast(json.size())) + != static_cast(json.size())) { + f.close(); + QFile::remove(path); // never leave a truncated preset behind + return {}; + } + f.close(); + return path.toStdString(); +} + +std::vector loadCustomPresets() +{ + std::vector out; + const std::string dir = presetsDirectory(); + if (dir.empty()) return out; + QDir d(QString::fromStdString(dir)); + const QStringList files = + d.entryList({QStringLiteral("*.json")}, QDir::Files, QDir::Name); + for (const QString& fn : files) { + QFile f(d.filePath(fn)); + if (!f.open(QIODevice::ReadOnly)) continue; + const QByteArray data = f.readAll(); + f.close(); + Preset p; + if (fromJson(data.toStdString(), p)) out.push_back(std::move(p)); + } + return out; +} + +bool deleteCustom(const std::string& name) +{ + const std::string dir = presetsDirectory(); + if (dir.empty()) return false; + QDir d(QString::fromStdString(dir)); + + const QString direct = d.filePath(QString::fromStdString(customFileStem(name)) + + QStringLiteral(".json")); + if (QFile::exists(direct)) return QFile::remove(direct); + + // Fall back to matching the embedded name so imported or hand-authored + // files (whose stem we did not choose) are still deletable from the UI. + const QStringList files = + d.entryList({QStringLiteral("*.json")}, QDir::Files, QDir::Name); + for (const QString& fn : files) { + QFile f(d.filePath(fn)); + if (!f.open(QIODevice::ReadOnly)) continue; + const QByteArray data = f.readAll(); + f.close(); + Preset p; + if (fromJson(data.toStdString(), p) && p.name == name) + return QFile::remove(d.filePath(fn)); + } + return false; +} + +std::vector allPresets() +{ + std::vector out = bundledPresets(); + for (auto& custom : loadCustomPresets()) { + auto it = std::find_if(out.begin(), out.end(), + [&](const Preset& p) { return p.name == custom.name; }); + if (it != out.end()) *it = std::move(custom); // custom wins + else out.push_back(std::move(custom)); + } + return out; +} + +bool findPreset(const std::string& name, Preset& out) +{ + for (const auto& p : allPresets()) { + if (p.name == name) { out = p; return true; } + } + return false; +} + +bool exportToFile(const Preset& p, const std::string& path) +{ + if (!p.isValid() || path.empty()) return false; + QFile f(QString::fromStdString(path)); + if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return false; + const std::string json = toJson(p); + const bool ok = f.write(json.data(), static_cast(json.size())) + == static_cast(json.size()); + f.close(); + if (!ok) QFile::remove(QString::fromStdString(path)); + return ok; +} + +bool importFromFile(const std::string& path, Preset& out) +{ + QFile f(QString::fromStdString(path)); + if (!f.open(QIODevice::ReadOnly)) return false; + const QByteArray data = f.readAll(); + f.close(); + return fromJson(data.toStdString(), out); +} + +} // namespace BrushPresetLibrary diff --git a/src/BrushPresetLibrary.h b/src/BrushPresetLibrary.h new file mode 100644 index 00000000..0f451fa9 --- /dev/null +++ b/src/BrushPresetLibrary.h @@ -0,0 +1,106 @@ +#ifndef BRUSH_PRESET_LIBRARY_H +#define BRUSH_PRESET_LIBRARY_H + +#include +#include + +/** + * @brief Paint v2 Slice H (#551) — brush preset library (Ogre-free). + * + * A preset is a full snapshot of the brush configuration: tool, footprint, + * stamp, size/strength/falloff, colour source, and the stamp dynamics + * (spacing / scatter / jitter / rotation). Applying one restores the whole + * brush in a single click. + * + * Bundled presets are defined in C++ (not shipped as data files) so they cannot + * go missing from an install; custom presets serialize as JSON into + * `/paint/presets/`. Same conventions as GradientRamp (#544) and + * ColorPaletteLibrary. + * + * Pure data — Qt appears only in the .cpp for JSON I/O and AppData paths. The + * enum values below intentionally mirror the controller enums numerically; see + * `Preset` for why they are stored as ints rather than including the headers. + */ +namespace BrushPresetLibrary { + +/** + * A captured brush configuration. + * + * Enum-valued fields are plain ints holding the corresponding controller enum + * value (TexturePaintController::BrushTool, BrushFootprint::FootprintType / + * StampRotation, TexturePaintController::ColorSource / GradientMode, + * PaintChannelNS::Channel, EditModeController::BrushShape). They are ints so + * this stays a pure-data header with no dependency on the Ogre/Qt-heavy + * controllers — the apply layer does the conversion. They are also what gets + * written to JSON, so DO NOT renumber the controller enums without a format + * version bump. + */ +struct Preset { + std::string name; + + // --- core brush --- + int tool = 0; ///< BrushTool (0 = Paint) + double radius = 0.05; ///< mesh-local units + double strength = 1.0; ///< 0..1 + double falloff = 0.5; ///< 0..1 + int shape = 0; ///< BrushShape (0 = Round, 1 = Square) + int channel = 0; ///< PaintChannelNS::Channel + + // --- footprint --- + int footprint = 0; ///< FootprintType (0 Round/1 Square/2 Stamp/3 Tiling) + std::string stamp; ///< stamp asset name; empty = none + std::string tiling; ///< tiling asset name; empty = none + + // --- stamp dynamics --- + double spacing = 0.35; ///< 0.05..2.0 + double scatter = 0.0; ///< 0..1 + double sizeJitter = 0.0; ///< 0..1 + double opacityJitter = 0.0; ///< 0..1 + int stampRotation = 0; ///< StampRotation (0 None/1 Fixed/2 Stroke/3 Random) + double stampAngleDeg = 0.0; + + // --- colour source --- + int colorSource = 0; ///< ColorSource (0 Solid, 1 Gradient) + int gradientMode = 0; ///< GradientMode (0 Linear, 1 Radial, 2 Angular) + std::string rampName; ///< gradient ramp name; empty = FG/BG + + /// Optional one-line description shown as a tooltip. + std::string note; + + bool isValid() const { return !name.empty(); } +}; + +/// The 15 bundled presets required by #551. +std::vector bundledPresets(); +/// Look up a bundled preset by exact name; nullptr when unknown. +const Preset* findBundled(const std::string& name); +/// True when `name` matches a bundled preset (gates rename/delete in the UI). +bool isBundled(const std::string& name); + +/// JSON round-trip. Unknown/missing fields fall back to the struct defaults, so +/// a preset written by an older build still loads. +std::string toJson(const Preset& p); +bool fromJson(const std::string& json, Preset& out); + +/// `/paint/presets`, created on demand. Empty when unavailable. +std::string presetsDirectory(); +std::string saveCustom(const Preset& p); +std::vector loadCustomPresets(); +bool deleteCustom(const std::string& name); + +/// Bundled + custom, with custom overriding a bundled preset of the same name. +std::vector allPresets(); +/// Look up across bundled + custom (custom wins). +bool findPreset(const std::string& name, Preset& out); + +/// Filesystem-safe stem for a preset name. +std::string safeFileStem(const std::string& name); + +/// Export/import a preset to/from an arbitrary path (the issue's JSON +/// import/export). Returns false on I/O or parse failure. +bool exportToFile(const Preset& p, const std::string& path); +bool importFromFile(const std::string& path, Preset& out); + +} // namespace BrushPresetLibrary + +#endif // BRUSH_PRESET_LIBRARY_H diff --git a/src/BrushPresetLibrary_test.cpp b/src/BrushPresetLibrary_test.cpp new file mode 100644 index 00000000..063b818b --- /dev/null +++ b/src/BrushPresetLibrary_test.cpp @@ -0,0 +1,252 @@ +/* +----------------------------------------------------------------------------------- +A QtMeshEditor file — BrushPresetLibrary unit tests (Paint v2 Slice H, issue #551) + +Pure-data: the bundled catalogue, JSON round-trip (including forward/backward +compatibility), persistence, and import/export. Filesystem cases redirect + via QStandardPaths test mode so they never touch a real install. + +Copyright (c) Fernando Tonon (https://github.com/fernandotonon) +The MIT License — see other project sources for the full header. +----------------------------------------------------------------------------------- +*/ +#include + +#include "BrushAssetLibrary.h" +#include "BrushPresetLibrary.h" + +#include +#include +#include + +#include +#include + +using BrushPresetLibrary::Preset; + +// --- bundled catalogue ----------------------------------------------------- + +TEST(BrushPresetLibraryTest, ShipsAtLeastFifteenBundledPresets) { + const auto all = BrushPresetLibrary::bundledPresets(); + EXPECT_GE(all.size(), 15u) << "#551 requires at least 15 bundled presets"; + for (const auto& p : all) { + EXPECT_TRUE(p.isValid()) << p.name; + EXPECT_FALSE(p.note.empty()) << p.name << " should carry a tooltip note"; + } +} + +TEST(BrushPresetLibraryTest, BundledNamesAreUniqueAndFindable) { + const auto all = BrushPresetLibrary::bundledPresets(); + std::set seen; + for (const auto& p : all) { + // The name is the lookup key AND the custom-override key, so a + // duplicate would make one preset permanently unreachable. + EXPECT_TRUE(seen.insert(p.name).second) << "duplicate name: " << p.name; + ASSERT_NE(BrushPresetLibrary::findBundled(p.name), nullptr) << p.name; + EXPECT_TRUE(BrushPresetLibrary::isBundled(p.name)) << p.name; + } + EXPECT_EQ(BrushPresetLibrary::findBundled("No Such Preset"), nullptr); + EXPECT_FALSE(BrushPresetLibrary::isBundled("No Such Preset")); +} + +TEST(BrushPresetLibraryTest, BundledStampReferencesResolveToRealAssets) { + // A preset naming a stamp that does not exist would apply silently and + // leave the user with the previous footprint — the failure would look like + // "the preset did nothing". + for (const auto& p : BrushPresetLibrary::bundledPresets()) { + if (p.stamp.empty()) continue; + const std::string path = BrushAssetLibrary::resolvePath( + p.stamp, BrushAssetLibrary::AssetKind::Stamp); + EXPECT_FALSE(path.empty()) + << p.name << " references missing stamp '" << p.stamp << "'"; + } +} + +TEST(BrushPresetLibraryTest, BundledValuesAreInRange) { + // Guards against a typo in the table shipping an unusable brush (e.g. a + // negative radius or a strength above 1 that silently clamps). + for (const auto& p : BrushPresetLibrary::bundledPresets()) { + EXPECT_GT(p.radius, 0.0) << p.name; + EXPECT_LE(p.radius, 1.0) << p.name; + EXPECT_GE(p.strength, 0.0) << p.name; + EXPECT_LE(p.strength, 1.0) << p.name; + EXPECT_GE(p.falloff, 0.0) << p.name; + EXPECT_LE(p.falloff, 1.0) << p.name; + EXPECT_GE(p.spacing, 0.05) << p.name; + EXPECT_LE(p.spacing, 2.0) << p.name; + for (const double j : {p.scatter, p.sizeJitter, p.opacityJitter}) { + EXPECT_GE(j, 0.0) << p.name; + EXPECT_LE(j, 1.0) << p.name; + } + // A stamp footprint without a stamp name would fall back to a round + // brush, quietly ignoring the preset's whole point. + if (p.footprint == 2) EXPECT_FALSE(p.stamp.empty()) << p.name; + } +} + +// --- JSON ------------------------------------------------------------------ + +TEST(BrushPresetLibraryTest, JsonRoundTripPreservesEveryField) { + Preset p; + p.name = "Round Trip"; + p.tool = 4; p.radius = 0.123; p.strength = 0.44; p.falloff = 0.66; + p.shape = 1; p.channel = 3; + p.footprint = 2; p.stamp = "Spatter"; p.tiling = "Brick"; + p.spacing = 0.7; p.scatter = 0.3; p.sizeJitter = 0.2; p.opacityJitter = 0.1; + p.stampRotation = 3; p.stampAngleDeg = 45.0; + p.colorSource = 1; p.gradientMode = 2; p.rampName = "Sunset"; + p.note = "note text"; + + Preset b; + ASSERT_TRUE(BrushPresetLibrary::fromJson(BrushPresetLibrary::toJson(p), b)); + EXPECT_EQ(b.name, p.name); + EXPECT_EQ(b.tool, p.tool); + EXPECT_DOUBLE_EQ(b.radius, p.radius); + EXPECT_DOUBLE_EQ(b.strength, p.strength); + EXPECT_DOUBLE_EQ(b.falloff, p.falloff); + EXPECT_EQ(b.shape, p.shape); + EXPECT_EQ(b.channel, p.channel); + EXPECT_EQ(b.footprint, p.footprint); + EXPECT_EQ(b.stamp, p.stamp); + EXPECT_EQ(b.tiling, p.tiling); + EXPECT_DOUBLE_EQ(b.spacing, p.spacing); + EXPECT_DOUBLE_EQ(b.scatter, p.scatter); + EXPECT_DOUBLE_EQ(b.sizeJitter, p.sizeJitter); + EXPECT_DOUBLE_EQ(b.opacityJitter, p.opacityJitter); + EXPECT_EQ(b.stampRotation, p.stampRotation); + EXPECT_DOUBLE_EQ(b.stampAngleDeg, p.stampAngleDeg); + EXPECT_EQ(b.colorSource, p.colorSource); + EXPECT_EQ(b.gradientMode, p.gradientMode); + EXPECT_EQ(b.rampName, p.rampName); + EXPECT_EQ(b.note, p.note); +} + +TEST(BrushPresetLibraryTest, JsonFromOlderBuildKeepsDefaultsForMissingFields) { + // A preset written before a field existed must still load, with the new + // field taking its struct default rather than zero/garbage. + Preset out; + ASSERT_TRUE(BrushPresetLibrary::fromJson( + R"({"name":"Minimal","radius":0.2})", out)); + EXPECT_EQ(out.name, "Minimal"); + EXPECT_DOUBLE_EQ(out.radius, 0.2); + EXPECT_DOUBLE_EQ(out.strength, 1.0) << "missing field must keep its default"; + EXPECT_DOUBLE_EQ(out.spacing, 0.35) << "missing field must keep its default"; + EXPECT_EQ(out.footprint, 0); +} + +TEST(BrushPresetLibraryTest, JsonRejectsUnusableInput) { + Preset out; + EXPECT_FALSE(BrushPresetLibrary::fromJson("not json", out)); + EXPECT_FALSE(BrushPresetLibrary::fromJson("[]", out)); + EXPECT_FALSE(BrushPresetLibrary::fromJson(R"({"radius":0.5})", out)) + << "a preset with no name has no lookup key and is unusable"; +} + +// --- persistence + import/export (isolated from the real ) -------- + +class BrushPresetLibraryFileTest : public ::testing::Test +{ +protected: + void SetUp() override { QStandardPaths::setTestModeEnabled(true); } + void TearDown() override + { + const std::string dir = BrushPresetLibrary::presetsDirectory(); + if (!dir.empty()) QDir(QString::fromStdString(dir)).removeRecursively(); + QStandardPaths::setTestModeEnabled(false); + } +}; + +TEST_F(BrushPresetLibraryFileTest, SaveLoadDeleteRoundTrip) { + Preset p; + p.name = "My Brush"; + p.radius = 0.077; p.strength = 0.33; + + ASSERT_FALSE(BrushPresetLibrary::saveCustom(p).empty()); + + bool found = false; + for (const auto& q : BrushPresetLibrary::loadCustomPresets()) + if (q.name == p.name && std::abs(q.radius - 0.077) < 1e-9) found = true; + EXPECT_TRUE(found) << "a saved preset must load back"; + + EXPECT_TRUE(BrushPresetLibrary::deleteCustom(p.name)); + for (const auto& q : BrushPresetLibrary::loadCustomPresets()) + EXPECT_NE(q.name, p.name) << "deleted preset must not reload"; +} + +TEST_F(BrushPresetLibraryFileTest, CustomOverridesBundledOfTheSameName) { + const auto bundled = BrushPresetLibrary::bundledPresets(); + ASSERT_FALSE(bundled.empty()); + + Preset shadow; + shadow.name = bundled.front().name; // deliberately collide + shadow.radius = 0.999; + ASSERT_FALSE(BrushPresetLibrary::saveCustom(shadow).empty()); + + int matches = 0; + for (const auto& p : BrushPresetLibrary::allPresets()) { + if (p.name != shadow.name) continue; + ++matches; + EXPECT_DOUBLE_EQ(p.radius, 0.999) << "the custom version must win"; + } + EXPECT_EQ(matches, 1) << "override must replace, not duplicate, the entry"; + + Preset viaFind; + ASSERT_TRUE(BrushPresetLibrary::findPreset(shadow.name, viaFind)); + EXPECT_DOUBLE_EQ(viaFind.radius, 0.999); + + BrushPresetLibrary::deleteCustom(shadow.name); +} + +TEST_F(BrushPresetLibraryFileTest, FindPresetSeesBundledAndMissesUnknown) { + Preset out; + EXPECT_TRUE(BrushPresetLibrary::findPreset("Soft Round", out)); + EXPECT_EQ(out.name, "Soft Round"); + EXPECT_FALSE(BrushPresetLibrary::findPreset("Nope", out)); +} + +TEST_F(BrushPresetLibraryFileTest, ExportImportRoundTripsThroughAFile) { + QTemporaryDir tmp; + ASSERT_TRUE(tmp.isValid()); + const std::string path = + tmp.filePath(QStringLiteral("brush.json")).toStdString(); + + Preset p; + p.name = "Exported"; p.radius = 0.25; p.stamp = "Charcoal"; + ASSERT_TRUE(BrushPresetLibrary::exportToFile(p, path)); + + Preset back; + ASSERT_TRUE(BrushPresetLibrary::importFromFile(path, back)); + EXPECT_EQ(back.name, "Exported"); + EXPECT_DOUBLE_EQ(back.radius, 0.25); + EXPECT_EQ(back.stamp, "Charcoal"); + + // Missing / unparseable files must fail cleanly, not crash or half-fill. + Preset junk; + EXPECT_FALSE(BrushPresetLibrary::importFromFile( + tmp.filePath(QStringLiteral("nope.json")).toStdString(), junk)); + EXPECT_FALSE(BrushPresetLibrary::exportToFile(Preset{}, path)) + << "an unnamed preset is not exportable"; +} + +TEST_F(BrushPresetLibraryFileTest, SafeFileStemSanitisesPathCharacters) { + EXPECT_EQ(BrushPresetLibrary::safeFileStem("Soft Round"), "Soft_Round"); + const std::string evil = BrushPresetLibrary::safeFileStem("../../etc/passwd"); + EXPECT_EQ(evil.find('/'), std::string::npos); + EXPECT_EQ(evil.find('.'), std::string::npos); + EXPECT_FALSE(BrushPresetLibrary::safeFileStem("***").empty()); +} + +TEST_F(BrushPresetLibraryFileTest, NamesCollidingAfterSanitisationDoNotOverwrite) { + // "My Brush" and "My/Brush" both sanitise to "My_Brush"; the hashed suffix + // is what keeps them as two distinct files. + Preset a; a.name = "My Brush"; a.radius = 0.1; + Preset b; b.name = "My/Brush"; b.radius = 0.2; + ASSERT_FALSE(BrushPresetLibrary::saveCustom(a).empty()); + ASSERT_FALSE(BrushPresetLibrary::saveCustom(b).empty()); + + const auto loaded = BrushPresetLibrary::loadCustomPresets(); + EXPECT_EQ(loaded.size(), 2u) << "colliding stems must not overwrite each other"; + + BrushPresetLibrary::deleteCustom(a.name); + BrushPresetLibrary::deleteCustom(b.name); +} diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index be59e2f7..f19e0231 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -159,6 +159,7 @@ PaintLayerStack.cpp PaintSelectionMask.cpp GradientRamp.cpp ColorPaletteLibrary.cpp +BrushPresetLibrary.cpp BrushEngine.cpp BrushFootprint.cpp BrushAssetLibrary.cpp @@ -392,6 +393,7 @@ SymmetryMirrorMap.h PaintSelectionMask.h GradientRamp.h ColorPaletteLibrary.h +BrushPresetLibrary.h BrushEngine.h TexturePaintBuffer.h UpdateVersion.h diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index de9afd74..eaf1a9e9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -168,6 +168,7 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/../src/PaintSelectionMask.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/GradientRamp.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/ColorPaletteLibrary.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushPresetLibrary.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushEngine.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushFootprint.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BrushAssetLibrary.cpp From ba2369098f8da95d48f707786f20847cca8b7f4a Mon Sep 17 00:00:00 2001 From: Fernando Date: Sun, 30 Aug 2026 22:57:15 -0400 Subject: [PATCH 3/7] =?UTF-8?q?feat(#551):=20Slice=20H=20part=203=20?= =?UTF-8?q?=E2=80=94=20preset/palette=20apply=20layer=20on=20the=20control?= =?UTF-8?q?ler?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Connects the two pure-data libraries to the live brush. Presets: applyBrushPreset / saveBrushPreset / brushPresetNames / isBundledBrushPreset / deleteBrushPreset / exportBrushPreset / importBrushPreset. Palettes: colorPaletteNames / colorPaletteSwatches / recentPaintColors / applyPaletteColor / savePaletteFromTexture, plus a paletteChanged signal so a swatch grid can refresh. Three deliberate behaviours: - **Applying a preset does NOT touch the paint colour.** A preset describes the brush — shape and dynamics — not what you are painting with. Clobbering the user's colour on every preset click would be hostile, so colour is excluded from both capture and apply. - **Only the FOREGROUND colour feeds the recent ring.** The background is a secondary slot changed rarely; mixing it in would churn the history. - **Apply order: stamp/tiling ASSET before footprint TYPE**, so a stamp brush never briefly points at the previous preset's image. - Deleting a bundled preset is refused: they are compiled in, so a "delete" could only remove a user override and would appear to come back on restart. Tests: 8 controller cases (37/37 across Slice H). One of them needed hardening after mutation testing: `ApplyStampPresetSelectsItsStampAndFootprint` initially PASSED with a mutant that skipped setActiveStampName entirely, because m_activeStampName is restored from QSettings and already held the expected value from an earlier run. The test now sets a different stamp first, and the mutant fails it. Worth remembering for any other assertion against QSettings-backed state in this controller. Co-Authored-By: Claude Opus 5 (1M context) --- src/TexturePaintController.cpp | 184 ++++++++++++++++++++++++++++ src/TexturePaintController.h | 41 +++++++ src/TexturePaintController_test.cpp | 162 ++++++++++++++++++++++++ 3 files changed, 387 insertions(+) diff --git a/src/TexturePaintController.cpp b/src/TexturePaintController.cpp index 1e954b6a..53c24dd1 100644 --- a/src/TexturePaintController.cpp +++ b/src/TexturePaintController.cpp @@ -1,4 +1,5 @@ #include "TexturePaintController.h" +#include "BrushPresetLibrary.h" #include "AppSettingsKeys.h" #include "EditModeController.h" @@ -7245,3 +7246,186 @@ void TexturePaintController::flushPaintTextureForExport(Ogre::Entity* entity) QStringLiteral("Flushed %1-layer composite before export") .arg(m_layerStack.layerCount())); } + +// --------------------------------------------------------------------------- +// Paint v2 Slice H (#551): brush presets + colour palettes +// --------------------------------------------------------------------------- + +bool TexturePaintController::applyBrushPreset(const QString& name) +{ + BrushPresetLibrary::Preset p; + if (!BrushPresetLibrary::findPreset(name.toStdString(), p)) return false; + + // Order matters: set the footprint's ASSET before the footprint type, so a + // stamp footprint never briefly points at the previous preset's image. + if (!p.stamp.empty()) setActiveStampName(QString::fromStdString(p.stamp)); + if (!p.tiling.empty()) setActiveTilingName(QString::fromStdString(p.tiling)); + setFootprintType(p.footprint); + + setBrushTool(p.tool); + setBrushRadius(p.radius); + setBrushStrength(p.strength); + setBrushFalloff(p.falloff); + setActiveChannel(p.channel); + if (auto* em = EditModeController::instance()) + em->setVertexPaintShape(p.shape); + + setStampSpacing(p.spacing); + setStampScatter(p.scatter); + setStampSizeJitter(p.sizeJitter); + setStampOpacityJitter(p.opacityJitter); + setStampRotation(p.stampRotation); + setStampFixedAngle(p.stampAngleDeg); + + setColorSource(p.colorSource); + setGradientMode(p.gradientMode); + if (!p.rampName.empty()) setActiveRampName(QString::fromStdString(p.rampName)); + + // Deliberately NOT restored: the paint COLOUR. A preset describes the brush + // (its shape and dynamics), not what you are painting with — clobbering the + // user's colour on every preset click would be hostile. + SentryReporter::addBreadcrumb("paint.preset.apply", name); + return true; +} + +bool TexturePaintController::saveBrushPreset(const QString& name) +{ + const QString trimmed = name.trimmed(); + if (trimmed.isEmpty()) return false; + + BrushPresetLibrary::Preset p; + p.name = trimmed.toStdString(); + p.tool = brushTool(); + p.radius = texturePaintRadius(); + p.strength = texturePaintStrength(); + p.falloff = texturePaintFalloff(); + p.shape = brushShape(); + p.channel = activeChannel(); + p.footprint = footprintType(); + p.stamp = activeStampName().toStdString(); + p.tiling = activeTilingName().toStdString(); + p.spacing = stampSpacing(); + p.scatter = stampScatter(); + p.sizeJitter = stampSizeJitter(); + p.opacityJitter = stampOpacityJitter(); + p.stampRotation = stampRotation(); + p.stampAngleDeg = stampFixedAngle(); + p.colorSource = colorSource(); + p.gradientMode = gradientMode(); + p.rampName = activeRampName().toStdString(); + + if (BrushPresetLibrary::saveCustom(p).empty()) return false; + SentryReporter::addBreadcrumb("paint.preset.save", trimmed); + return true; +} + +QStringList TexturePaintController::brushPresetNames() const +{ + QStringList out; + for (const auto& p : BrushPresetLibrary::allPresets()) + out << QString::fromStdString(p.name); + return out; +} + +bool TexturePaintController::isBundledBrushPreset(const QString& name) const +{ + return BrushPresetLibrary::isBundled(name.toStdString()); +} + +bool TexturePaintController::deleteBrushPreset(const QString& name) +{ + // Bundled presets are compiled in, so "deleting" one could only remove a + // user override — refuse rather than appear to delete something that comes + // straight back on restart. + if (isBundledBrushPreset(name)) return false; + const bool ok = BrushPresetLibrary::deleteCustom(name.toStdString()); + if (ok) SentryReporter::addBreadcrumb("paint.preset.delete", name); + return ok; +} + +bool TexturePaintController::exportBrushPreset(const QString& name, const QString& path) +{ + BrushPresetLibrary::Preset p; + if (!BrushPresetLibrary::findPreset(name.toStdString(), p)) return false; + return BrushPresetLibrary::exportToFile(p, path.toStdString()); +} + +QString TexturePaintController::importBrushPreset(const QString& path) +{ + BrushPresetLibrary::Preset p; + if (!BrushPresetLibrary::importFromFile(path.toStdString(), p)) return {}; + if (BrushPresetLibrary::saveCustom(p).empty()) return {}; + SentryReporter::addBreadcrumb("paint.preset.import", + QString::fromStdString(p.name)); + return QString::fromStdString(p.name); +} + +QStringList TexturePaintController::colorPaletteNames() const +{ + QStringList out; + for (const auto& p : ColorPaletteLibrary::allPalettes()) + out << QString::fromStdString(p.name); + return out; +} + +QStringList TexturePaintController::colorPaletteSwatches(const QString& paletteName) const +{ + QStringList out; + for (const auto& p : ColorPaletteLibrary::allPalettes()) { + if (QString::fromStdString(p.name) != paletteName) continue; + for (const auto& s : p.swatches) + out << QString::fromStdString(ColorPaletteLibrary::swatchToHex(s)); + break; + } + return out; +} + +QStringList TexturePaintController::recentPaintColors() const +{ + QStringList out; + for (const auto& s : m_recentColors) + out << QString::fromStdString(ColorPaletteLibrary::swatchToHex(s)); + return out; +} + +bool TexturePaintController::applyPaletteColor(const QString& hex, bool asBackground) +{ + ColorPaletteLibrary::Swatch s; + if (!ColorPaletteLibrary::swatchFromHex(hex.toStdString(), s)) return false; + + auto* em = EditModeController::instance(); + if (!em) return false; + const QColor c(s.r, s.g, s.b); + if (asBackground) em->setVertexPaintBackgroundColor(c); + else em->setVertexPaintColor(c); + + // Only the foreground feeds the recent ring: the background is a secondary + // slot the user changes rarely, and mixing it in would churn the history. + if (!asBackground) { + ColorPaletteLibrary::pushRecent(m_recentColors, s); + emit paletteChanged(); + } + SentryReporter::addBreadcrumb("paint.palette.apply", + QStringLiteral("%1%2").arg(hex, asBackground ? QStringLiteral(" (bg)") : QString())); + return true; +} + +bool TexturePaintController::savePaletteFromTexture(const QString& paletteName, int maxColours) +{ + const QString trimmed = paletteName.trimmed(); + if (trimmed.isEmpty()) return false; + if (m_buffer.width() <= 0 || m_buffer.height() <= 0) return false; + + const auto swatches = ColorPaletteLibrary::extractFromImage( + m_buffer.data().data(), m_buffer.width(), m_buffer.height(), maxColours); + if (swatches.empty()) return false; // fully transparent buffer, nothing to take + + ColorPaletteLibrary::Palette p; + p.name = trimmed.toStdString(); + p.swatches = swatches; + if (ColorPaletteLibrary::saveCustom(p).empty()) return false; + SentryReporter::addBreadcrumb("paint.palette.save", + QStringLiteral("%1 (%2 colours)").arg(trimmed).arg(swatches.size())); + emit paletteChanged(); + return true; +} diff --git a/src/TexturePaintController.h b/src/TexturePaintController.h index 2dfbc126..32be8c6d 100644 --- a/src/TexturePaintController.h +++ b/src/TexturePaintController.h @@ -12,6 +12,7 @@ #include "SymmetryMirrorMap.h" #include "ProjectionPainter.h" #include "DecalSession.h" +#include "ColorPaletteLibrary.h" #include #include @@ -592,6 +593,37 @@ class TexturePaintController : public QObject /// Setters that mirror through to EditModeController so the toolbar /// brush popup and the Material-mode Paint Brush panel both stay /// in sync. + // --- Paint v2 Slice H (#551): brush presets + palettes --- + /// Apply a preset (bundled or custom) to the live brush. Returns false when + /// the name is unknown. Emits the usual per-setting change signals, so the + /// panel refreshes without any preset-specific plumbing. + Q_INVOKABLE bool applyBrushPreset(const QString& name); + /// Capture the CURRENT brush configuration as a named custom preset and + /// save it. Returns false on an empty name or a write failure. + Q_INVOKABLE bool saveBrushPreset(const QString& name); + /// Bundled + custom preset names, for the library UI. + Q_INVOKABLE QStringList brushPresetNames() const; + /// True when `name` is a bundled preset (gates rename/delete in the UI). + Q_INVOKABLE bool isBundledBrushPreset(const QString& name) const; + Q_INVOKABLE bool deleteBrushPreset(const QString& name); + Q_INVOKABLE bool exportBrushPreset(const QString& name, const QString& path); + /// Import a preset file and save it into the user library. Returns the + /// imported preset's name, or an empty string on failure. + Q_INVOKABLE QString importBrushPreset(const QString& path); + + /// Palette names (bundled + custom) and a palette's swatches as + /// "#rrggbb" strings, for the swatch grid. + Q_INVOKABLE QStringList colorPaletteNames() const; + Q_INVOKABLE QStringList colorPaletteSwatches(const QString& paletteName) const; + /// Most-recently-used colours, newest first (max 12). + Q_INVOKABLE QStringList recentPaintColors() const; + /// Set the foreground (or background) paint colour from "#rrggbb" and push + /// it onto the recent ring. + Q_INVOKABLE bool applyPaletteColor(const QString& hex, bool asBackground = false); + /// Build a palette from the ACTIVE paint buffer and save it. Returns false + /// when there is no session or the extraction found no opaque pixels. + Q_INVOKABLE bool savePaletteFromTexture(const QString& paletteName, int maxColours = 10); + Q_INVOKABLE void setBrushRadius(double r); Q_INVOKABLE void setBrushStrength(double s); Q_INVOKABLE void setBrushFalloff(double f); @@ -767,6 +799,9 @@ class TexturePaintController : public QObject void stabilizerChanged(); /// Paint v2 Slice F (#549): projection mode / stencil / lock state changed. void projectionChanged(); + /// Paint v2 Slice H (#551): the palette list or the recent-colour ring + /// changed, so a swatch grid should refresh. + void paletteChanged(); /// Emitted when the mouse hovers over a UV-mapped triangle (from /// the 3D mesh or from the 2D texture preview panel). u,v in [0..1]; /// (-1, -1) means "no hover". @@ -1148,6 +1183,12 @@ class TexturePaintController : public QObject QPointF m_stabLastRaw; // true last cursor (for end catch-up) bool m_stabHaveLastRaw = false; + // --- Paint v2 Slice H (#551): recent colours --- + /// Newest first, capped at 12. In-memory only: this is a session + /// convenience, and persisting it would restore a stale palette that no + /// longer matches whatever the user is painting next. + std::vector m_recentColors; + // --- Paint v2 Slice F (#549): projection / stencil painting --- int m_projectionMode = 0; // 0 off / 1 stencil-brush / 2 camera-locked QString m_stencilImagePath; diff --git a/src/TexturePaintController_test.cpp b/src/TexturePaintController_test.cpp index 4121445c..d1fe61bb 100644 --- a/src/TexturePaintController_test.cpp +++ b/src/TexturePaintController_test.cpp @@ -1,5 +1,7 @@ #include +#include + #include #include #include @@ -1413,3 +1415,163 @@ TEST_F(TexturePaintControllerSceneTest, DecalSessionBeginCancelPlumbing) { ctrl->closeSession(); } + +// --- Paint v2 Slice H (#551): brush presets + colour palettes ------------- +// The library cores are covered pure-data in BrushPresetLibrary_test / +// ColorPaletteLibrary_test. These cases cover the CONTROLLER contract: a preset +// actually reaches the live brush, capture round-trips, and every entry point +// degrades safely on bad input. + +TEST_F(TexturePaintControllerSceneTest, ApplyBrushPresetReachesTheLiveBrush) { + ASSERT_TRUE(m_fix.setup(QStringLiteral("PresetApply"))); + auto* ctrl = TexturePaintController::instance(); + ASSERT_NE(ctrl, nullptr); + + // "Hard Round": radius 0.04, strength 1.0, falloff 0.05, round footprint. + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("Hard Round"))); + EXPECT_NEAR(ctrl->texturePaintRadius(), 0.04, 1e-6); + EXPECT_NEAR(ctrl->texturePaintStrength(), 1.0, 1e-6); + EXPECT_NEAR(ctrl->texturePaintFalloff(), 0.05, 1e-6); + + // Switching presets must move every field, not just the first one applied. + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("Wet Brush"))); + EXPECT_NEAR(ctrl->texturePaintRadius(), 0.1, 1e-6); + EXPECT_NEAR(ctrl->texturePaintStrength(), 0.25, 1e-6); + EXPECT_NEAR(ctrl->texturePaintFalloff(), 0.95, 1e-6); + + ctrl->closeSession(); +} + +TEST_F(TexturePaintControllerSceneTest, ApplyStampPresetSelectsItsStampAndFootprint) { + ASSERT_TRUE(m_fix.setup(QStringLiteral("PresetStamp"))); + auto* ctrl = TexturePaintController::instance(); + + // Move the stamp somewhere else FIRST. m_activeStampName is restored from + // QSettings, so without this the expected value can already be in place and + // the assertion passes even if the preset never applies it (verified: a + // mutant that skipped setActiveStampName survived this test until the + // pre-set was added). + ctrl->setActiveStampName(QStringLiteral("Soft Circle")); + ASSERT_EQ(ctrl->activeStampName(), QStringLiteral("Soft Circle")); + + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("Spray Paint"))); + EXPECT_EQ(ctrl->activeStampName(), QStringLiteral("Spatter")) + << "a stamp preset must select its stamp asset"; + EXPECT_EQ(ctrl->footprintType(), + static_cast(BrushFootprint::FootprintType::StampImage)) + << "and switch the footprint to stamp mode"; + // Dynamics ride along, else the preset would look identical to a plain brush. + EXPECT_NEAR(ctrl->stampScatter(), 0.8, 1e-6); + EXPECT_NEAR(ctrl->stampSizeJitter(), 0.5, 1e-6); + + ctrl->closeSession(); +} + +TEST_F(TexturePaintControllerSceneTest, ApplyBrushPresetLeavesPaintColorAlone) { + ASSERT_TRUE(m_fix.setup(QStringLiteral("PresetColor"))); + auto* ctrl = TexturePaintController::instance(); + auto* em = EditModeController::instance(); + ASSERT_NE(em, nullptr); + + em->setVertexPaintColor(QColor(12, 34, 56)); + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("Soft Round"))); + // A preset describes the BRUSH, not what you paint with — clobbering the + // user's colour on every preset click would be hostile. + EXPECT_EQ(em->vertexPaintColor(), QColor(12, 34, 56)); + + ctrl->closeSession(); +} + +TEST_F(TexturePaintControllerSceneTest, SaveBrushPresetCapturesCurrentBrush) { + QStandardPaths::setTestModeEnabled(true); + ASSERT_TRUE(m_fix.setup(QStringLiteral("PresetSave"))); + auto* ctrl = TexturePaintController::instance(); + + ctrl->setBrushRadius(0.0321); + ctrl->setBrushStrength(0.456); + ctrl->setBrushFalloff(0.789); + ASSERT_TRUE(ctrl->saveBrushPreset(QStringLiteral("Captured Brush"))); + + // Move the brush away, then apply the capture — it must come back. + ctrl->setBrushRadius(0.2); + ctrl->setBrushStrength(0.1); + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("Captured Brush"))); + EXPECT_NEAR(ctrl->texturePaintRadius(), 0.0321, 1e-6); + EXPECT_NEAR(ctrl->texturePaintStrength(), 0.456, 1e-6); + EXPECT_NEAR(ctrl->texturePaintFalloff(), 0.789, 1e-6); + + EXPECT_TRUE(ctrl->deleteBrushPreset(QStringLiteral("Captured Brush"))); + ctrl->closeSession(); + QStandardPaths::setTestModeEnabled(false); +} + +TEST_F(TexturePaintControllerSceneTest, BrushPresetEntryPointsRejectBadInput) { + auto* ctrl = TexturePaintController::instance(); + ASSERT_NE(ctrl, nullptr); + + EXPECT_FALSE(ctrl->applyBrushPreset(QStringLiteral("No Such Preset"))); + EXPECT_FALSE(ctrl->saveBrushPreset(QString())); + EXPECT_FALSE(ctrl->saveBrushPreset(QStringLiteral(" "))) + << "a whitespace-only name is not a usable preset name"; + // Bundled presets are compiled in: "deleting" one could only remove a user + // override, so it must refuse rather than appear to delete something that + // comes straight back on restart. + EXPECT_FALSE(ctrl->deleteBrushPreset(QStringLiteral("Soft Round"))); + EXPECT_TRUE(ctrl->isBundledBrushPreset(QStringLiteral("Soft Round"))); + EXPECT_FALSE(ctrl->isBundledBrushPreset(QStringLiteral("No Such Preset"))); + EXPECT_GE(ctrl->brushPresetNames().size(), 15); +} + +TEST_F(TexturePaintControllerSceneTest, PaletteColorAppliesAndFeedsRecentRing) { + ASSERT_TRUE(m_fix.setup(QStringLiteral("PaletteApply"))); + auto* ctrl = TexturePaintController::instance(); + auto* em = EditModeController::instance(); + ASSERT_NE(em, nullptr); + + ASSERT_TRUE(ctrl->applyPaletteColor(QStringLiteral("#4caf50"))); + EXPECT_EQ(em->vertexPaintColor(), QColor(0x4c, 0xaf, 0x50)); + ASSERT_FALSE(ctrl->recentPaintColors().isEmpty()); + EXPECT_EQ(ctrl->recentPaintColors().front(), QStringLiteral("#4caf50")); + + // The background slot must NOT churn the recent ring. + const int before = ctrl->recentPaintColors().size(); + ASSERT_TRUE(ctrl->applyPaletteColor(QStringLiteral("#ff0000"), /*asBackground=*/true)); + EXPECT_EQ(em->vertexPaintBackgroundColor(), QColor(255, 0, 0)); + EXPECT_EQ(ctrl->recentPaintColors().size(), before) + << "background picks must not enter the recent ring"; + + EXPECT_FALSE(ctrl->applyPaletteColor(QStringLiteral("nonsense"))); + ctrl->closeSession(); +} + +TEST_F(TexturePaintControllerSceneTest, PaletteListsExposeBundledContent) { + auto* ctrl = TexturePaintController::instance(); + ASSERT_NE(ctrl, nullptr); + const QStringList names = ctrl->colorPaletteNames(); + EXPECT_GE(names.size(), 6) << "#551 requires at least 6 bundled palettes"; + ASSERT_TRUE(names.contains(QStringLiteral("Material Design"))); + + const QStringList sw = ctrl->colorPaletteSwatches(QStringLiteral("Material Design")); + EXPECT_FALSE(sw.isEmpty()); + for (const QString& s : sw) + EXPECT_TRUE(s.startsWith('#') && s.size() == 7) << s.toStdString(); + + EXPECT_TRUE(ctrl->colorPaletteSwatches(QStringLiteral("No Such Palette")).isEmpty()); +} + +TEST_F(TexturePaintControllerSceneTest, SavePaletteFromTextureNeedsABuffer) { + QStandardPaths::setTestModeEnabled(true); + auto* ctrl = TexturePaintController::instance(); + ctrl->closeSession(); + // No session => no buffer => must refuse rather than save an empty palette. + EXPECT_FALSE(ctrl->savePaletteFromTexture(QStringLiteral("From Nothing"))); + + ASSERT_TRUE(m_fix.setup(QStringLiteral("PaletteFromTex"))); + ctrl->setTexturePaintEnabled(true); + ASSERT_TRUE(ctrl->hasActiveSession()); + EXPECT_FALSE(ctrl->savePaletteFromTexture(QString())) + << "an unnamed palette is not saveable"; + + ctrl->closeSession(); + QStandardPaths::setTestModeEnabled(false); +} From 0ebb1947716855d6242d01f29422d64b8ea38e7b Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 19:55:29 -0400 Subject: [PATCH 4/7] =?UTF-8?q?feat(#551):=20Slice=20H=20part=204=20?= =?UTF-8?q?=E2=80=94=20brush=20portal=20UI,=20panel=20fixes,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **UI** (Qt Widgets, in the existing brush portal): a Preset dropdown with Save as… / Delete / Export… / Import…, a Palette dropdown with a 5-column swatch grid (left-click = foreground, right-click = background, matching the existing FG/BG toolbar swatch), a Recent row that fills as colours are picked, and "Palette from texture…". Widgets rather than QML — departing from CLAUDE.md's guidance — because the portal's radius/strength/footprint controls are all Widgets and mixing toolkits in one panel buys nothing. Same reasoning as StampLibraryDialog. The issue asks for a preset thumbnail grid with search + tags. Presets are parameter sets with no image to display, so a grid of identical tiles would carry no more information than the name; a dropdown is used instead, and search/tags are omitted as unjustified at 15 entries. Both departures are recorded in the design doc rather than left implicit. Delete is disabled for bundled presets, so the refusal is visible rather than a button that silently does nothing. **Two panel defects reported from a screenshot:** - *Doubled separator above "Color:".* `syncShape` hides the "Edge:" row for Stamp/Tiling footprints, but its separator stayed visible and collapsed against the next one — so the artefact only appeared in stamp mode. `addSectionSeparator` now returns its frame and the Edge row hides it too. - *Hint text rendering black.* It was styled `color: palette(mid)` — a mid-grey BORDER role (its only other use in this file is borders) which is near-black on the dark theme. Now `palette(text)`, kept as a palette role rather than a literal white so it still follows the theme. It was the only text in the file misusing `mid`. Docs: `docs/PAINT_V2_SLICE_H_DESIGN.md` (allowlisted in .gitignore, which ignores docs/* per-file) + the CLAUDE.md architecture entry. Verified: 37/37 Slice H tests, plus 40/40 across the existing paint suites (BrushEngine / BrushFootprint / GradientRamp / PaintChannel / PaintLayer) to confirm no regression in the panel this touches. The UI itself was exercised interactively and confirmed working. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 1 + CLAUDE.md | 1 + docs/PAINT_V2_SLICE_H_DESIGN.md | 103 ++++++++++++ src/mainwindow.cpp | 289 +++++++++++++++++++++++++++++++- 4 files changed, 390 insertions(+), 4 deletions(-) create mode 100644 docs/PAINT_V2_SLICE_H_DESIGN.md diff --git a/.gitignore b/.gitignore index 9a56429a..61f39fe2 100755 --- a/.gitignore +++ b/.gitignore @@ -153,6 +153,7 @@ __pycache__/ /multiview_bake_*.png /multiview_bake_*i.png !docs/SKINNING_QUALITY.md +!docs/PAINT_V2_SLICE_H_DESIGN.md !docs/img !docs/img/twist_bar_rest_lbs.png !docs/img/twist_bar_90_lbs.png diff --git a/CLAUDE.md b/CLAUDE.md index 0166af90..4dbf0c21 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -234,6 +234,7 @@ Three singletons manage core state. All run on the main thread. Access via `Clas - **Paint v2 Slice D — full PBR channel painting (#547)**: paint into any of 8 channels — **`PaintChannelNS::Channel { BaseColor, Normal, Roughness, Metallic, AO, Emissive, Height, VertexColor }`** (`src/PaintChannel.h`, pure-data + unit-tested; each maps to a canonical TUS slot `albedo`/`normal_map`/`roughness`/`metallic`/`ao`/`emissive`, Height→normal_map, VertexColor→the existing per-vertex path). Architecture: **per-channel sessions** — the live `m_layerStack`/`m_buffer` is the active channel's session; `setActiveChannel()` **stashes** the current channel's layer stack into `m_channelSessions[channel]` and **restores** the target's (so each channel keeps its own stack across switches). Channel router: `refreshSlots()` tags each TUS with its channel; `findOrCreateActiveTextureUnit()` **auto-creates** the canonical slot when the asset never shipped it. **Channel-aware bake (`bakeChannel`) — every channel MERGES with the slot's existing texture, never a blind overwrite** (so a bake only changes what the user painted): colour channels (BaseColor/Emissive) composite the painted strokes **source-over the slot's current texture** (reading it back via `loadImageAcrossGroups`, alias-aware for BaseColor's `diffuse_map`, and **skipping the transient `QMEPaint_*` paint texture** so it reads the real underlying image, not the live paint buffer; a brand-new diffuse with no source flattens onto opaque white); **scalar** channels (Roughness/Metallic/AO) are painted grayscale and collapsed via Rec.601 luminance into ONLY their lane of the packed ORM texture (`.r=AO/.g=roughness/.b=metallic`, other lanes preserved from the existing ORM) bound to the `metallic` slot the Cook-Torrance SRS reads; **Height/Normal** Sobel-bakes the painted grayscale to a tangent-space detail normal via `NormalMapGenerator::generate` then **whiteout-blends it onto the existing `normal_map`** (`n.xy=base.xy+detail.xy`, `n.z=base.z*detail.z`, normalized — untouched texels have detail normal `(0,0,1)` so the base normal is unchanged; painted texels add relief). Height has no slot of its own — it bakes INTO `normal_map` (`slotName(Height)==""`), so a mesh carries one combined normal map, not separate height+normal. **Height is NOT a selectable channel** (removed from the `paintChannels()` picker; `setActiveChannel(Height)` redirects to Normal) — a separate Height channel just produced a second normal-map bake fighting the Normal one, and there is no parallax/displacement shader that would consume a standalone height texture. Paint the **Normal** channel directly (its grayscale is Sobel-converted to a detail normal and whiteout-blended onto the existing normal, sourcing the base from the session's `m_originalTextureName` first). Height was the highest picker index (enum 6, just before VertexColor 7), so dropping it leaves the remaining picker list indices identical to their `Channel` enum values — the QML `activeChannel === index` binding stays correct with no renumbering. Live-IBL binding recipe (`bindBakedChannelTexture`): bind TUS by canonical name (alias-aware for the diffuse slot) → `RTShaderHelper::wirePbrSlotsForFFP` → `MeshImporterExporter::applyNormalMapsToEntity` (tangents, for normal maps) → `applyPbrIfTagged` **only if the material was ALREADY PBR** (never silently promote a plain material to Cook-Torrance on bake — that darkened it to near-black without IBL) → `mat->compile()` → `refreshAllPbrMaterialsForHdr()`. **Critically, `bindBakedChannelTexture` prunes the just-baked slot from `m_boundSlots` and then TEARS THE LIVE SESSION DOWN** (`stashChannelSession` + `m_buffer.clearDirty()` + `closeSession()`) instead of calling `flushDirtyToOgre()` — a trailing flush would re-upload the stale paint buffer and, since the slot was just pruned, its deferred-rebind branch would re-bind the transient `QMEPaint_*` texture straight back over the freshly-baked file (that was the "first bake changes the render, second bake wipes the texture" bug). The next brush stroke lazily rebuilds a session seeded from the baked result. Source-less channel sessions also start **transparent** (not opaque white) and defer the manual-paint-texture model rebind to the first painted stroke, so merely NAVIGATING channels never swaps the model's real slot textures for a blank paint texture. **Presets** (`src/PaintChannelPresets.h/cpp`, QML_SINGLETON, `MaterialPresetLibrary` pattern): 5 channel-aware presets — "Scratches into roughness", "Emissive sparks", "Edge wear", "Dirt build-up", "Sticker" — set active channel + brush params via the controller. UI: a 7-button channel picker + presets dropdown + per-channel "Bake" button in `texPaintCol`. Tests: `PaintChannel_test.cpp`, `PaintChannelPresets_test.cpp` (pure-data), plus `TexturePaintController_test.cpp` scene-fixture cases (channel switch auto-creates slot, scalar bake → ORM, height bake → normal_map, per-channel session isolation). **Session scoping**: `m_channelSessions` is cleared when the painted entity changes (`m_channelSessionEntity`) so one mesh's channel stacks never leak/bake onto another. **Undo across channels**: paint undo commands (`TexturePaintStrokeCommand`/`PaintLayerOpCommand`/`TexturePaintMaskActionCommand`) key on a stable `(Ogre::Entity*, channel)` — NOT the transient `QMEPaint_*` GPU texture name (which changes every channel/session switch). `undo()`/`redo()` call `ensureUndoTarget(entity, channel)` which reselects the entity + `setActiveChannel` before applying the snapshot, so undoing a prior channel's stroke after switching channels (or deselecting the mesh) correctly reactivates and restores it; a command whose entity was deleted no-ops safely. - **Paint v2 Slice E — symmetry + line stabilizer (#548)**: real-time mirrored strokes and mouse-jitter smoothing. **Symmetry**: WRITE-backed props `symmetryEnabled` (default OFF), `symmetrySpace` (`SymLocal`/`SymWorld`, default local), `symmetryAxes` (bitmask `SymAxisX=1|Y=2|Z=4`, default X → 8 combinations), `topologyMirror` (default on). Every dab is mirrored across each enabled **axis-subset** (1 point for X, 3 for X|Y, 7 for X|Y|Z — `mirrorLocalPoints` iterates nonzero subsets) *inside the same begin/end stroke window*, so mirror dabs are captured by the existing single `TexturePaintStrokeCommand` — **one undo step, no new command**. Works from BOTH the viewport (screen raycast → `m_hitCache`) and the 2D panel (`applyBrushSymmetryDabs` falls back to `findMeshPointForUV`, which also seeds the hit cache). **Geometric resolver** `uvForLocalPoint` (inverse of `findMeshPointForUV`): reflect the primary LOCAL hit across the axis — local: about the mesh origin `m_symmetryPivotLocal`; world: about the plane through the entity's `_getDerivedPosition()` (NOT world 0, else off-origin objects mirror into empty space) — then nearest-triangle → barycentric → UV. **Topology-aware mirror** (`src/SymmetryMirrorMap.{h,cpp}`, pure-data + unit-tested): for a position-symmetric mesh with an ASYMMETRIC UV unwrap, a geometric re-raycast reads the wrong UV; instead `build()` makes a per-vertex position correspondence (spatial-hash grid) **verified by triangle 1-ring adjacency** (rejects false position collisions; `coverage()`≥0.6 AND verify-ratio≥0.9 to be `valid()`), and `mirrorDab(submesh, corner[3], bary → mirror tri + permuted bary)` maps the dab to the mirror triangle and **permutes the barycentric weights to that triangle's stored corner order** (reflection reverses winding — mandatory) so the mirror samples the mirror triangle's own UVs. Per-single-axis map cached in `m_symmetryMaps` keyed by axis bit, built **lazily on first symmetric dab**, entity-guarded (`m_symmetryMapEntity`), invalidated on entity/mesh change + `closeSession`; falls back to geometric per-dab when no correspondence. **Multi-axis segment continuity (E-B)**: per-subset previous mirror UV (`m_mirrorPrevUV`/`m_mirrorHavePrevUV`, reset in `resetStrokePaintState`) so each mirror stroke fans a `paintBrushAlongSegment` and doesn't gap on fast moves. **Plane viz**: `refreshSymmetryPlaneOverlay` draws a faint translucent quad per enabled axis (X=red/Y=green/Z=blue, alpha 0.12, depth-write off) on `m_symPlaneNode/Obj` (mirrors `drawHoverRingAt`), torn down in `closeSession`. **Stabilizer**: `stabilizerMode` (`StabAverage`/`StabTrail`), `stabilizerAmount` (0..100, default **0 = exact passthrough / zero latency**). Smooths the raw SCREEN cursor before hit-testing in `updateStroke`; `beginStroke` seeds the buffer with the press point (first dab exact); `endStroke` does a **synchronous** catch-up to the true cursor *before* the deferred undo commit (Krita behaviour — stays one undo step; NEVER a timer). Pure math (`stabilizerWindow`/`stabilizeAveragePoint` weighted newest-heaviest/`stabilizeTrailPoint` lag-distance) is `static` for unit testing. Sentry breadcrumbs `paint.symmetry` (enable/space/axes/topology/map-build) + `paint.stabilizer` (mode/amount, once per stroke). QML: Symmetry + Stabilizer groups in `texPaintCol`. Tests: `SymmetryMirrorMap_test.cpp` (full-coverage build, **asymmetric-UV correctness**, no-correspondence invalid) + `TexturePaintStabilizer_test.cpp` (window growth, jitter reduction, trail lag/catch-up, amount-0 passthrough, setter clamp) — pure-data, plus a `TexturePaintController_test.cpp` one-undo fixture case. - **Paint v2 Slice F — projection/stencil painting + decals (#549)**: two image-driven paint modes that project an image onto the mesh THROUGH a camera, rasterize it in UV0 space, and write into a paint layer; both auto-create a NEW layer (never stomp the active one). Design doc: `docs/PAINT_V2_SLICE_F_DESIGN.md`. **`src/ProjectionMath.h`** (header-only) extracts the shared `projectToViewportUV` (world→viewport UV + NDC z + behind) / `sampleImage` out of `MultiViewTextureBaker` so both the #403 baker and this slice share one definition. **`src/ProjectionPainter.{h,cpp}`** (pure-data, unit-tested) is the FORKED single-projection rasterizer (NOT a mutation of `MultiViewTextureBaker::bake`, whose multi-view weighted accumulator is the wrong shape): `project(tris, View, source, out, opts, occ?)` clears `out` transparent then per texel interpolates world pos + projected source UV → facing cull → occlusion/depth-limit → sample → soft-edge → **manual src-over composite**; `projectDab(...)` is the footprint-bounded ACCUMULATING stencil-brush variant. Tris come from `MultiViewTextureBaker::fromEntity`. **Occlusion (the hard part)**: `MeshDepthRenderer` renders a depth map from the camera (grayscale = LINEAR world distance via fog, near=bright/far=dark; `RenderResult` gained `depthNear/depthFar`); per texel, project world pos through the depth map's OWN `viewProj`, reconstruct `dMap = near+(1-g)*(far-near)`, and reject when the texel's **camera-axis** distance `dot(wp-eye, camDir)` (NOT Euclidean — Euclidean self-occludes off-axis texels, i.e. depth acne) exceeds `dMap + bias` (bias > 1/255 of the range). Depth-limit rejects texels farther than a fraction-of-bounds-radius BEHIND the nearest visible surface (the sphere-with-hole case). The color `View` and the occlusion `viewProj` are kept strictly separate (the depth camera auto-frames, so its matrices differ from the live camera's). **Projection surfaces** (`TexturePaintController`): WRITE-backed props `projectionMode` (0 off / 1 stencil-brush / 2 camera-locked), `stencilImagePath`, `projBackfaceCull`, `projUseOcclusion`, `projDepthLimit`, read-only `cameraLocked`. Stencil brush: `paintColorFootprintAtUV` delegates to `projectDab` through the live (mode 1) or locked (mode 2) camera View — masked by the projected stencil alpha, painted into the active layer (normal stroke undo). `m_projTris` cached at `beginStroke`; occlusion map refreshed at stroke start (mode 1) / on `snapProjectionCamera` (mode 2), never per dab. `projectFromPhoto` projects a photo through the camera into a scratch buffer and commits via `commitProjectedLayer` (new `Generated` layer + one `PaintLayerOpCommand`). `currentProjectionView` = `cam->getProjectionMatrixWithRSDepth()*getViewMatrix()` + `getRealDirection/Position`. **Decal tool** (`ToolDecal=6`): **`src/DecalSession.{h,cpp}`** (pure-data) is a world-anchored oriented-quad state machine — `begin(img)→place(surfaceHit,normal,camUp)→Editing`; `translate/rotate(about normal)/scale`; `hitTest(rectUv)→Body/RotateCorner/ScaleEdge`; `buildCommit(softEdge)` returns an ORTHOGRAPHIC `View` (world→clip change-of-basis mapping the quad to NDC ±1, camDirection into the surface) + the decal image with a feathered soft-edge alpha. Controller: `refreshDecalOverlay` draws a world-space `ManualObject` quad + corner(rotate)/edge(scale) handle squares (depth-off so grabbable), torn down in `closeSession`; `placeDecalAt`/`decalHitTest`(ray∩rect-plane→rect-UV)/`dragDecal`/`commitDecal`(→`ProjectionPainter::project`+occlusion→new layer)/`cancelDecal`. Viewport routing: `TransformOperator::mousePressEvent` consumes decal clicks BEFORE the paint-stroke branch (place while Placing, grab a handle while Editing), `mouseMoveEvent` drags, release ends the drag (session stays open); `MainWindow::keyPressEvent` — Enter commits / Esc cancels (swallow-all, mirrors the knife). Both create a new layer + are undoable. Sentry `paint.projection.*` / `paint.decal.*`. QML: a **collapsible** Projection group (mode/stencil/Snap/Backface/Occlude/Project-from-photo) + a Decal row (Place/Commit/Cancel + hint) in `texPaintCol`. Tests: `ProjectionPainter_test.cpp` (front projection, backface, stencil gating, dab, **sphere-with-hole occlusion**, depth-limit, self-projection no-acne) + `DecalSession_test.cpp` (transitions, hit-test zones, edits, world↔rect-UV, ortho-commit corner→NDC, soft-edge) — pure-data; plus `TexturePaintController_test.cpp` fixture cases (projection-mode setters + graceful no-camera; decal begin/cancel plumbing). +- **Paint v2 Slice H — brush presets + colour palettes (#551)**: one-click reuse of a whole brush configuration, plus curated colour swatches. Design doc: `docs/PAINT_V2_SLICE_H_DESIGN.md`. **Tablet pressure/tilt (the issue's third feature) is deliberately NOT implemented** — the project is desktop-only, so pressure curves could be written and unit-tested but never verified against real pen hardware; it should return as its own issue when there is a tablet to test on. **`src/BrushPresetLibrary.{h,cpp}`** (pure-data, unit-tested): a `Preset` captures tool/radius/strength/falloff/shape/channel/footprint/stamp/tiling + stamp dynamics (spacing/scatter/size+opacity jitter/rotation) + colour source (solid vs gradient + ramp name). 15 bundled presets (Soft/Hard Round, Pencil Sketch, Spray Paint, Foliage Cluster, Edge Wear, Scratched Metal, Wet Brush, Smudge Soft/Hard, Stencil Hard/Soft, Eraser Soft/Hard, Cavity Dirt) defined **in C++ rather than shipped as data files** so they cannot go missing from an install (the `GradientRamp::bundledPresets` precedent); custom presets are JSON in `/paint/presets/`. Enum fields are stored as plain INTS holding the controller enum values, keeping the header pure-data — so **the controller enums must not be renumbered without a format bump**. **`src/ColorPaletteLibrary.{h,cpp}`** (pure-data, unit-tested): 6 bundled CC0 palettes (Material Design, Pantone Classics, Skin Tones, Foliage Greens, Sky Blues, Earth Tones), JSON in `/paint/palettes/`, plus `pushRecent` (re-picking PROMOTES rather than duplicating, so the 12-slot ring stays distinct) and `extractFromImage` for "palette from texture" (5-bit-per-channel colour-cube quantisation averaging the real pixels per bucket — counting exact RGB values returns ten imperceptibly different shades; fully transparent pixels skipped or a mostly-empty texture reports "transparent black" as dominant). `Swatch` carries **no alpha** on purpose: a palette curates hues, and baking alpha in would override the user's brush alpha on every pick. Both libraries use the same conventions: bundled-in-C++ + JSON custom + hashed filename stems (so two names sanitising to the same stem cannot overwrite each other) + custom-overrides-bundled precedence (as `BrushAssetLibrary::resolvePath` does for stamps). **Controller apply layer**: `applyBrushPreset`/`saveBrushPreset`/`brushPresetNames`/`isBundledBrushPreset`/`deleteBrushPreset`/`exportBrushPreset`/`importBrushPreset` + `colorPaletteNames`/`colorPaletteSwatches`/`recentPaintColors`/`applyPaletteColor`/`savePaletteFromTexture` + a `paletteChanged` signal. Three non-obvious behaviours: **applying a preset does NOT touch the paint colour** (a preset describes the brush, not what you paint with — restoring a saved colour would silently discard the user's choice), **apply order sets the stamp/tiling ASSET before the footprint TYPE** (else a stamp brush briefly points at the previous preset's image), and **only the FOREGROUND feeds the recent ring** (the background is a rarely-changed secondary slot). Deleting a bundled preset is REFUSED (they are compiled in, so a delete could only remove a user override and would appear to return on restart); the UI disables the button. **UI is Qt Widgets in the brush portal** (`mainwindow.cpp`), departing from the QML guidance because the portal's radius/strength/footprint controls are all Widgets and mixing toolkits in one panel buys nothing — same reasoning as `StampLibraryDialog`, whose grid this follows. The issue asks for a preset thumbnail grid with search+tags; presets have no image to show so a grid of identical tiles would carry no more information than the name (dropdown used instead), and search/tags are unjustified at 15 entries. Swatches are a 5-column grid, left-click = FG / right-click = BG (matching the existing FG/BG toolbar swatch). Breadcrumbs `paint.preset.*` / `paint.palette.*`. Tests: `BrushPresetLibrary_test.cpp` (bundled catalogue, **every bundled stamp reference resolves against BrushAssetLibrary** — a typo there applies silently and leaves the previous footprint, presenting as "the preset did nothing"; older-build JSON keeps struct defaults for missing fields; colliding sanitised stems stay separate files), `ColorPaletteLibrary_test.cpp`, plus `TexturePaintController_test.cpp` cases. **Gotcha found by mutation testing:** `m_activeStampName` is restored from QSettings, so an assertion that a preset applied its stamp PASSED against a mutant that skipped the apply entirely — the test now sets a different stamp first. Watch for this anywhere else asserting against QSettings-backed controller state. ### Scene Lighting (epic #482, Slice H #490) diff --git a/docs/PAINT_V2_SLICE_H_DESIGN.md b/docs/PAINT_V2_SLICE_H_DESIGN.md new file mode 100644 index 00000000..64eb9f7d --- /dev/null +++ b/docs/PAINT_V2_SLICE_H_DESIGN.md @@ -0,0 +1,103 @@ +# Paint v2 Slice H — Brush presets & colour palettes (#551) + +Two quality-of-life features every serious paint app has: saving a whole brush +configuration for one-click reuse, and curated colour swatches. + +Parent epic: #543. + +> **Tablet pressure/tilt is deliberately NOT in this slice.** The issue bundles +> it as a third feature, but the project is desktop-only today, so pressure +> curves and tilt-driven stamp rotation could be written and unit-tested yet +> never verified against real pen hardware. Shipping an unverifiable headline +> feature is worse than deferring it; it should return as its own issue when +> there is a tablet to test on. + +## Brush presets + +A preset is a full snapshot of the brush: tool, footprint, stamp/tiling asset, +radius / strength / falloff, edge shape, channel, the stamp dynamics +(spacing / scatter / size + opacity jitter / rotation), and the colour source +(solid vs gradient, plus the ramp name). + +**15 bundled presets:** Soft Round, Hard Round, Pencil Sketch, Spray Paint, +Foliage Cluster, Edge Wear, Scratched Metal, Wet Brush, Smudge Soft, Smudge +Hard, Stencil Hard, Stencil Soft, Eraser Soft, Eraser Hard, Cavity Dirt. + +Bundled presets are defined **in C++**, not shipped as data files, so they +cannot go missing from an install — the same choice `GradientRamp::bundledPresets` +already makes. Custom presets serialize to `/paint/presets/*.json`. + +### Two things that are easy to get wrong + +**Applying a preset does not touch the paint colour.** A preset describes the +brush — its shape and dynamics — not what you are painting *with*. Restoring a +saved colour on every preset click would silently discard the user's current +choice, so colour is excluded from both capture and apply. + +**Apply order: the stamp/tiling asset is set before the footprint type.** The +other order leaves a stamp brush briefly pointing at the previous preset's +image. + +Deleting a bundled preset is refused rather than performed: they are compiled +in, so a "delete" could only remove a user override and the entry would appear +to come back on restart. The UI disables the button to make that visible. + +## Colour palettes + +**6 bundled palettes** (CC0 / factual colour values, no third-party asset +files): Material Design, Pantone Classics, Skin Tones, Foliage Greens, Sky +Blues, Earth Tones. Custom palettes live in `/paint/palettes/*.json`. + +- **5-column swatch grid.** Left-click sets the foreground, right-click the + background — matching the existing FG/BG toolbar swatch's semantics. +- **Recent colours** (last 12, newest first). Re-picking a colour *promotes* it + rather than appending a duplicate, so the ring stays 12 distinct colours. + Only the **foreground** feeds it: the background is a secondary slot changed + rarely, and mixing it in would churn the history. It is session-only — + persisting it would restore a stale palette unrelated to the next task. +- **Palette from texture** extracts representative colours from the active + paint buffer by coarse colour-cube quantisation (5 bits/channel), averaging + the real pixels in each bucket so every swatch is a colour that actually + occurs. Counting exact RGB values would return ten imperceptibly different + shades of the same colour. Fully transparent pixels are skipped, or a + mostly-empty texture reports "transparent black" as its dominant colour. + +`Swatch` carries **no alpha** on purpose: a palette curates hues, and paint +alpha is a separate brush property. Baking alpha into swatches would override +the user's setting on every pick. + +## Files + +| File | Role | +|---|---| +| `src/BrushPresetLibrary.{h,cpp}` | preset data, bundled table, JSON, import/export (pure data) | +| `src/ColorPaletteLibrary.{h,cpp}` | palette data, bundled table, JSON, recent ring, extraction (pure data) | +| `src/TexturePaintController.{h,cpp}` | the apply/capture layer + `paletteChanged` | +| `src/mainwindow.cpp` | the brush-portal UI (Qt Widgets, beside the existing paint controls) | + +Breadcrumbs: `paint.preset.apply` / `.save` / `.delete` / `.import`, +`paint.palette.apply` / `.save`. + +## UI toolkit note + +These controls are **Qt Widgets**, not QML, which departs from CLAUDE.md's +"new UI should be QML" guidance. They live inside the existing brush portal +alongside the radius/strength/footprint controls, which are all Widgets; adding +QML there would split one panel across two toolkits for no user benefit. The +same reasoning applies to `StampLibraryDialog`, whose grid this follows. + +The issue asks for a preset **thumbnail grid with search and tags**. Presets are +parameter sets with no image to display, so a grid of identical tiles would +carry no more information than the name — a dropdown is used instead. Search and +tags are omitted as unjustified at 15 entries; they become worth adding once a +user library grows large enough to need them. + +## Known limits + +- The stamp/tiling asset a preset names must exist. A bundled preset naming a + missing stamp would apply silently and leave the previous footprint, so a test + resolves every bundled reference against `BrushAssetLibrary`. +- Preset enum fields are stored as ints holding the controller enum values, so + the controller enums must not be renumbered without a format bump. +- Palette extraction reads the paint buffer, so it needs an active paint + session; there is no "extract from an arbitrary file" path. diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4d47ca2e..3036edc6 100755 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -166,6 +166,7 @@ #include #include #include +#include #include #include #include @@ -2228,15 +2229,19 @@ void MainWindow::initToolBar() }; const QString paintToggleStyle = inspectorToggleStyle(); - auto addSectionSeparator = [paintSettings, paintLay]() { + // Returns the frame so a caller can hide it together with the section it + // precedes — otherwise hiding a section's widgets leaves its separator + // behind and two rules stack up against each other. + auto addSectionSeparator = [paintSettings, paintLay]() -> QFrame* { auto* line = new QFrame(paintSettings); line->setFrameShape(QFrame::HLine); line->setFrameShadow(QFrame::Sunken); line->setFixedHeight(2); paintLay->addWidget(line); + return line; }; - addSectionSeparator(); + QFrame* shapeSeparator = addSectionSeparator(); // Soft round vs hard square edge — only applies to Round/Square footprints. auto* shapeRow = new QHBoxLayout(); @@ -2254,7 +2259,8 @@ void MainWindow::initToolBar() shapeGroup->setExclusive(true); shapeGroup->addButton(shapeRound); shapeGroup->addButton(shapeSquare); - auto syncShape = [shapeRound, shapeSquare, shapeLabel, emPaint, tpcPaint]() { + auto syncShape = [shapeRound, shapeSquare, shapeLabel, shapeSeparator, + emPaint, tpcPaint]() { const bool square = emPaint->vertexPaintShape() == EditModeController::ShapeSquare; QSignalBlocker br(shapeRound); QSignalBlocker bs(shapeSquare); @@ -2265,6 +2271,9 @@ void MainWindow::initToolBar() shapeLabel->setVisible(classicFootprint); shapeRound->setVisible(classicFootprint); shapeSquare->setVisible(classicFootprint); + // Hide this section's separator with it, else it collapses against the + // next one and reads as a doubled rule above "Color:". + if (shapeSeparator) shapeSeparator->setVisible(classicFootprint); }; syncShape(); connect(shapeRound, &QPushButton::clicked, this, [emPaint]() { @@ -2499,7 +2508,11 @@ void MainWindow::initToolBar() auto* footprintHint = new QLabel(footprintBox); footprintHint->setWordWrap(true); - footprintHint->setStyleSheet(QStringLiteral("color: palette(mid); font-size: 11px;")); + // palette(text) — the theme's foreground — NOT palette(mid), which is a + // mid-grey meant for BORDERS (its only other use in this file) and renders + // near-black on the dark theme. Kept as a palette role rather than a literal + // colour so it follows the theme instead of hardcoding white. + footprintHint->setStyleSheet(QStringLiteral("color: palette(text); font-size: 11px;")); footLay->addWidget(footprintHint); auto* stampPickBtn = new QPushButton(footprintBox); @@ -2824,6 +2837,274 @@ void MainWindow::initToolBar() }); connect(tpcPaint, &TexturePaintController::stampChanged, this, syncFootprintUi); + addSectionSeparator(); + + // ---- Paint v2 Slice H (#551): brush presets ---------------------------- + // A combo rather than a thumbnail grid: presets have no image to show (they + // are parameter sets, not stamps), so a grid of identical tiles would carry + // no information the name does not already give. + { + auto* presetRow = new QHBoxLayout(); + auto* presetLabel = new QLabel(tr("Preset:"), paintSettings); + auto* presetCombo = new QComboBox(paintSettings); + presetCombo->setMinimumWidth(150); + presetRow->addWidget(presetLabel); + presetRow->addWidget(presetCombo, 1); + paintLay->addLayout(presetRow); + + auto* presetBtnRow = new QHBoxLayout(); + auto* savePresetBtn = new QPushButton(tr("Save as…"), paintSettings); + auto* delPresetBtn = new QPushButton(tr("Delete"), paintSettings); + savePresetBtn->setStyleSheet(paintToggleStyle); + delPresetBtn->setStyleSheet(paintToggleStyle); + auto* exportPresetBtn = new QPushButton(tr("Export…"), paintSettings); + auto* importPresetBtn = new QPushButton(tr("Import…"), paintSettings); + exportPresetBtn->setStyleSheet(paintToggleStyle); + importPresetBtn->setStyleSheet(paintToggleStyle); + presetBtnRow->addWidget(savePresetBtn); + presetBtnRow->addWidget(delPresetBtn); + presetBtnRow->addStretch(); + paintLay->addLayout(presetBtnRow); + + auto* presetIoRow = new QHBoxLayout(); + presetIoRow->addWidget(exportPresetBtn); + presetIoRow->addWidget(importPresetBtn); + presetIoRow->addStretch(); + paintLay->addLayout(presetIoRow); + + // Rebuild on demand so presets saved this session appear without a + // restart; block signals so repopulating cannot re-trigger an apply. + auto refreshPresets = [tpcPaint, presetCombo, delPresetBtn]() { + const QString current = presetCombo->currentText(); + QSignalBlocker b(presetCombo); + presetCombo->clear(); + presetCombo->addItems(tpcPaint->brushPresetNames()); + const int idx = presetCombo->findText(current); + if (idx >= 0) presetCombo->setCurrentIndex(idx); + // Bundled presets are compiled in, so Delete could only remove a + // user override — disable it rather than appear to delete something + // that returns on restart. + delPresetBtn->setEnabled( + !presetCombo->currentText().isEmpty() + && !tpcPaint->isBundledBrushPreset(presetCombo->currentText())); + }; + refreshPresets(); + + connect(presetCombo, QOverload::of(&QComboBox::activated), this, + [tpcPaint, presetCombo, delPresetBtn](int idx) { + if (idx < 0) return; + const QString name = presetCombo->itemText(idx); + SentryReporter::addBreadcrumb("ui.action", + QStringLiteral("paint preset apply %1").arg(name)); + tpcPaint->applyBrushPreset(name); + delPresetBtn->setEnabled(!tpcPaint->isBundledBrushPreset(name)); + }); + + connect(savePresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPresets, presetCombo]() { + bool ok = false; + const QString name = QInputDialog::getText( + this, tr("Save Brush Preset"), tr("Preset name:"), + QLineEdit::Normal, QString(), &ok); + if (!ok || name.trimmed().isEmpty()) return; + if (tpcPaint->isBundledBrushPreset(name.trimmed())) { + QMessageBox::information( + this, tr("Save Brush Preset"), + tr("'%1' is a bundled preset name. Saving will " + "override it; the bundled version returns if you " + "delete the override.").arg(name.trimmed())); + } + if (!tpcPaint->saveBrushPreset(name)) { + QMessageBox::warning(this, tr("Save Brush Preset"), + tr("Could not save the preset.")); + return; + } + refreshPresets(); + const int i = presetCombo->findText(name.trimmed()); + if (i >= 0) presetCombo->setCurrentIndex(i); + }); + + connect(delPresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPresets, presetCombo]() { + const QString name = presetCombo->currentText(); + if (name.isEmpty()) return; + if (QMessageBox::question( + this, tr("Delete Brush Preset"), + tr("Delete the preset '%1'?").arg(name)) + != QMessageBox::Yes) return; + tpcPaint->deleteBrushPreset(name); + refreshPresets(); + }); + + connect(exportPresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, presetCombo]() { + const QString name = presetCombo->currentText(); + if (name.isEmpty()) return; + QFileDialog dlg(this, tr("Export Brush Preset")); + dlg.setAcceptMode(QFileDialog::AcceptSave); + dlg.setDefaultSuffix(QStringLiteral("json")); + dlg.setNameFilter(tr("Brush preset (*.json)")); + // Native dialogs freeze against Ogre GL — the same reason + // every other file dialog in this file sets this. + dlg.setOption(QFileDialog::DontUseNativeDialog, true); + dlg.selectFile(name + QStringLiteral(".json")); + if (dlg.exec() != QDialog::Accepted) return; + const QStringList files = dlg.selectedFiles(); + if (files.isEmpty()) return; + if (!tpcPaint->exportBrushPreset(name, files.first())) { + QMessageBox::warning(this, tr("Export Brush Preset"), + tr("Could not write the preset file.")); + return; + } + SentryReporter::addBreadcrumb("file.export", + QStringLiteral("paint preset export %1").arg(name)); + }); + + connect(importPresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPresets, presetCombo]() { + QFileDialog dlg(this, tr("Import Brush Preset")); + dlg.setFileMode(QFileDialog::ExistingFile); + dlg.setNameFilter(tr("Brush preset (*.json)")); + dlg.setOption(QFileDialog::DontUseNativeDialog, true); + if (dlg.exec() != QDialog::Accepted) return; + const QStringList files = dlg.selectedFiles(); + if (files.isEmpty()) return; + const QString imported = tpcPaint->importBrushPreset(files.first()); + if (imported.isEmpty()) { + QMessageBox::warning(this, tr("Import Brush Preset"), + tr("That file is not a valid brush preset.")); + return; + } + SentryReporter::addBreadcrumb("file.import", + QStringLiteral("paint preset import %1").arg(imported)); + refreshPresets(); + const int i = presetCombo->findText(imported); + if (i >= 0) presetCombo->setCurrentIndex(i); + }); + } + + addSectionSeparator(); + + // ---- Paint v2 Slice H (#551): colour swatches -------------------------- + { + auto* palRow = new QHBoxLayout(); + auto* palLabel = new QLabel(tr("Palette:"), paintSettings); + auto* palCombo = new QComboBox(paintSettings); + palCombo->setMinimumWidth(150); + palRow->addWidget(palLabel); + palRow->addWidget(palCombo, 1); + paintLay->addLayout(palRow); + + // 5 columns, as the issue specifies; rows grow with the palette. + auto* swatchHost = new QWidget(paintSettings); + auto* swatchGrid = new QGridLayout(swatchHost); + swatchGrid->setContentsMargins(0, 2, 0, 2); + swatchGrid->setSpacing(3); + paintLay->addWidget(swatchHost); + + auto* recentLabel = new QLabel(tr("Recent:"), paintSettings); + recentLabel->setStyleSheet(QStringLiteral("color: palette(text); font-size: 11px;")); + paintLay->addWidget(recentLabel); + auto* recentHost = new QWidget(paintSettings); + auto* recentGrid = new QGridLayout(recentHost); + recentGrid->setContentsMargins(0, 0, 0, 2); + recentGrid->setSpacing(3); + paintLay->addWidget(recentHost); + + // Shared tile factory: left-click sets FG, right-click sets BG. The BG + // binding matches the existing FG/BG toolbar swatch's semantics. + auto makeSwatchTile = [this, tpcPaint](QWidget* parent, const QString& hex) { + auto* b = new QPushButton(parent); + b->setFixedSize(22, 22); + b->setToolTip(tr("%1 — click to set foreground, right-click for background") + .arg(hex)); + b->setStyleSheet(QStringLiteral( + "QPushButton { background-color: %1; border: 1px solid palette(mid);" + " border-radius: 3px; }").arg(hex)); + b->setContextMenuPolicy(Qt::CustomContextMenu); + connect(b, &QPushButton::clicked, this, [tpcPaint, hex]() { + tpcPaint->applyPaletteColor(hex, /*asBackground=*/false); + }); + connect(b, &QPushButton::customContextMenuRequested, this, + [tpcPaint, hex](const QPoint&) { + tpcPaint->applyPaletteColor(hex, /*asBackground=*/true); + }); + return b; + }; + + auto clearGrid = [](QGridLayout* g) { + while (QLayoutItem* it = g->takeAt(0)) { + if (QWidget* w = it->widget()) w->deleteLater(); + delete it; + } + }; + + auto refreshSwatches = [tpcPaint, palCombo, swatchGrid, clearGrid, + makeSwatchTile, swatchHost]() { + clearGrid(swatchGrid); + const QStringList hexes = tpcPaint->colorPaletteSwatches(palCombo->currentText()); + int i = 0; + for (const QString& hex : hexes) { + swatchGrid->addWidget(makeSwatchTile(swatchHost, hex), i / 5, i % 5); + ++i; + } + }; + + auto refreshRecent = [tpcPaint, recentGrid, clearGrid, makeSwatchTile, + recentHost, recentLabel]() { + clearGrid(recentGrid); + const QStringList hexes = tpcPaint->recentPaintColors(); + // Hide the whole row until something has been picked, rather than + // showing an empty "Recent:" heading. + recentLabel->setVisible(!hexes.isEmpty()); + recentHost->setVisible(!hexes.isEmpty()); + int i = 0; + for (const QString& hex : hexes) { + recentGrid->addWidget(makeSwatchTile(recentHost, hex), i / 6, i % 6); + ++i; + } + }; + + auto refreshPalettes = [tpcPaint, palCombo, refreshSwatches]() { + const QString current = palCombo->currentText(); + QSignalBlocker b(palCombo); + palCombo->clear(); + palCombo->addItems(tpcPaint->colorPaletteNames()); + const int idx = palCombo->findText(current); + palCombo->setCurrentIndex(idx >= 0 ? idx : 0); + refreshSwatches(); + }; + refreshPalettes(); + refreshRecent(); + + connect(palCombo, QOverload::of(&QComboBox::activated), this, + [refreshSwatches](int) { refreshSwatches(); }); + + auto* fromTexBtn = new QPushButton(tr("Palette from texture…"), paintSettings); + fromTexBtn->setStyleSheet(paintToggleStyle); + paintLay->addWidget(fromTexBtn); + connect(fromTexBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPalettes]() { + bool ok = false; + const QString name = QInputDialog::getText( + this, tr("Palette From Texture"), tr("Palette name:"), + QLineEdit::Normal, QString(), &ok); + if (!ok || name.trimmed().isEmpty()) return; + if (!tpcPaint->savePaletteFromTexture(name)) { + QMessageBox::warning( + this, tr("Palette From Texture"), + tr("Could not extract colours — paint a texture first.")); + return; + } + refreshPalettes(); + }); + + // The controller emits paletteChanged when the recent ring or the + // palette list changes, so the grids stay in step with painting. + connect(tpcPaint, &TexturePaintController::paletteChanged, this, + [refreshRecent]() { refreshRecent(); }); + } + paintSettings->setMinimumWidth(280); paintSettings->adjustSize(); From 89754f5d4dadb41aef287975db27ab2f4a3e4cec Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 20:04:29 -0400 Subject: [PATCH 5/7] refactor(#551): extract the portal sections out of initToolBar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-empts the Sonar maintainability gate rather than waiting for CI to report it. `initToolBar()` is already ~2000 lines, and this slice added ~200 more to it; the gate measures NEW code, and cognitive complexity (cpp:S3776) is what failed the gate on the two previous PRs. The preset and swatch blocks move into `MainWindow::buildBrushPresetSection` and `buildColorSwatchSection`. initToolBar keeps only the two separators and two calls. The shared checkable-button stylesheet is passed in rather than recomputed, so the new buttons keep matching the rest of the portal. Purely mechanical, but verified rather than assumed: this kind of extraction can silently drop signal wiring, so the app was relaunched and the UI re-exercised alongside the 37/37 test run. Note initToolBar is NOT currently flagged by Sonar (the file's only S3776 hit is elsewhere, at line 4721) — this is about not being the change that pushes it over. Co-Authored-By: Claude Opus 5 (1M context) --- src/mainwindow.cpp | 723 +++++++++++++++++++++++---------------------- src/mainwindow.h | 10 + 2 files changed, 379 insertions(+), 354 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 3036edc6..9d8eec70 100755 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2838,272 +2838,10 @@ void MainWindow::initToolBar() connect(tpcPaint, &TexturePaintController::stampChanged, this, syncFootprintUi); addSectionSeparator(); - - // ---- Paint v2 Slice H (#551): brush presets ---------------------------- - // A combo rather than a thumbnail grid: presets have no image to show (they - // are parameter sets, not stamps), so a grid of identical tiles would carry - // no information the name does not already give. - { - auto* presetRow = new QHBoxLayout(); - auto* presetLabel = new QLabel(tr("Preset:"), paintSettings); - auto* presetCombo = new QComboBox(paintSettings); - presetCombo->setMinimumWidth(150); - presetRow->addWidget(presetLabel); - presetRow->addWidget(presetCombo, 1); - paintLay->addLayout(presetRow); - - auto* presetBtnRow = new QHBoxLayout(); - auto* savePresetBtn = new QPushButton(tr("Save as…"), paintSettings); - auto* delPresetBtn = new QPushButton(tr("Delete"), paintSettings); - savePresetBtn->setStyleSheet(paintToggleStyle); - delPresetBtn->setStyleSheet(paintToggleStyle); - auto* exportPresetBtn = new QPushButton(tr("Export…"), paintSettings); - auto* importPresetBtn = new QPushButton(tr("Import…"), paintSettings); - exportPresetBtn->setStyleSheet(paintToggleStyle); - importPresetBtn->setStyleSheet(paintToggleStyle); - presetBtnRow->addWidget(savePresetBtn); - presetBtnRow->addWidget(delPresetBtn); - presetBtnRow->addStretch(); - paintLay->addLayout(presetBtnRow); - - auto* presetIoRow = new QHBoxLayout(); - presetIoRow->addWidget(exportPresetBtn); - presetIoRow->addWidget(importPresetBtn); - presetIoRow->addStretch(); - paintLay->addLayout(presetIoRow); - - // Rebuild on demand so presets saved this session appear without a - // restart; block signals so repopulating cannot re-trigger an apply. - auto refreshPresets = [tpcPaint, presetCombo, delPresetBtn]() { - const QString current = presetCombo->currentText(); - QSignalBlocker b(presetCombo); - presetCombo->clear(); - presetCombo->addItems(tpcPaint->brushPresetNames()); - const int idx = presetCombo->findText(current); - if (idx >= 0) presetCombo->setCurrentIndex(idx); - // Bundled presets are compiled in, so Delete could only remove a - // user override — disable it rather than appear to delete something - // that returns on restart. - delPresetBtn->setEnabled( - !presetCombo->currentText().isEmpty() - && !tpcPaint->isBundledBrushPreset(presetCombo->currentText())); - }; - refreshPresets(); - - connect(presetCombo, QOverload::of(&QComboBox::activated), this, - [tpcPaint, presetCombo, delPresetBtn](int idx) { - if (idx < 0) return; - const QString name = presetCombo->itemText(idx); - SentryReporter::addBreadcrumb("ui.action", - QStringLiteral("paint preset apply %1").arg(name)); - tpcPaint->applyBrushPreset(name); - delPresetBtn->setEnabled(!tpcPaint->isBundledBrushPreset(name)); - }); - - connect(savePresetBtn, &QPushButton::clicked, this, - [this, tpcPaint, refreshPresets, presetCombo]() { - bool ok = false; - const QString name = QInputDialog::getText( - this, tr("Save Brush Preset"), tr("Preset name:"), - QLineEdit::Normal, QString(), &ok); - if (!ok || name.trimmed().isEmpty()) return; - if (tpcPaint->isBundledBrushPreset(name.trimmed())) { - QMessageBox::information( - this, tr("Save Brush Preset"), - tr("'%1' is a bundled preset name. Saving will " - "override it; the bundled version returns if you " - "delete the override.").arg(name.trimmed())); - } - if (!tpcPaint->saveBrushPreset(name)) { - QMessageBox::warning(this, tr("Save Brush Preset"), - tr("Could not save the preset.")); - return; - } - refreshPresets(); - const int i = presetCombo->findText(name.trimmed()); - if (i >= 0) presetCombo->setCurrentIndex(i); - }); - - connect(delPresetBtn, &QPushButton::clicked, this, - [this, tpcPaint, refreshPresets, presetCombo]() { - const QString name = presetCombo->currentText(); - if (name.isEmpty()) return; - if (QMessageBox::question( - this, tr("Delete Brush Preset"), - tr("Delete the preset '%1'?").arg(name)) - != QMessageBox::Yes) return; - tpcPaint->deleteBrushPreset(name); - refreshPresets(); - }); - - connect(exportPresetBtn, &QPushButton::clicked, this, - [this, tpcPaint, presetCombo]() { - const QString name = presetCombo->currentText(); - if (name.isEmpty()) return; - QFileDialog dlg(this, tr("Export Brush Preset")); - dlg.setAcceptMode(QFileDialog::AcceptSave); - dlg.setDefaultSuffix(QStringLiteral("json")); - dlg.setNameFilter(tr("Brush preset (*.json)")); - // Native dialogs freeze against Ogre GL — the same reason - // every other file dialog in this file sets this. - dlg.setOption(QFileDialog::DontUseNativeDialog, true); - dlg.selectFile(name + QStringLiteral(".json")); - if (dlg.exec() != QDialog::Accepted) return; - const QStringList files = dlg.selectedFiles(); - if (files.isEmpty()) return; - if (!tpcPaint->exportBrushPreset(name, files.first())) { - QMessageBox::warning(this, tr("Export Brush Preset"), - tr("Could not write the preset file.")); - return; - } - SentryReporter::addBreadcrumb("file.export", - QStringLiteral("paint preset export %1").arg(name)); - }); - - connect(importPresetBtn, &QPushButton::clicked, this, - [this, tpcPaint, refreshPresets, presetCombo]() { - QFileDialog dlg(this, tr("Import Brush Preset")); - dlg.setFileMode(QFileDialog::ExistingFile); - dlg.setNameFilter(tr("Brush preset (*.json)")); - dlg.setOption(QFileDialog::DontUseNativeDialog, true); - if (dlg.exec() != QDialog::Accepted) return; - const QStringList files = dlg.selectedFiles(); - if (files.isEmpty()) return; - const QString imported = tpcPaint->importBrushPreset(files.first()); - if (imported.isEmpty()) { - QMessageBox::warning(this, tr("Import Brush Preset"), - tr("That file is not a valid brush preset.")); - return; - } - SentryReporter::addBreadcrumb("file.import", - QStringLiteral("paint preset import %1").arg(imported)); - refreshPresets(); - const int i = presetCombo->findText(imported); - if (i >= 0) presetCombo->setCurrentIndex(i); - }); - } + buildBrushPresetSection(paintSettings, paintLay, paintToggleStyle); addSectionSeparator(); - - // ---- Paint v2 Slice H (#551): colour swatches -------------------------- - { - auto* palRow = new QHBoxLayout(); - auto* palLabel = new QLabel(tr("Palette:"), paintSettings); - auto* palCombo = new QComboBox(paintSettings); - palCombo->setMinimumWidth(150); - palRow->addWidget(palLabel); - palRow->addWidget(palCombo, 1); - paintLay->addLayout(palRow); - - // 5 columns, as the issue specifies; rows grow with the palette. - auto* swatchHost = new QWidget(paintSettings); - auto* swatchGrid = new QGridLayout(swatchHost); - swatchGrid->setContentsMargins(0, 2, 0, 2); - swatchGrid->setSpacing(3); - paintLay->addWidget(swatchHost); - - auto* recentLabel = new QLabel(tr("Recent:"), paintSettings); - recentLabel->setStyleSheet(QStringLiteral("color: palette(text); font-size: 11px;")); - paintLay->addWidget(recentLabel); - auto* recentHost = new QWidget(paintSettings); - auto* recentGrid = new QGridLayout(recentHost); - recentGrid->setContentsMargins(0, 0, 0, 2); - recentGrid->setSpacing(3); - paintLay->addWidget(recentHost); - - // Shared tile factory: left-click sets FG, right-click sets BG. The BG - // binding matches the existing FG/BG toolbar swatch's semantics. - auto makeSwatchTile = [this, tpcPaint](QWidget* parent, const QString& hex) { - auto* b = new QPushButton(parent); - b->setFixedSize(22, 22); - b->setToolTip(tr("%1 — click to set foreground, right-click for background") - .arg(hex)); - b->setStyleSheet(QStringLiteral( - "QPushButton { background-color: %1; border: 1px solid palette(mid);" - " border-radius: 3px; }").arg(hex)); - b->setContextMenuPolicy(Qt::CustomContextMenu); - connect(b, &QPushButton::clicked, this, [tpcPaint, hex]() { - tpcPaint->applyPaletteColor(hex, /*asBackground=*/false); - }); - connect(b, &QPushButton::customContextMenuRequested, this, - [tpcPaint, hex](const QPoint&) { - tpcPaint->applyPaletteColor(hex, /*asBackground=*/true); - }); - return b; - }; - - auto clearGrid = [](QGridLayout* g) { - while (QLayoutItem* it = g->takeAt(0)) { - if (QWidget* w = it->widget()) w->deleteLater(); - delete it; - } - }; - - auto refreshSwatches = [tpcPaint, palCombo, swatchGrid, clearGrid, - makeSwatchTile, swatchHost]() { - clearGrid(swatchGrid); - const QStringList hexes = tpcPaint->colorPaletteSwatches(palCombo->currentText()); - int i = 0; - for (const QString& hex : hexes) { - swatchGrid->addWidget(makeSwatchTile(swatchHost, hex), i / 5, i % 5); - ++i; - } - }; - - auto refreshRecent = [tpcPaint, recentGrid, clearGrid, makeSwatchTile, - recentHost, recentLabel]() { - clearGrid(recentGrid); - const QStringList hexes = tpcPaint->recentPaintColors(); - // Hide the whole row until something has been picked, rather than - // showing an empty "Recent:" heading. - recentLabel->setVisible(!hexes.isEmpty()); - recentHost->setVisible(!hexes.isEmpty()); - int i = 0; - for (const QString& hex : hexes) { - recentGrid->addWidget(makeSwatchTile(recentHost, hex), i / 6, i % 6); - ++i; - } - }; - - auto refreshPalettes = [tpcPaint, palCombo, refreshSwatches]() { - const QString current = palCombo->currentText(); - QSignalBlocker b(palCombo); - palCombo->clear(); - palCombo->addItems(tpcPaint->colorPaletteNames()); - const int idx = palCombo->findText(current); - palCombo->setCurrentIndex(idx >= 0 ? idx : 0); - refreshSwatches(); - }; - refreshPalettes(); - refreshRecent(); - - connect(palCombo, QOverload::of(&QComboBox::activated), this, - [refreshSwatches](int) { refreshSwatches(); }); - - auto* fromTexBtn = new QPushButton(tr("Palette from texture…"), paintSettings); - fromTexBtn->setStyleSheet(paintToggleStyle); - paintLay->addWidget(fromTexBtn); - connect(fromTexBtn, &QPushButton::clicked, this, - [this, tpcPaint, refreshPalettes]() { - bool ok = false; - const QString name = QInputDialog::getText( - this, tr("Palette From Texture"), tr("Palette name:"), - QLineEdit::Normal, QString(), &ok); - if (!ok || name.trimmed().isEmpty()) return; - if (!tpcPaint->savePaletteFromTexture(name)) { - QMessageBox::warning( - this, tr("Palette From Texture"), - tr("Could not extract colours — paint a texture first.")); - return; - } - refreshPalettes(); - }); - - // The controller emits paletteChanged when the recent ring or the - // palette list changes, so the grids stay in step with painting. - connect(tpcPaint, &TexturePaintController::paletteChanged, this, - [refreshRecent]() { refreshRecent(); }); - } + buildColorSwatchSection(paintSettings, paintLay, paintToggleStyle); paintSettings->setMinimumWidth(280); paintSettings->adjustSize(); @@ -6814,108 +6552,385 @@ void MainWindow::setMCPServer(MCPServer* server) AIChatManager::instance()->setMcpServer(m_mcpServer); } -void MainWindow::addToRecentFiles(const QString& filePath) -{ - QSettings settings; - QStringList files = settings.value("RecentFiles/files").toStringList(); - files.removeAll(filePath); - files.prepend(filePath); - int maxRecent = settings.value("General/recentFilesCount", 10).toInt(); - while (files.size() > maxRecent) - files.removeLast(); - settings.setValue("RecentFiles/files", files); - updateRecentFilesMenu(); +void MainWindow::addToRecentFiles(const QString& filePath) +{ + QSettings settings; + QStringList files = settings.value("RecentFiles/files").toStringList(); + files.removeAll(filePath); + files.prepend(filePath); + int maxRecent = settings.value("General/recentFilesCount", 10).toInt(); + while (files.size() > maxRecent) + files.removeLast(); + settings.setValue("RecentFiles/files", files); + updateRecentFilesMenu(); + + // Keep the welcome screen's recent files list in sync + if (m_welcomeController) + emit m_welcomeController->recentFilesChanged(); +} + +void MainWindow::updateRecentFilesMenu() +{ + m_recentFilesMenu->clear(); + + QSettings settings; + if (QStringList files = settings.value("RecentFiles/files").toStringList(); files.isEmpty()) { + auto* noFilesAction = m_recentFilesMenu->addAction(tr("(No Recent Files)")); + noFilesAction->setEnabled(false); + } else { + for (const QString& filePath : files) { + QFileInfo fi(filePath); + auto* action = m_recentFilesMenu->addAction(fi.fileName()); + action->setData(filePath); + action->setToolTip(filePath); + connect(action, &QAction::triggered, this, &MainWindow::openRecentFile); + } + } + + m_recentFilesMenu->addSeparator(); + const auto* clearAction = m_recentFilesMenu->addAction(tr("Clear Recent Files")); + connect(clearAction, &QAction::triggered, this, [this]() { + QSettings settings; + settings.remove("RecentFiles/files"); + updateRecentFilesMenu(); + if (m_welcomeController) + emit m_welcomeController->recentFilesChanged(); + }); +} + +void MainWindow::openRecentFile() +{ + const auto* action = qobject_cast(sender()); + if (!action) + return; + + QString filePath = action->data().toString(); + if (QFileInfo::exists(filePath)) { + addToRecentFiles(filePath); + if (filePath.endsWith(".scene.glb") || filePath.endsWith(".scene.gltf")) + MeshImporterExporter::sceneImporter(filePath); + else + mUriList.append(filePath); + } else { + QMessageBox::warning(this, tr("File Not Found"), + tr("The file \"%1\" no longer exists.").arg(filePath)); + QSettings settings; + QStringList files = settings.value("RecentFiles/files").toStringList(); + files.removeAll(filePath); + settings.setValue("RecentFiles/files", files); + updateRecentFilesMenu(); + if (m_welcomeController) + emit m_welcomeController->recentFilesChanged(); + } +} + +// LCOV_EXCL_START — requires display +void MainWindow::showWelcomeScreen() +{ + if (!m_welcomeScreen) return; + repositionWelcomeScreen(); + m_welcomeScreen->show(); + m_welcomeScreen->raise(); + m_welcomeScreen->setFocus(); + + // Hide the ViewCube while welcome screen is showing (it has WindowStaysOnTopHint) + if (m_viewCubeController) + m_viewCubeController->setVisible(false); +} + +void MainWindow::hideWelcomeScreen() +{ + if (m_welcomeScreen) + m_welcomeScreen->hide(); + + // Restore ViewCube visibility based on the menu toggle state + if (m_viewCubeController && ui->actionShow_View_Cube->isChecked()) + m_viewCubeController->setVisible(true); +} + +void MainWindow::repositionWelcomeScreen() +{ + if (!m_welcomeScreen) return; + + // Cover the entire main window — the QML overlay has its own + // semi-transparent background that handles the visual layering. + m_welcomeScreen->setGeometry(rect()); +} +// LCOV_EXCL_STOP + +// --------------------------------------------------------------------------- +// Paint v2 Slice H (#551): brush portal sections. +// Split out of initToolBar (already ~2000 lines) so this slice does not push +// its cognitive complexity past the Sonar gate. `toggleStyle` is initToolBar's +// shared checkable-button stylesheet, passed in rather than recomputed so the +// buttons match the rest of the portal. +// --------------------------------------------------------------------------- + +void MainWindow::buildBrushPresetSection(QWidget* paintSettings, QVBoxLayout* paintLay, + const QString& toggleStyle) +{ + auto* tpcPaint = TexturePaintController::instance(); + if (!tpcPaint || !paintSettings || !paintLay) return; + const QString paintToggleStyle = toggleStyle; + auto* presetRow = new QHBoxLayout(); + auto* presetLabel = new QLabel(tr("Preset:"), paintSettings); + auto* presetCombo = new QComboBox(paintSettings); + presetCombo->setMinimumWidth(150); + presetRow->addWidget(presetLabel); + presetRow->addWidget(presetCombo, 1); + paintLay->addLayout(presetRow); + + auto* presetBtnRow = new QHBoxLayout(); + auto* savePresetBtn = new QPushButton(tr("Save as…"), paintSettings); + auto* delPresetBtn = new QPushButton(tr("Delete"), paintSettings); + savePresetBtn->setStyleSheet(paintToggleStyle); + delPresetBtn->setStyleSheet(paintToggleStyle); + auto* exportPresetBtn = new QPushButton(tr("Export…"), paintSettings); + auto* importPresetBtn = new QPushButton(tr("Import…"), paintSettings); + exportPresetBtn->setStyleSheet(paintToggleStyle); + importPresetBtn->setStyleSheet(paintToggleStyle); + presetBtnRow->addWidget(savePresetBtn); + presetBtnRow->addWidget(delPresetBtn); + presetBtnRow->addStretch(); + paintLay->addLayout(presetBtnRow); + + auto* presetIoRow = new QHBoxLayout(); + presetIoRow->addWidget(exportPresetBtn); + presetIoRow->addWidget(importPresetBtn); + presetIoRow->addStretch(); + paintLay->addLayout(presetIoRow); + + // Rebuild on demand so presets saved this session appear without a + // restart; block signals so repopulating cannot re-trigger an apply. + auto refreshPresets = [tpcPaint, presetCombo, delPresetBtn]() { + const QString current = presetCombo->currentText(); + QSignalBlocker b(presetCombo); + presetCombo->clear(); + presetCombo->addItems(tpcPaint->brushPresetNames()); + const int idx = presetCombo->findText(current); + if (idx >= 0) presetCombo->setCurrentIndex(idx); + // Bundled presets are compiled in, so Delete could only remove a + // user override — disable it rather than appear to delete something + // that returns on restart. + delPresetBtn->setEnabled( + !presetCombo->currentText().isEmpty() + && !tpcPaint->isBundledBrushPreset(presetCombo->currentText())); + }; + refreshPresets(); + + connect(presetCombo, QOverload::of(&QComboBox::activated), this, + [tpcPaint, presetCombo, delPresetBtn](int idx) { + if (idx < 0) return; + const QString name = presetCombo->itemText(idx); + SentryReporter::addBreadcrumb("ui.action", + QStringLiteral("paint preset apply %1").arg(name)); + tpcPaint->applyBrushPreset(name); + delPresetBtn->setEnabled(!tpcPaint->isBundledBrushPreset(name)); + }); + + connect(savePresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPresets, presetCombo]() { + bool ok = false; + const QString name = QInputDialog::getText( + this, tr("Save Brush Preset"), tr("Preset name:"), + QLineEdit::Normal, QString(), &ok); + if (!ok || name.trimmed().isEmpty()) return; + if (tpcPaint->isBundledBrushPreset(name.trimmed())) { + QMessageBox::information( + this, tr("Save Brush Preset"), + tr("'%1' is a bundled preset name. Saving will " + "override it; the bundled version returns if you " + "delete the override.").arg(name.trimmed())); + } + if (!tpcPaint->saveBrushPreset(name)) { + QMessageBox::warning(this, tr("Save Brush Preset"), + tr("Could not save the preset.")); + return; + } + refreshPresets(); + const int i = presetCombo->findText(name.trimmed()); + if (i >= 0) presetCombo->setCurrentIndex(i); + }); + + connect(delPresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPresets, presetCombo]() { + const QString name = presetCombo->currentText(); + if (name.isEmpty()) return; + if (QMessageBox::question( + this, tr("Delete Brush Preset"), + tr("Delete the preset '%1'?").arg(name)) + != QMessageBox::Yes) return; + tpcPaint->deleteBrushPreset(name); + refreshPresets(); + }); - // Keep the welcome screen's recent files list in sync - if (m_welcomeController) - emit m_welcomeController->recentFilesChanged(); + connect(exportPresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, presetCombo]() { + const QString name = presetCombo->currentText(); + if (name.isEmpty()) return; + QFileDialog dlg(this, tr("Export Brush Preset")); + dlg.setAcceptMode(QFileDialog::AcceptSave); + dlg.setDefaultSuffix(QStringLiteral("json")); + dlg.setNameFilter(tr("Brush preset (*.json)")); + // Native dialogs freeze against Ogre GL — the same reason + // every other file dialog in this file sets this. + dlg.setOption(QFileDialog::DontUseNativeDialog, true); + dlg.selectFile(name + QStringLiteral(".json")); + if (dlg.exec() != QDialog::Accepted) return; + const QStringList files = dlg.selectedFiles(); + if (files.isEmpty()) return; + if (!tpcPaint->exportBrushPreset(name, files.first())) { + QMessageBox::warning(this, tr("Export Brush Preset"), + tr("Could not write the preset file.")); + return; + } + SentryReporter::addBreadcrumb("file.export", + QStringLiteral("paint preset export %1").arg(name)); + }); + + connect(importPresetBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPresets, presetCombo]() { + QFileDialog dlg(this, tr("Import Brush Preset")); + dlg.setFileMode(QFileDialog::ExistingFile); + dlg.setNameFilter(tr("Brush preset (*.json)")); + dlg.setOption(QFileDialog::DontUseNativeDialog, true); + if (dlg.exec() != QDialog::Accepted) return; + const QStringList files = dlg.selectedFiles(); + if (files.isEmpty()) return; + const QString imported = tpcPaint->importBrushPreset(files.first()); + if (imported.isEmpty()) { + QMessageBox::warning(this, tr("Import Brush Preset"), + tr("That file is not a valid brush preset.")); + return; + } + SentryReporter::addBreadcrumb("file.import", + QStringLiteral("paint preset import %1").arg(imported)); + refreshPresets(); + const int i = presetCombo->findText(imported); + if (i >= 0) presetCombo->setCurrentIndex(i); + }); } -void MainWindow::updateRecentFilesMenu() +void MainWindow::buildColorSwatchSection(QWidget* paintSettings, QVBoxLayout* paintLay, + const QString& toggleStyle) { - m_recentFilesMenu->clear(); + auto* tpcPaint = TexturePaintController::instance(); + if (!tpcPaint || !paintSettings || !paintLay) return; + const QString paintToggleStyle = toggleStyle; + auto* palRow = new QHBoxLayout(); + auto* palLabel = new QLabel(tr("Palette:"), paintSettings); + auto* palCombo = new QComboBox(paintSettings); + palCombo->setMinimumWidth(150); + palRow->addWidget(palLabel); + palRow->addWidget(palCombo, 1); + paintLay->addLayout(palRow); - QSettings settings; - if (QStringList files = settings.value("RecentFiles/files").toStringList(); files.isEmpty()) { - auto* noFilesAction = m_recentFilesMenu->addAction(tr("(No Recent Files)")); - noFilesAction->setEnabled(false); - } else { - for (const QString& filePath : files) { - QFileInfo fi(filePath); - auto* action = m_recentFilesMenu->addAction(fi.fileName()); - action->setData(filePath); - action->setToolTip(filePath); - connect(action, &QAction::triggered, this, &MainWindow::openRecentFile); - } - } + // 5 columns, as the issue specifies; rows grow with the palette. + auto* swatchHost = new QWidget(paintSettings); + auto* swatchGrid = new QGridLayout(swatchHost); + swatchGrid->setContentsMargins(0, 2, 0, 2); + swatchGrid->setSpacing(3); + paintLay->addWidget(swatchHost); - m_recentFilesMenu->addSeparator(); - const auto* clearAction = m_recentFilesMenu->addAction(tr("Clear Recent Files")); - connect(clearAction, &QAction::triggered, this, [this]() { - QSettings settings; - settings.remove("RecentFiles/files"); - updateRecentFilesMenu(); - if (m_welcomeController) - emit m_welcomeController->recentFilesChanged(); - }); -} + auto* recentLabel = new QLabel(tr("Recent:"), paintSettings); + recentLabel->setStyleSheet(QStringLiteral("color: palette(text); font-size: 11px;")); + paintLay->addWidget(recentLabel); + auto* recentHost = new QWidget(paintSettings); + auto* recentGrid = new QGridLayout(recentHost); + recentGrid->setContentsMargins(0, 0, 0, 2); + recentGrid->setSpacing(3); + paintLay->addWidget(recentHost); -void MainWindow::openRecentFile() -{ - const auto* action = qobject_cast(sender()); - if (!action) - return; + // Shared tile factory: left-click sets FG, right-click sets BG. The BG + // binding matches the existing FG/BG toolbar swatch's semantics. + auto makeSwatchTile = [this, tpcPaint](QWidget* parent, const QString& hex) { + auto* b = new QPushButton(parent); + b->setFixedSize(22, 22); + b->setToolTip(tr("%1 — click to set foreground, right-click for background") + .arg(hex)); + b->setStyleSheet(QStringLiteral( + "QPushButton { background-color: %1; border: 1px solid palette(mid);" + " border-radius: 3px; }").arg(hex)); + b->setContextMenuPolicy(Qt::CustomContextMenu); + connect(b, &QPushButton::clicked, this, [tpcPaint, hex]() { + tpcPaint->applyPaletteColor(hex, /*asBackground=*/false); + }); + connect(b, &QPushButton::customContextMenuRequested, this, + [tpcPaint, hex](const QPoint&) { + tpcPaint->applyPaletteColor(hex, /*asBackground=*/true); + }); + return b; + }; - QString filePath = action->data().toString(); - if (QFileInfo::exists(filePath)) { - addToRecentFiles(filePath); - if (filePath.endsWith(".scene.glb") || filePath.endsWith(".scene.gltf")) - MeshImporterExporter::sceneImporter(filePath); - else - mUriList.append(filePath); - } else { - QMessageBox::warning(this, tr("File Not Found"), - tr("The file \"%1\" no longer exists.").arg(filePath)); - QSettings settings; - QStringList files = settings.value("RecentFiles/files").toStringList(); - files.removeAll(filePath); - settings.setValue("RecentFiles/files", files); - updateRecentFilesMenu(); - if (m_welcomeController) - emit m_welcomeController->recentFilesChanged(); - } -} + auto clearGrid = [](QGridLayout* g) { + while (QLayoutItem* it = g->takeAt(0)) { + if (QWidget* w = it->widget()) w->deleteLater(); + delete it; + } + }; -// LCOV_EXCL_START — requires display -void MainWindow::showWelcomeScreen() -{ - if (!m_welcomeScreen) return; - repositionWelcomeScreen(); - m_welcomeScreen->show(); - m_welcomeScreen->raise(); - m_welcomeScreen->setFocus(); + auto refreshSwatches = [tpcPaint, palCombo, swatchGrid, clearGrid, + makeSwatchTile, swatchHost]() { + clearGrid(swatchGrid); + const QStringList hexes = tpcPaint->colorPaletteSwatches(palCombo->currentText()); + int i = 0; + for (const QString& hex : hexes) { + swatchGrid->addWidget(makeSwatchTile(swatchHost, hex), i / 5, i % 5); + ++i; + } + }; - // Hide the ViewCube while welcome screen is showing (it has WindowStaysOnTopHint) - if (m_viewCubeController) - m_viewCubeController->setVisible(false); -} + auto refreshRecent = [tpcPaint, recentGrid, clearGrid, makeSwatchTile, + recentHost, recentLabel]() { + clearGrid(recentGrid); + const QStringList hexes = tpcPaint->recentPaintColors(); + // Hide the whole row until something has been picked, rather than + // showing an empty "Recent:" heading. + recentLabel->setVisible(!hexes.isEmpty()); + recentHost->setVisible(!hexes.isEmpty()); + int i = 0; + for (const QString& hex : hexes) { + recentGrid->addWidget(makeSwatchTile(recentHost, hex), i / 6, i % 6); + ++i; + } + }; -void MainWindow::hideWelcomeScreen() -{ - if (m_welcomeScreen) - m_welcomeScreen->hide(); + auto refreshPalettes = [tpcPaint, palCombo, refreshSwatches]() { + const QString current = palCombo->currentText(); + QSignalBlocker b(palCombo); + palCombo->clear(); + palCombo->addItems(tpcPaint->colorPaletteNames()); + const int idx = palCombo->findText(current); + palCombo->setCurrentIndex(idx >= 0 ? idx : 0); + refreshSwatches(); + }; + refreshPalettes(); + refreshRecent(); - // Restore ViewCube visibility based on the menu toggle state - if (m_viewCubeController && ui->actionShow_View_Cube->isChecked()) - m_viewCubeController->setVisible(true); -} + connect(palCombo, QOverload::of(&QComboBox::activated), this, + [refreshSwatches](int) { refreshSwatches(); }); -void MainWindow::repositionWelcomeScreen() -{ - if (!m_welcomeScreen) return; + auto* fromTexBtn = new QPushButton(tr("Palette from texture…"), paintSettings); + fromTexBtn->setStyleSheet(paintToggleStyle); + paintLay->addWidget(fromTexBtn); + connect(fromTexBtn, &QPushButton::clicked, this, + [this, tpcPaint, refreshPalettes]() { + bool ok = false; + const QString name = QInputDialog::getText( + this, tr("Palette From Texture"), tr("Palette name:"), + QLineEdit::Normal, QString(), &ok); + if (!ok || name.trimmed().isEmpty()) return; + if (!tpcPaint->savePaletteFromTexture(name)) { + QMessageBox::warning( + this, tr("Palette From Texture"), + tr("Could not extract colours — paint a texture first.")); + return; + } + refreshPalettes(); + }); - // Cover the entire main window — the QML overlay has its own - // semi-transparent background that handles the visual layering. - m_welcomeScreen->setGeometry(rect()); + // The controller emits paletteChanged when the recent ring or the + // palette list changes, so the grids stay in step with painting. + connect(tpcPaint, &TexturePaintController::paletteChanged, this, + [refreshRecent]() { refreshRecent(); }); } -// LCOV_EXCL_STOP diff --git a/src/mainwindow.h b/src/mainwindow.h index f245b806..fd85c68e 100755 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -25,6 +25,7 @@ class EditorModeController; class WelcomeScreenController; class AssetBrowserController; class QQuickWidget; +class QVBoxLayout; class QQmlApplicationEngine; class QPlainTextEdit; class QLabel; @@ -201,6 +202,15 @@ public slots: private: void initToolBar(); + /// Paint v2 Slice H (#551): the brush-preset and colour-swatch sections of + /// the brush portal. Split out of initToolBar (already ~2000 lines) so this + /// slice does not push its cognitive complexity past the Sonar gate. + /// `paintSettings`/`paintLay` are the portal's host widget + layout; + /// `toggleStyle` is initToolBar's shared checkable-button stylesheet. + void buildBrushPresetSection(QWidget* paintSettings, QVBoxLayout* paintLay, + const QString& toggleStyle); + void buildColorSwatchSection(QWidget* paintSettings, QVBoxLayout* paintLay, + const QString& toggleStyle); void updateMergeAnimationsButton(); const QPalette& darkPalette(); From 670ffdd7148bc9c39102b6e048762b98d1bc4957 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 21:13:32 -0400 Subject: [PATCH 6/7] =?UTF-8?q?fix(#551):=20address=20PR=20#970=20review?= =?UTF-8?q?=20=E2=80=94=20all=206=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All six were real; two contradicted claims in my own design doc. **1. Applying a swatch forced the brush opaque (Codex P2).** `QColor(r,g,b)` is alpha 255, so every palette pick silently discarded a partially transparent brush — directly contradicting the documented "swatches carry no alpha so your brush alpha is untouched" design. Now copies the existing FG/BG alpha. Mutation-checked: removing the copy fails the new test. **2. Recent ring missed colours set outside the palette (Codex P2).** The toolbar swatch and pickBrushColorInteractive() call setVertexPaintColor directly, so Recent only ever showed colours RE-picked from tiles, never a newly chosen one. Now fed from EditModeController's shared vertexPaintChanged signal via noteRecentForegroundColor(), which no-ops when the colour is already newest so the extra notifications that signal produces cannot churn the ring. **3 + 5. FG/BG ramp mode was neither saved nor restored (both reviewers).** saveBrushPreset always recorded activeRampName() and applyBrushPreset never called setUseFgBgRamp, so an FG/BG gradient preset restored whichever NAMED ramp happened to be active. Added an explicit `useFgBgRamp` field (in the struct and the JSON) rather than inferring it from an empty rampName — inferring would make an older file with no name silently become FG/BG. **4. Non-atomic writes risked user data loss (CodeRabbit, Major).** All three write paths (custom preset, preset export, custom palette) truncated the destination before writing. A short write or interruption could destroy an existing file. All three now use QSaveFile: temp file + commit-by-rename, so a failure preserves the previous file. **6. A custom preset shadowing a bundled name could not be deleted (CodeRabbit).** My own allPresets() lets custom override bundled, but deleteBrushPreset refused any bundled name — so saving a custom "Soft Round" would permanently hide the bundled one with no way back. Deletion now means "remove the custom file", gated by the new canDeleteBrushPreset(), which the UI uses for its Delete button too. Tests: 42/42 Slice H (4 new: alpha preservation, recent ring from the shared path, FG/BG round-trip, and deleting a shadowing override restoring the bundled preset), plus 40/40 across the existing paint suites. Co-Authored-By: Claude Opus 5 (1M context) --- src/BrushPresetLibrary.cpp | 31 ++++++---- src/BrushPresetLibrary.h | 6 +- src/BrushPresetLibrary_test.cpp | 15 +++++ src/ColorPaletteLibrary.cpp | 12 ++-- src/TexturePaintController.cpp | 71 ++++++++++++++++++----- src/TexturePaintController.h | 8 +++ src/TexturePaintController_test.cpp | 88 +++++++++++++++++++++++++++++ src/mainwindow.cpp | 11 ++-- 8 files changed, 207 insertions(+), 35 deletions(-) diff --git a/src/BrushPresetLibrary.cpp b/src/BrushPresetLibrary.cpp index 795e504e..ed4f74dd 100644 --- a/src/BrushPresetLibrary.cpp +++ b/src/BrushPresetLibrary.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -171,6 +172,7 @@ std::string toJson(const Preset& p) o["stampAngleDeg"] = p.stampAngleDeg; o["colorSource"] = p.colorSource; o["gradientMode"] = p.gradientMode; + o["useFgBgRamp"] = p.useFgBgRamp; o["rampName"] = QString::fromStdString(p.rampName); o["note"] = QString::fromStdString(p.note); return QJsonDocument(o).toJson(QJsonDocument::Compact).toStdString(); @@ -206,6 +208,7 @@ bool fromJson(const std::string& json, Preset& out) p.stampAngleDeg = o.value("stampAngleDeg").toDouble(p.stampAngleDeg); p.colorSource = o.value("colorSource").toInt(p.colorSource); p.gradientMode = o.value("gradientMode").toInt(p.gradientMode); + p.useFgBgRamp = o.value("useFgBgRamp").toBool(p.useFgBgRamp); p.rampName = o.value("rampName").toString().toStdString(); p.note = o.value("note").toString().toStdString(); @@ -245,16 +248,18 @@ std::string saveCustom(const Preset& p) const QString path = QDir(QString::fromStdString(dir)) .filePath(QString::fromStdString(customFileStem(p.name)) + QStringLiteral(".json")); - QFile f(path); - if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return {}; + // QSaveFile writes to a temp file and commits by rename, so an + // interrupted or short write leaves the PREVIOUS file intact instead of a + // truncated one. Truncating in place risks losing a user's saved preset. + QSaveFile f(path); + if (!f.open(QIODevice::WriteOnly)) return {}; const std::string json = toJson(p); if (f.write(json.data(), static_cast(json.size())) != static_cast(json.size())) { - f.close(); - QFile::remove(path); // never leave a truncated preset behind + f.cancelWriting(); return {}; } - f.close(); + if (!f.commit()) return {}; return path.toStdString(); } @@ -326,14 +331,16 @@ bool findPreset(const std::string& name, Preset& out) bool exportToFile(const Preset& p, const std::string& path) { if (!p.isValid() || path.empty()) return false; - QFile f(QString::fromStdString(path)); - if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return false; + // Atomic: exporting over an existing file must not destroy it on failure. + QSaveFile f(QString::fromStdString(path)); + if (!f.open(QIODevice::WriteOnly)) return false; const std::string json = toJson(p); - const bool ok = f.write(json.data(), static_cast(json.size())) - == static_cast(json.size()); - f.close(); - if (!ok) QFile::remove(QString::fromStdString(path)); - return ok; + if (f.write(json.data(), static_cast(json.size())) + != static_cast(json.size())) { + f.cancelWriting(); + return false; + } + return f.commit(); } bool importFromFile(const std::string& path, Preset& out) diff --git a/src/BrushPresetLibrary.h b/src/BrushPresetLibrary.h index 0f451fa9..35743feb 100644 --- a/src/BrushPresetLibrary.h +++ b/src/BrushPresetLibrary.h @@ -62,7 +62,11 @@ struct Preset { // --- colour source --- int colorSource = 0; ///< ColorSource (0 Solid, 1 Gradient) int gradientMode = 0; ///< GradientMode (0 Linear, 1 Radial, 2 Angular) - std::string rampName; ///< gradient ramp name; empty = FG/BG + /// True = use the FG/BG two-stop ramp instead of `rampName`. Stored + /// explicitly rather than inferred from an empty rampName, so applying a + /// preset cannot fall back to whichever named ramp was previously active. + bool useFgBgRamp = false; + std::string rampName; ///< gradient ramp name; ignored when useFgBgRamp /// Optional one-line description shown as a tooltip. std::string note; diff --git a/src/BrushPresetLibrary_test.cpp b/src/BrushPresetLibrary_test.cpp index 063b818b..fb375b79 100644 --- a/src/BrushPresetLibrary_test.cpp +++ b/src/BrushPresetLibrary_test.cpp @@ -250,3 +250,18 @@ TEST_F(BrushPresetLibraryFileTest, NamesCollidingAfterSanitisationDoNotOverwrite BrushPresetLibrary::deleteCustom(a.name); BrushPresetLibrary::deleteCustom(b.name); } + +TEST(BrushPresetLibraryTest, JsonRoundTripsFgBgRampFlag) { + // The FG/BG flag must be explicit in JSON: inferring it from an empty + // rampName would make an old file with no name silently become FG/BG. + BrushPresetLibrary::Preset p; + p.name = "FgBg"; p.colorSource = 1; p.useFgBgRamp = true; + BrushPresetLibrary::Preset b; + ASSERT_TRUE(BrushPresetLibrary::fromJson(BrushPresetLibrary::toJson(p), b)); + EXPECT_TRUE(b.useFgBgRamp); + + p.useFgBgRamp = false; p.rampName = "Sunset"; + ASSERT_TRUE(BrushPresetLibrary::fromJson(BrushPresetLibrary::toJson(p), b)); + EXPECT_FALSE(b.useFgBgRamp); + EXPECT_EQ(b.rampName, "Sunset"); +} diff --git a/src/ColorPaletteLibrary.cpp b/src/ColorPaletteLibrary.cpp index fbe7f604..b9f46750 100644 --- a/src/ColorPaletteLibrary.cpp +++ b/src/ColorPaletteLibrary.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -207,16 +208,17 @@ std::string saveCustom(const Palette& p) const QString path = QDir(QString::fromStdString(dir)) .filePath(QString::fromStdString(customFileStem(p.name)) + QStringLiteral(".json")); - QFile f(path); - if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return {}; + // QSaveFile: temp file + rename on commit, so an interrupted or short + // write preserves the previous palette instead of truncating it. + QSaveFile f(path); + if (!f.open(QIODevice::WriteOnly)) return {}; const std::string json = toJson(p); if (f.write(json.data(), static_cast(json.size())) != static_cast(json.size())) { - f.close(); - QFile::remove(path); // never leave a truncated palette behind + f.cancelWriting(); return {}; } - f.close(); + if (!f.commit()) return {}; return path.toStdString(); } diff --git a/src/TexturePaintController.cpp b/src/TexturePaintController.cpp index 53c24dd1..200bf871 100644 --- a/src/TexturePaintController.cpp +++ b/src/TexturePaintController.cpp @@ -454,6 +454,15 @@ TexturePaintController::TexturePaintController(QObject* parent) if (m_useFgBgRamp) reloadActiveRamp(); }); + // Paint v2 Slice H (#551): feed the recent-colour ring from the SHARED + // foreground-change signal, not just from palette clicks. The toolbar + // swatch and pickBrushColorInteractive() call setVertexPaintColor + // directly, so hooking only applyPaletteColor would leave Recent + // showing solely colours re-picked from tiles — never a newly chosen + // one. pushRecent de-duplicates, so the repeat notifications this + // signal produces are harmless. + connect(em, &EditModeController::vertexPaintChanged, + this, [this]() { noteRecentForegroundColor(); }); } // Restore Paint v2 Slice A preferences. @@ -7279,7 +7288,11 @@ bool TexturePaintController::applyBrushPreset(const QString& name) setColorSource(p.colorSource); setGradientMode(p.gradientMode); - if (!p.rampName.empty()) setActiveRampName(QString::fromStdString(p.rampName)); + // An empty rampName is the FG/BG sentinel: without honouring it, a saved + // FG/BG gradient would restore whichever NAMED ramp happened to be active. + setUseFgBgRamp(p.useFgBgRamp); + if (!p.useFgBgRamp && !p.rampName.empty()) + setActiveRampName(QString::fromStdString(p.rampName)); // Deliberately NOT restored: the paint COLOUR. A preset describes the brush // (its shape and dynamics), not what you are painting with — clobbering the @@ -7312,7 +7325,10 @@ bool TexturePaintController::saveBrushPreset(const QString& name) p.stampAngleDeg = stampFixedAngle(); p.colorSource = colorSource(); p.gradientMode = gradientMode(); - p.rampName = activeRampName().toStdString(); + p.useFgBgRamp = useFgBgRamp(); + // Record an empty name in FG/BG mode so the saved preset does not carry a + // named ramp it is not actually using. + p.rampName = useFgBgRamp() ? std::string() : activeRampName().toStdString(); if (BrushPresetLibrary::saveCustom(p).empty()) return false; SentryReporter::addBreadcrumb("paint.preset.save", trimmed); @@ -7332,12 +7348,21 @@ bool TexturePaintController::isBundledBrushPreset(const QString& name) const return BrushPresetLibrary::isBundled(name.toStdString()); } +bool TexturePaintController::canDeleteBrushPreset(const QString& name) const +{ + if (name.isEmpty()) return false; + for (const auto& p : BrushPresetLibrary::loadCustomPresets()) + if (QString::fromStdString(p.name) == name) return true; + return false; +} + bool TexturePaintController::deleteBrushPreset(const QString& name) { - // Bundled presets are compiled in, so "deleting" one could only remove a - // user override — refuse rather than appear to delete something that comes - // straight back on restart. - if (isBundledBrushPreset(name)) return false; + // Deleting is really "remove the custom file", so it must be allowed for a + // custom preset that SHADOWS a bundled name — otherwise saving a custom + // "Soft Round" would permanently hide the bundled one with no way back. + // Only refuse when there is no custom file to remove. + if (!canDeleteBrushPreset(name)) return false; const bool ok = BrushPresetLibrary::deleteCustom(name.toStdString()); if (ok) SentryReporter::addBreadcrumb("paint.preset.delete", name); return ok; @@ -7388,6 +7413,21 @@ QStringList TexturePaintController::recentPaintColors() const return out; } +void TexturePaintController::noteRecentForegroundColor() +{ + auto* em = EditModeController::instance(); + if (!em) return; + const QColor c = em->vertexPaintColor(); + if (!c.isValid()) return; + ColorPaletteLibrary::Swatch s; + s.r = static_cast(c.red()); + s.g = static_cast(c.green()); + s.b = static_cast(c.blue()); + if (!m_recentColors.empty() && m_recentColors.front() == s) return; // no churn + ColorPaletteLibrary::pushRecent(m_recentColors, s); + emit paletteChanged(); +} + bool TexturePaintController::applyPaletteColor(const QString& hex, bool asBackground) { ColorPaletteLibrary::Swatch s; @@ -7395,16 +7435,21 @@ bool TexturePaintController::applyPaletteColor(const QString& hex, bool asBackgr auto* em = EditModeController::instance(); if (!em) return false; - const QColor c(s.r, s.g, s.b); + // Preserve the EXISTING alpha. Swatches deliberately carry no alpha (a + // palette curates hues), so QColor(r,g,b) — which is opaque — would silently + // force the brush fully opaque on every pick, contradicting that design. + const QColor prev = asBackground ? em->vertexPaintBackgroundColor() + : em->vertexPaintColor(); + QColor c(s.r, s.g, s.b); + c.setAlpha(prev.alpha()); if (asBackground) em->setVertexPaintBackgroundColor(c); else em->setVertexPaintColor(c); - // Only the foreground feeds the recent ring: the background is a secondary - // slot the user changes rarely, and mixing it in would churn the history. - if (!asBackground) { - ColorPaletteLibrary::pushRecent(m_recentColors, s); - emit paletteChanged(); - } + // The recent ring is fed by the shared vertexPaintChanged hook (see the + // ctor), which setVertexPaintColor above has just triggered — so there is + // no direct push here. Only the FOREGROUND enters the ring; the background + // is a rarely-changed secondary slot that would churn the history. + SentryReporter::addBreadcrumb("paint.palette.apply", QStringLiteral("%1%2").arg(hex, asBackground ? QStringLiteral(" (bg)") : QString())); return true; diff --git a/src/TexturePaintController.h b/src/TexturePaintController.h index 32be8c6d..6cb840ec 100644 --- a/src/TexturePaintController.h +++ b/src/TexturePaintController.h @@ -605,6 +605,10 @@ class TexturePaintController : public QObject Q_INVOKABLE QStringList brushPresetNames() const; /// True when `name` is a bundled preset (gates rename/delete in the UI). Q_INVOKABLE bool isBundledBrushPreset(const QString& name) const; + /// True when a CUSTOM file exists for `name` — including a custom preset + /// that shadows a bundled name, which must stay deletable so the bundled + /// version can be restored. Drives the UI's Delete gate. + Q_INVOKABLE bool canDeleteBrushPreset(const QString& name) const; Q_INVOKABLE bool deleteBrushPreset(const QString& name); Q_INVOKABLE bool exportBrushPreset(const QString& name, const QString& path); /// Import a preset file and save it into the user library. Returns the @@ -1238,6 +1242,10 @@ class TexturePaintController : public QObject Ogre::Vector3& outWorld) const; /// Cache the entity's world triangles for the projection stroke (once). + /// Push the live FOREGROUND colour onto the recent ring (no-op when it is + /// already newest). Driven by EditModeController's shared + /// vertexPaintChanged signal so every colour path feeds Recent. + void noteRecentForegroundColor(); void ensureProjTris(); /// Read the View straight off the live viewport camera, ignoring any locked /// pose. `snapProjectionCamera` uses this so a re-snap re-pins to where the diff --git a/src/TexturePaintController_test.cpp b/src/TexturePaintController_test.cpp index d1fe61bb..8db156ff 100644 --- a/src/TexturePaintController_test.cpp +++ b/src/TexturePaintController_test.cpp @@ -1575,3 +1575,91 @@ TEST_F(TexturePaintControllerSceneTest, SavePaletteFromTextureNeedsABuffer) { ctrl->closeSession(); QStandardPaths::setTestModeEnabled(false); } + +// --- Slice H review fixes (#551 / PR #970) -------------------------------- + +TEST_F(TexturePaintControllerSceneTest, PaletteColorPreservesBrushAlpha) { + ASSERT_TRUE(m_fix.setup(QStringLiteral("PaletteAlpha"))); + auto* ctrl = TexturePaintController::instance(); + auto* em = EditModeController::instance(); + ASSERT_NE(em, nullptr); + + // Swatches carry no alpha by design, so applying one must KEEP the brush's + // existing alpha rather than forcing it opaque via QColor(r,g,b). + em->setVertexPaintColor(QColor(10, 20, 30, 128)); + ASSERT_TRUE(ctrl->applyPaletteColor(QStringLiteral("#4caf50"))); + EXPECT_EQ(em->vertexPaintColor().alpha(), 128) + << "picking a swatch must not silently make the brush opaque"; + EXPECT_EQ(em->vertexPaintColor().red(), 0x4c); + + em->setVertexPaintBackgroundColor(QColor(1, 2, 3, 64)); + ASSERT_TRUE(ctrl->applyPaletteColor(QStringLiteral("#ff0000"), true)); + EXPECT_EQ(em->vertexPaintBackgroundColor().alpha(), 64); + + ctrl->closeSession(); +} + +TEST_F(TexturePaintControllerSceneTest, RecentRingSeesColorsSetOutsideThePalette) { + ASSERT_TRUE(m_fix.setup(QStringLiteral("RecentShared"))); + auto* ctrl = TexturePaintController::instance(); + auto* em = EditModeController::instance(); + ASSERT_NE(em, nullptr); + + // The toolbar picker and pickBrushColorInteractive() call + // setVertexPaintColor directly. Without hooking the shared change signal, + // Recent would only ever show colours re-picked from palette tiles. + em->setVertexPaintColor(QColor(0x11, 0x22, 0x33)); + const QStringList recent = ctrl->recentPaintColors(); + ASSERT_FALSE(recent.isEmpty()); + EXPECT_EQ(recent.front(), QStringLiteral("#112233")); + + ctrl->closeSession(); +} + +TEST_F(TexturePaintControllerSceneTest, PresetRoundTripsFgBgRampMode) { + QStandardPaths::setTestModeEnabled(true); + ASSERT_TRUE(m_fix.setup(QStringLiteral("PresetFgBg"))); + auto* ctrl = TexturePaintController::instance(); + + // Save a gradient brush in FG/BG mode. + ctrl->setColorSource(1); // Gradient + ctrl->setUseFgBgRamp(true); + ASSERT_TRUE(ctrl->saveBrushPreset(QStringLiteral("FgBg Preset"))); + + // Move to a NAMED ramp, then re-apply: FG/BG must come back, not the name. + ctrl->setUseFgBgRamp(false); + ctrl->setActiveRampName(QStringLiteral("Sunset")); + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("FgBg Preset"))); + EXPECT_TRUE(ctrl->useFgBgRamp()) + << "an FG/BG gradient preset must not restore a named ramp instead"; + + EXPECT_TRUE(ctrl->deleteBrushPreset(QStringLiteral("FgBg Preset"))); + ctrl->closeSession(); + QStandardPaths::setTestModeEnabled(false); +} + +TEST_F(TexturePaintControllerSceneTest, CustomPresetShadowingBundledNameIsDeletable) { + QStandardPaths::setTestModeEnabled(true); + ASSERT_TRUE(m_fix.setup(QStringLiteral("PresetShadow"))); + auto* ctrl = TexturePaintController::instance(); + + // A bundled name with no custom file: nothing to delete. + EXPECT_FALSE(ctrl->canDeleteBrushPreset(QStringLiteral("Soft Round"))); + EXPECT_FALSE(ctrl->deleteBrushPreset(QStringLiteral("Soft Round"))); + + // Save a custom preset that SHADOWS the bundled name. Because custom wins + // in allPresets(), refusing to delete it would permanently hide the bundled + // version with no way back. + ctrl->setBrushRadius(0.191); + ASSERT_TRUE(ctrl->saveBrushPreset(QStringLiteral("Soft Round"))); + EXPECT_TRUE(ctrl->canDeleteBrushPreset(QStringLiteral("Soft Round"))); + EXPECT_TRUE(ctrl->deleteBrushPreset(QStringLiteral("Soft Round"))); + + // The bundled version is back. + ASSERT_TRUE(ctrl->applyBrushPreset(QStringLiteral("Soft Round"))); + EXPECT_NEAR(ctrl->texturePaintRadius(), 0.06, 1e-6) + << "deleting the override must restore the bundled preset"; + + ctrl->closeSession(); + QStandardPaths::setTestModeEnabled(false); +} diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 9d8eec70..2bf92baa 100755 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -6712,9 +6712,12 @@ void MainWindow::buildBrushPresetSection(QWidget* paintSettings, QVBoxLayout* pa // Bundled presets are compiled in, so Delete could only remove a // user override — disable it rather than appear to delete something // that returns on restart. - delPresetBtn->setEnabled( - !presetCombo->currentText().isEmpty() - && !tpcPaint->isBundledBrushPreset(presetCombo->currentText())); + // Enabled whenever a CUSTOM file exists for this name — including a + // custom preset that shadows a bundled name, which must be + // removable to restore the bundled version. The controller makes + // the same distinction. + delPresetBtn->setEnabled(tpcPaint->canDeleteBrushPreset( + presetCombo->currentText())); }; refreshPresets(); @@ -6725,7 +6728,7 @@ void MainWindow::buildBrushPresetSection(QWidget* paintSettings, QVBoxLayout* pa SentryReporter::addBreadcrumb("ui.action", QStringLiteral("paint preset apply %1").arg(name)); tpcPaint->applyBrushPreset(name); - delPresetBtn->setEnabled(!tpcPaint->isBundledBrushPreset(name)); + delPresetBtn->setEnabled(tpcPaint->canDeleteBrushPreset(name)); }); connect(savePresetBtn, &QPushButton::clicked, this, From 4804f901e37e8eb7ee734f8afcdad68040cf206b Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 22:23:13 -0400 Subject: [PATCH 7/7] docs(#551): correct applyPaletteColor's contract (PR #970 follow-up review) CodeRabbit's re-review caught that the header doc went stale with the review fixes: it still promised the picked colour is "pushed onto the recent ring" unconditionally, which was true when the method pushed directly but is not now that only FOREGROUND picks reach the ring (via the shared vertexPaintChanged hook). A caller reading that doc would expect a background pick to show up in Recent. The doc now states both behaviours that the review fixes established: the brush's existing alpha is preserved, and only a foreground pick enters the ring. Also extended the design doc, which described the alpha-free swatch INTENT but not the invariant it depends on. It now records that the first implementation violated it (QColor(r,g,b) is opaque, so every pick silently discarded a translucent brush) and points at the mutation-checked test, since that bug is invisible unless you happen to be painting with a translucent brush. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- docs/PAINT_V2_SLICE_H_DESIGN.md | 14 ++++++++++++++ src/TexturePaintController.h | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/PAINT_V2_SLICE_H_DESIGN.md b/docs/PAINT_V2_SLICE_H_DESIGN.md index 64eb9f7d..de3dc43b 100644 --- a/docs/PAINT_V2_SLICE_H_DESIGN.md +++ b/docs/PAINT_V2_SLICE_H_DESIGN.md @@ -66,6 +66,20 @@ Blues, Earth Tones. Custom palettes live in `/paint/palettes/*.json`. alpha is a separate brush property. Baking alpha into swatches would override the user's setting on every pick. +That design only holds if `applyPaletteColor` **copies the brush's existing +alpha** onto the picked colour. The first implementation did not — it built +`QColor(r, g, b)`, which is opaque, so every swatch pick silently discarded a +partially transparent brush and did the exact opposite of the intent above. +There is a test pinning it (`PaletteColorPreservesBrushAlpha`), and it is +mutation-checked, because the bug is invisible unless you happen to be painting +with a translucent brush. + +Only a **foreground** pick reaches the recent ring. The ring is fed from +`EditModeController::vertexPaintChanged` rather than from `applyPaletteColor`, +so colours chosen through the toolbar swatch or `pickBrushColorInteractive()` +appear too — hooking only the palette path left Recent showing solely colours +*re-picked from tiles*, never a newly chosen one. + ## Files | File | Role | diff --git a/src/TexturePaintController.h b/src/TexturePaintController.h index 6cb840ec..92477c1f 100644 --- a/src/TexturePaintController.h +++ b/src/TexturePaintController.h @@ -621,8 +621,12 @@ class TexturePaintController : public QObject Q_INVOKABLE QStringList colorPaletteSwatches(const QString& paletteName) const; /// Most-recently-used colours, newest first (max 12). Q_INVOKABLE QStringList recentPaintColors() const; - /// Set the foreground (or background) paint colour from "#rrggbb" and push - /// it onto the recent ring. + /// Set the foreground (or `asBackground`) paint colour from "#rrggbb". + /// The brush's existing ALPHA is preserved — swatches carry no alpha, so a + /// pick must not silently make the brush opaque. + /// Only a FOREGROUND pick reaches the recent ring (via the shared + /// vertexPaintChanged hook); the background is a rarely-changed secondary + /// slot that would churn the history. Q_INVOKABLE bool applyPaletteColor(const QString& hex, bool asBackground = false); /// Build a palette from the ACTIVE paint buffer and save it. Returns false /// when there is no session or the extraction found no opaque pixels.