mirror of
https://github.com/crosspoint-reader/crosspoint-reader.git
synced 2026-04-29 10:26:52 -07:00
perf: Improve large CSS files handling (#779)
## Summary Closes #766. Thank you for the help @bramschulting! **What is the goal of this PR?** - First and foremost, fix issue #766. - Through working on that, I realized the current CSS parsing/loading code can be improved dramatically for large files and still had additional performance improvements to be made, even with EPUBs with small CSS. **What changes are included?** - Stream CSS parsing and reuse normalization buffers to cut allocations - Add rule limits and selector validation to release rules and free up memory when needed - Skip CSS parsing/loading entirely when "Book's Embedded Style" is off ## Additional Context - My test EPUB has been updated [here](https://github.com/jdk2pq/css-test-epub) to include a very large CSS file to test this out --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**YES**_, Codex
This commit is contained in:
+8
-25
@@ -208,30 +208,14 @@ bool Epub::parseTocNavFile() const {
|
||||
return true;
|
||||
}
|
||||
|
||||
std::string Epub::getCssRulesCache() const { return cachePath + "/css_rules.cache"; }
|
||||
|
||||
bool Epub::loadCssRulesFromCache() const {
|
||||
FsFile cssCacheFile;
|
||||
if (Storage.openFileForRead("EBP", getCssRulesCache(), cssCacheFile)) {
|
||||
if (cssParser->loadFromCache(cssCacheFile)) {
|
||||
cssCacheFile.close();
|
||||
LOG_DBG("EBP", "Loaded CSS rules from cache");
|
||||
return true;
|
||||
}
|
||||
cssCacheFile.close();
|
||||
LOG_DBG("EBP", "CSS cache invalid, reparsing");
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
void Epub::parseCssFiles() const {
|
||||
if (cssFiles.empty()) {
|
||||
LOG_DBG("EBP", "No CSS files to parse, but CssParser created for inline styles");
|
||||
}
|
||||
|
||||
// Try to load from CSS cache first
|
||||
if (!loadCssRulesFromCache()) {
|
||||
// Cache miss - parse CSS files
|
||||
// See if we have a cached version of the CSS rules
|
||||
if (!cssParser->hasCache()) {
|
||||
// No cache yet - parse CSS files
|
||||
for (const auto& cssPath : cssFiles) {
|
||||
LOG_DBG("EBP", "Parsing CSS file: %s", cssPath.c_str());
|
||||
|
||||
@@ -262,11 +246,10 @@ void Epub::parseCssFiles() const {
|
||||
}
|
||||
|
||||
// Save to cache for next time
|
||||
FsFile cssCacheFile;
|
||||
if (Storage.openFileForWrite("EBP", getCssRulesCache(), cssCacheFile)) {
|
||||
cssParser->saveToCache(cssCacheFile);
|
||||
cssCacheFile.close();
|
||||
if (!cssParser->saveToCache()) {
|
||||
LOG_ERR("EBP", "Failed to save CSS rules to cache");
|
||||
}
|
||||
cssParser->clear();
|
||||
|
||||
LOG_DBG("EBP", "Loaded %zu CSS style rules from %zu files", cssParser->ruleCount(), cssFiles.size());
|
||||
}
|
||||
@@ -279,11 +262,11 @@ bool Epub::load(const bool buildIfMissing, const bool skipLoadingCss) {
|
||||
// Initialize spine/TOC cache
|
||||
bookMetadataCache.reset(new BookMetadataCache(cachePath));
|
||||
// Always create CssParser - needed for inline style parsing even without CSS files
|
||||
cssParser.reset(new CssParser());
|
||||
cssParser.reset(new CssParser(cachePath));
|
||||
|
||||
// Try to load existing cache first
|
||||
if (bookMetadataCache->load()) {
|
||||
if (!skipLoadingCss && !loadCssRulesFromCache()) {
|
||||
if (!skipLoadingCss && !cssParser->hasCache()) {
|
||||
LOG_DBG("EBP", "Warning: CSS rules cache not found, attempting to parse CSS files");
|
||||
// to get CSS file list
|
||||
if (!parseContentOpf(bookMetadataCache->coreMetadata)) {
|
||||
|
||||
+1
-3
@@ -35,8 +35,6 @@ class Epub {
|
||||
bool parseTocNcxFile() const;
|
||||
bool parseTocNavFile() const;
|
||||
void parseCssFiles() const;
|
||||
std::string getCssRulesCache() const;
|
||||
bool loadCssRulesFromCache() const;
|
||||
|
||||
public:
|
||||
explicit Epub(std::string filepath, const std::string& cacheDir) : filepath(std::move(filepath)) {
|
||||
@@ -73,5 +71,5 @@ class Epub {
|
||||
|
||||
size_t getBookSize() const;
|
||||
float calculateProgress(int currentSpineIndex, float currentSpineRead) const;
|
||||
const CssParser* getCssParser() const { return cssParser.get(); }
|
||||
CssParser* getCssParser() const { return cssParser.get(); }
|
||||
};
|
||||
|
||||
@@ -181,11 +181,20 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c
|
||||
viewportHeight, hyphenationEnabled, embeddedStyle);
|
||||
std::vector<uint32_t> lut = {};
|
||||
|
||||
CssParser* cssParser = nullptr;
|
||||
if (embeddedStyle) {
|
||||
cssParser = epub->getCssParser();
|
||||
if (cssParser) {
|
||||
if (!cssParser->loadFromCache()) {
|
||||
LOG_ERR("SCT", "Failed to load CSS from cache");
|
||||
}
|
||||
}
|
||||
}
|
||||
ChapterHtmlSlimParser visitor(
|
||||
tmpHtmlPath, renderer, fontId, lineCompression, extraParagraphSpacing, paragraphAlignment, viewportWidth,
|
||||
viewportHeight, hyphenationEnabled,
|
||||
[this, &lut](std::unique_ptr<Page> page) { lut.emplace_back(this->onPageComplete(std::move(page))); },
|
||||
embeddedStyle, popupFn, embeddedStyle ? epub->getCssParser() : nullptr);
|
||||
embeddedStyle, popupFn, cssParser);
|
||||
Hyphenator::setPreferredLanguage(epub->getLanguage());
|
||||
success = visitor.parseAndBuildPages();
|
||||
|
||||
@@ -194,6 +203,9 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c
|
||||
LOG_ERR("SCT", "Failed to parse XML and build pages");
|
||||
file.close();
|
||||
Storage.remove(filePath.c_str());
|
||||
if (cssParser) {
|
||||
cssParser->clear();
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -220,6 +232,9 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c
|
||||
serialization::writePod(file, pageCount);
|
||||
serialization::writePod(file, lutOffset);
|
||||
file.close();
|
||||
if (cssParser) {
|
||||
cssParser->clear();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
+326
-259
File diff suppressed because it is too large
Load Diff
@@ -4,6 +4,7 @@
|
||||
|
||||
#include <string>
|
||||
#include <unordered_map>
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
|
||||
#include "CssStyle.h"
|
||||
@@ -29,7 +30,7 @@
|
||||
*/
|
||||
class CssParser {
|
||||
public:
|
||||
CssParser() = default;
|
||||
explicit CssParser(std::string cachePath) : cachePath(std::move(cachePath)) {}
|
||||
~CssParser() = default;
|
||||
|
||||
// Non-copyable
|
||||
@@ -76,28 +77,35 @@ class CssParser {
|
||||
*/
|
||||
void clear() { rulesBySelector_.clear(); }
|
||||
|
||||
/**
|
||||
* Check if CSS rules cache file exists
|
||||
*/
|
||||
bool hasCache() const;
|
||||
|
||||
/**
|
||||
* Save parsed CSS rules to a cache file.
|
||||
* @param file Open file handle to write to
|
||||
* @return true if cache was written successfully
|
||||
*/
|
||||
bool saveToCache(FsFile& file) const;
|
||||
bool saveToCache() const;
|
||||
|
||||
/**
|
||||
* Load CSS rules from a cache file.
|
||||
* Clears any existing rules before loading.
|
||||
* @param file Open file handle to read from
|
||||
* @return true if cache was loaded successfully
|
||||
*/
|
||||
bool loadFromCache(FsFile& file);
|
||||
bool loadFromCache();
|
||||
|
||||
private:
|
||||
// Storage: maps normalized selector -> style properties
|
||||
std::unordered_map<std::string, CssStyle> rulesBySelector_;
|
||||
|
||||
std::string cachePath;
|
||||
|
||||
// Internal parsing helpers
|
||||
void processRuleBlock(const std::string& selectorGroup, const std::string& declarations);
|
||||
void processRuleBlockWithStyle(const std::string& selectorGroup, const CssStyle& style);
|
||||
static CssStyle parseDeclarations(const std::string& declBlock);
|
||||
static void parseDeclarationIntoStyle(const std::string& decl, CssStyle& style, std::string& propNameBuf,
|
||||
std::string& propValueBuf);
|
||||
|
||||
// Individual property value parsers
|
||||
static CssTextAlign interpretAlignment(const std::string& val);
|
||||
@@ -105,10 +113,10 @@ class CssParser {
|
||||
static CssFontWeight interpretFontWeight(const std::string& val);
|
||||
static CssTextDecoration interpretDecoration(const std::string& val);
|
||||
static CssLength interpretLength(const std::string& val);
|
||||
static int8_t interpretSpacing(const std::string& val);
|
||||
|
||||
// String utilities
|
||||
static std::string normalized(const std::string& s);
|
||||
static void normalizedInto(const std::string& s, std::string& out);
|
||||
static std::vector<std::string> splitOnChar(const std::string& s, char delimiter);
|
||||
static std::vector<std::string> splitWhitespace(const std::string& s);
|
||||
};
|
||||
|
||||
+4
-1
@@ -27,10 +27,13 @@ build_flags =
|
||||
# https://libexpat.github.io/doc/api/latest/#XML_GE
|
||||
-DXML_GE=0
|
||||
-DXML_CONTEXT_BYTES=1024
|
||||
-std=c++2a
|
||||
-std=gnu++2a
|
||||
# Enable UTF-8 long file names in SdFat
|
||||
-DUSE_UTF8_LONG_NAMES=1
|
||||
|
||||
build_unflags =
|
||||
-std=gnu++11
|
||||
|
||||
; Board configuration
|
||||
board_build.flash_mode = dio
|
||||
board_build.flash_size = 16MB
|
||||
|
||||
@@ -88,7 +88,7 @@ RecentBook RecentBooksStore::getDataFromBook(std::string path) const {
|
||||
// If epub, try to load the metadata for title/author and cover
|
||||
if (StringUtils::checkFileExtension(lastBookFileName, ".epub")) {
|
||||
Epub epub(path, "/.crosspoint");
|
||||
epub.load(false);
|
||||
epub.load(false, true);
|
||||
return RecentBook{path, epub.getTitle(), epub.getAuthor(), epub.getThumbBmpPath()};
|
||||
} else if (StringUtils::checkFileExtension(lastBookFileName, ".xtch") ||
|
||||
StringUtils::checkFileExtension(lastBookFileName, ".xtc")) {
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
#include <HalStorage.h>
|
||||
|
||||
#include "CrossPointSettings.h"
|
||||
#include "Epub.h"
|
||||
#include "EpubReaderActivity.h"
|
||||
#include "Txt.h"
|
||||
@@ -35,7 +36,7 @@ std::unique_ptr<Epub> ReaderActivity::loadEpub(const std::string& path) {
|
||||
}
|
||||
|
||||
auto epub = std::unique_ptr<Epub>(new Epub(path, "/.crosspoint"));
|
||||
if (epub->load()) {
|
||||
if (epub->load(true, SETTINGS.embeddedStyle == 0)) {
|
||||
return epub;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user