perf(skins): skip redundant skin parsing, base reloads and theme writes
Skin handling did more work than it needed: - the bundled-skin scan fully parsed each of the 9 bundled skin files to read name, author, dark and images from their first 327-345 bytes. The build copies the rest of ui.toml into every skin, so each file is 51-54 KB; - setActiveSkin() reloaded ui.toml right after main() had loaded it, a second full parse with nothing merged in between; - in the Windows single-file exe, every getBundledSkinsDirectory() call (skin refresh, the dragonx entry, each image-dir lookup) re-extracted all embedded themes with a blind overwrite: more than once per launch, and again on skin changes. Now: - bundled skins are parsed only up to the first table header after [theme] other than [theme.images]. Anything unusual (no [theme], images defined later, a parse error in the slice) falls back to the full parse, and user skins are still parsed in full; - UISchema tracks whether its state is exactly the base (the last change was a base load and no hot-reload is pending), and setActiveSkin() skips reloadBase() in that case; - the bundled-skins directory is resolved once per process (re-checked with exists()), and theme extraction rewrites a file only when its bytes differ, through the new resources::writeFileIfDifferent(). It compares bytes, not sizes, so a same-length palette edit still updates. Measured on the Lite build (median of 5 runs): - bundled-skin scan: 15 -> 1 ms; - from "Initializing ObsidianDragon" to the activated skin: 29 -> 12 ms; - ui.toml is loaded once per launch instead of twice, and the light skin merges its overlay exactly once. A Python mirror of the header-slice parse agrees with a full parse on name, author, dark and images for all 18 source and build-expanded skin files, and full screenshot sweeps over all 10 skins match the previous build within noise. New unit tests cover writeFileIfDifferent(): missing file, identical bytes (mtime untouched), same-length change, and a longer file truncated. Behaviour changes: - a bundled skin with a TOML error after its header is now listed instead of skipped; the error surfaces only when it is selected; - deleting a single file inside the extracted bundled-themes dir mid-session is repaired only at the next launch; - the "Extracted N embedded bundled themes" count includes files that were already current. Not verifiable here: Linux has no embedded themes, so the new writeFileIfDifferent() ran only in the unit tests and the extraction path was only compiled, in the Windows cross-build. That a second Windows launch leaves the theme files' mtimes untouched is still to be checked on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -122,6 +122,27 @@ const EmbeddedTheme* getEmbeddedThemes()
|
||||
return themes.data();
|
||||
}
|
||||
|
||||
int writeFileIfDifferent(const std::string& path, const uint8_t* data, size_t size)
|
||||
{
|
||||
{
|
||||
std::ifstream cur(path, std::ios::binary | std::ios::ate);
|
||||
if (cur.is_open()) {
|
||||
const std::streamoff len = cur.tellg();
|
||||
if (len >= 0 && static_cast<size_t>(len) == size) {
|
||||
std::string buf(size, '\0');
|
||||
cur.seekg(0);
|
||||
if (size == 0 || (cur.read(&buf[0], static_cast<std::streamsize>(size)) &&
|
||||
std::memcmp(buf.data(), data, size) == 0))
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
} // reader closed before the writer opens (Windows sharing)
|
||||
std::ofstream f(path, std::ios::binary | std::ios::trunc);
|
||||
if (!f.is_open()) return -1;
|
||||
f.write(reinterpret_cast<const char*>(data), static_cast<std::streamsize>(size));
|
||||
return f.good() ? 1 : -1;
|
||||
}
|
||||
|
||||
int extractBundledThemes(const std::string& destDir)
|
||||
{
|
||||
namespace fs = std::filesystem;
|
||||
@@ -140,14 +161,12 @@ int extractBundledThemes(const std::string& destDir)
|
||||
|
||||
for (const auto* t = themes; t->data != nullptr; ++t) {
|
||||
fs::path dest = fs::path(destDir) / t->filename;
|
||||
// Always overwrite — bundled themes should match the binary version
|
||||
std::ofstream f(dest, std::ios::binary);
|
||||
if (f.is_open()) {
|
||||
f.write(reinterpret_cast<const char*>(t->data), t->size);
|
||||
f.close();
|
||||
DEBUG_LOGF("[INFO] EmbeddedResources: Extracted theme: %s (%zu bytes)\n",
|
||||
t->filename, t->size);
|
||||
count++;
|
||||
// Bundled themes must match the binary version: rewrite only when the bytes differ (not size-only:
|
||||
// a same-length palette edit must still update).
|
||||
const int w = writeFileIfDifferent(dest.string(), t->data, t->size);
|
||||
if (w >= 0) {
|
||||
if (w == 1) DEBUG_LOGF("[INFO] EmbeddedResources: Extracted theme: %s (%zu bytes)\n", t->filename, t->size);
|
||||
count++; // present and current (caller only checks > 0)
|
||||
} else {
|
||||
DEBUG_LOGF("[ERROR] EmbeddedResources: Failed to write theme: %s\n", dest.string().c_str());
|
||||
}
|
||||
|
||||
@@ -131,6 +131,10 @@ const EmbeddedTheme* getEmbeddedThemes();
|
||||
// Returns number of themes extracted
|
||||
int extractBundledThemes(const std::string& destDir);
|
||||
|
||||
// Write `size` bytes to `path` unless the file already holds exactly those bytes.
|
||||
// Returns 1 = written, 0 = already identical (untouched), -1 = write failed.
|
||||
int writeFileIfDifferent(const std::string& path, const uint8_t* data, size_t size);
|
||||
|
||||
// Update stale bundled theme files in the given directory.
|
||||
// Compares on-disk file size to embedded size; overwrites on mismatch.
|
||||
// Returns number of themes updated.
|
||||
|
||||
@@ -16,6 +16,8 @@
|
||||
#include <filesystem>
|
||||
#include <algorithm>
|
||||
#include <cstdio>
|
||||
#include <iterator>
|
||||
#include <string_view>
|
||||
#include "../../util/logger.h"
|
||||
|
||||
namespace fs = std::filesystem;
|
||||
@@ -38,6 +40,14 @@ SkinManager& SkinManager::instance() {
|
||||
// ============================================================================
|
||||
|
||||
std::string SkinManager::getBundledSkinsDirectory() {
|
||||
static std::string s_cached; // resolved once; updateBundledThemes()/extractBundledThemes() run once, not per call
|
||||
std::error_code cacheEc;
|
||||
if (!s_cached.empty() && fs::exists(s_cached, cacheEc)) return s_cached;
|
||||
s_cached = resolveBundledSkinsDirectory();
|
||||
return s_cached;
|
||||
}
|
||||
|
||||
std::string SkinManager::resolveBundledSkinsDirectory() {
|
||||
// Bundled skins live in res/themes/ next to the executable
|
||||
fs::path exe_dir = util::getExecutableDirectory();
|
||||
fs::path themes_dir = exe_dir / "res" / "themes";
|
||||
@@ -83,6 +93,35 @@ std::string SkinManager::getUserSkinsDirectory() {
|
||||
// Scan for skins
|
||||
// ============================================================================
|
||||
|
||||
namespace {
|
||||
// Bundled skins keep name/author/dark/images in the first lines of [theme]: parse only that slice (up to the first
|
||||
// other table header) instead of the whole build-expanded file, falling back to the full parse whenever the slice
|
||||
// is not self-sufficient.
|
||||
toml::table parseThemeMetadata(const std::string& path) {
|
||||
std::ifstream in(path, std::ios::binary);
|
||||
std::string text((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
|
||||
if (!text.empty()) {
|
||||
size_t pos = text.find("[theme]"); // table header at start of a line
|
||||
while (pos != std::string::npos && pos != 0 && text[pos - 1] != '\n') pos = text.find("[theme]", pos + 1);
|
||||
if (pos != std::string::npos) {
|
||||
size_t cut = text.size();
|
||||
for (size_t ls = text.find('\n', pos); ls != std::string::npos; ls = text.find('\n', ls + 1)) {
|
||||
if (ls + 1 < text.size() && text[ls + 1] == '[' &&
|
||||
text.compare(ls + 1, 14, "[theme.images]") != 0) { cut = ls + 1; break; }
|
||||
}
|
||||
try {
|
||||
toml::table slice = toml::parse(std::string_view(text).substr(0, cut), path);
|
||||
auto* theme = slice["theme"].as_table();
|
||||
// [theme.images] later in the file would be missing from the slice: then use the full parse
|
||||
if (theme && (theme->contains("images") || text.find("[theme.images]") == std::string::npos))
|
||||
return slice;
|
||||
} catch (...) {} // fall through to the full parse
|
||||
}
|
||||
}
|
||||
return toml::parse_file(path); // original behaviour (throws like before)
|
||||
}
|
||||
} // namespace
|
||||
|
||||
void SkinManager::scanDirectory(const std::string& dir, bool bundled) {
|
||||
if (!fs::exists(dir) || !fs::is_directory(dir)) {
|
||||
DEBUG_LOGF("[SkinManager] Directory does not exist: %s\n", dir.c_str());
|
||||
@@ -105,10 +144,12 @@ void SkinManager::scanDirectory(const std::string& dir, bool bundled) {
|
||||
// Skip ui.toml - it's the base theme handled separately as "dragonx"
|
||||
if (stem == "ui") continue;
|
||||
|
||||
// Try to parse and extract metadata
|
||||
// Try to parse and extract metadata. Bundled skins keep name/author/dark/images in the first lines of
|
||||
// [theme]; parse only that slice (before the first other table header), falling back to the full
|
||||
// file whenever the slice is not self-sufficient.
|
||||
toml::table root;
|
||||
try {
|
||||
root = toml::parse_file(p.string());
|
||||
root = parseThemeMetadata(p.string());
|
||||
} catch (...) {
|
||||
DEBUG_LOGF("[SkinManager] Skipping '%s': invalid TOML\n", p.filename().string().c_str());
|
||||
continue;
|
||||
@@ -598,13 +639,13 @@ bool SkinManager::setActiveSkin(const std::string& id) {
|
||||
|
||||
if (!basePath.empty() || schema.hasEmbeddedBase()) {
|
||||
if (!basePath.empty() && basePath == skin->path) {
|
||||
// Switching back to the base theme: full reload, clear overlay
|
||||
schema.reloadBase();
|
||||
// Switching back to the base theme: full reload, clear overlay (nothing to reload when still pristine)
|
||||
if (!schema.isPristineBase()) schema.reloadBase();
|
||||
schema.reapplyColorsToImGui();
|
||||
loaded = true;
|
||||
} else {
|
||||
// Switching to a non-base skin: reload base then merge overlay
|
||||
if (schema.reloadBase()) {
|
||||
if (schema.isPristineBase() || schema.reloadBase()) {
|
||||
if (schema.mergeOverlayFromFile(skin->path)) {
|
||||
schema.reapplyColorsToImGui();
|
||||
loaded = true;
|
||||
@@ -623,7 +664,7 @@ bool SkinManager::setActiveSkin(const std::string& id) {
|
||||
// just reload the base to restore the default theme
|
||||
auto& schema = UISchema::instance();
|
||||
if (schema.hasEmbeddedBase() || !schema.basePath().empty()) {
|
||||
schema.reloadBase();
|
||||
if (!schema.isPristineBase()) schema.reloadBase();
|
||||
schema.reapplyColorsToImGui();
|
||||
loaded = true;
|
||||
}
|
||||
|
||||
@@ -170,6 +170,9 @@ private:
|
||||
|
||||
void scanDirectory(const std::string& dir, bool bundled);
|
||||
|
||||
/// Uncached body of getBundledSkinsDirectory() (which memoises it per process)
|
||||
static std::string resolveBundledSkinsDirectory();
|
||||
|
||||
/// Resolve the image directory for a given skin
|
||||
std::filesystem::path resolveImgDir(const SkinInfo* skin) const;
|
||||
|
||||
|
||||
@@ -99,6 +99,7 @@ bool UISchema::loadFromFile(const std::string& path) {
|
||||
overlayPath_.clear(); // No overlay yet
|
||||
loaded_ = true;
|
||||
dirty_ = false;
|
||||
pristineBase_ = (path == basePath_); // base reload == exact current state
|
||||
++generation_;
|
||||
|
||||
// Record initial modification time
|
||||
@@ -184,6 +185,7 @@ bool UISchema::loadFromString(const std::string& tomlStr, const std::string& lab
|
||||
currentPath_ = label; // Track what is loaded (for logging)
|
||||
loaded_ = true;
|
||||
dirty_ = false;
|
||||
pristineBase_ = true;
|
||||
++generation_;
|
||||
|
||||
DEBUG_LOGF("[UISchema] Loaded from %s (theme: %s, %s, %zu elements)\n",
|
||||
@@ -260,6 +262,7 @@ bool UISchema::mergeOverlayFromFile(const std::string& path) {
|
||||
}
|
||||
|
||||
overlayPath_ = path;
|
||||
pristineBase_ = false;
|
||||
// Track overlay file for hot-reload
|
||||
currentPath_ = path;
|
||||
++generation_;
|
||||
|
||||
@@ -108,6 +108,10 @@ public:
|
||||
*/
|
||||
bool reloadBase();
|
||||
|
||||
/// True while the loaded state is exactly the base file/string (no overlay merged since the last base load,
|
||||
/// no pending hot-reload).
|
||||
bool isPristineBase() const { return pristineBase_ && !dirty_; }
|
||||
|
||||
/**
|
||||
* @brief Get the active overlay file path (empty if no overlay)
|
||||
*/
|
||||
@@ -337,6 +341,7 @@ private:
|
||||
std::filesystem::file_time_type lastModTime_;
|
||||
std::filesystem::file_time_type baseModTime_; ///< Track base file changes when overlay active
|
||||
bool dirty_ = false;
|
||||
bool pristineBase_ = false; ///< last mutation was a base load (loadFromFile(basePath_) / loadFromString)
|
||||
bool fonts_changed_ = false;
|
||||
|
||||
// Theme
|
||||
|
||||
@@ -15332,6 +15332,44 @@ void testLoadLanguageFromLooseFiles()
|
||||
EXPECT_TRUE(i18n.loadLanguage("en")); // restore the built-in English table
|
||||
EXPECT_EQ(std::string(i18n.translate("overview")), std::string("Overview"));
|
||||
}
|
||||
|
||||
// resources::writeFileIfDifferent (bundled-theme extraction): rewrite a file only when its bytes differ.
|
||||
void testWriteFileIfDifferent()
|
||||
{
|
||||
namespace res = dragonx::resources;
|
||||
const fs::path dir = testTmpPath("obsidian-write-if-different");
|
||||
removeTree(dir);
|
||||
fs::create_directories(dir);
|
||||
const fs::path p = dir / "theme.toml";
|
||||
const std::string v1 = "[theme]\nname = \"A\"\n";
|
||||
auto bytes = [](const std::string& str) { return reinterpret_cast<const uint8_t*>(str.data()); };
|
||||
|
||||
// (a) missing file -> written with the right content
|
||||
EXPECT_EQ(res::writeFileIfDifferent(p.string(), bytes(v1), v1.size()), 1);
|
||||
EXPECT_EQ(readProvisionFile(p), v1);
|
||||
|
||||
// (b) identical bytes -> untouched (mtime pinned to a known older value first, so any rewrite would show)
|
||||
const auto pinned = fs::last_write_time(p) - std::chrono::hours(1);
|
||||
fs::last_write_time(p, pinned);
|
||||
EXPECT_EQ(res::writeFileIfDifferent(p.string(), bytes(v1), v1.size()), 0);
|
||||
EXPECT_TRUE(fs::last_write_time(p) == pinned);
|
||||
EXPECT_EQ(readProvisionFile(p), v1);
|
||||
|
||||
// (c) same length, one byte flipped -> rewritten (not a size-only check)
|
||||
std::string v2 = v1;
|
||||
v2[v2.size() - 3] = 'B';
|
||||
EXPECT_EQ(v2.size(), v1.size());
|
||||
EXPECT_EQ(res::writeFileIfDifferent(p.string(), bytes(v2), v2.size()), 1);
|
||||
EXPECT_EQ(readProvisionFile(p), v2);
|
||||
|
||||
// (d) longer file on disk -> rewritten and truncated to the new size
|
||||
writeTestFile(p, v2 + "# trailing bytes that must disappear\n");
|
||||
EXPECT_EQ(res::writeFileIfDifferent(p.string(), bytes(v1), v1.size()), 1);
|
||||
EXPECT_EQ(readProvisionFile(p), v1);
|
||||
EXPECT_EQ(static_cast<size_t>(fs::file_size(p)), v1.size());
|
||||
|
||||
removeTree(dir);
|
||||
}
|
||||
} // namespace
|
||||
|
||||
int main()
|
||||
@@ -15582,6 +15620,7 @@ int main()
|
||||
testTextFit();
|
||||
testTextFitBanner();
|
||||
testLoadLanguageFromLooseFiles();
|
||||
testWriteFileIfDifferent();
|
||||
|
||||
if (g_failures != 0) {
|
||||
std::cerr << g_failures << " test assertion(s) failed\n";
|
||||
|
||||
Reference in New Issue
Block a user