fix: Fixed Image Sizing When No Width is Set (#1002)

## Summary

* **What is the goal of this PR?** (e.g., Implements the new feature for
file uploading.)
When no width is set for an image, the image currently automatically
sets to the width of the page. However, with this fix, the parser will
use the height and aspect ratio of the image to properly set a height
for it. See below example:


Before:

![IMG_8862](https://github.com/user-attachments/assets/64b3b92f-1165-45ca-8bdb-8e69613d9725)

After:

![IMG_8863](https://github.com/user-attachments/assets/5cb99b12-d150-4b37-ae4c-c8a20eb9f3a0)


* **What changes are included?✱
Changes to the CSS parser

## Additional Context

* Add any other information that might be helpful for the reviewer
(e.g., performance implications, potential risks,
  specific areas to focus on).

---

### 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, Cursor
This commit is contained in:
DestinySpeaker
2026-02-19 21:34:28 -08:00
committed by GitHub
parent 388fbf206a
commit 5da23eed82
6 changed files with 172 additions and 33 deletions

View File

@@ -339,14 +339,22 @@ bool Epub::load(const bool buildIfMissing, const bool skipLoadingCss) {
// Try to load existing cache first // Try to load existing cache first
if (bookMetadataCache->load()) { if (bookMetadataCache->load()) {
if (!skipLoadingCss && !cssParser->hasCache()) { if (!skipLoadingCss) {
LOG_DBG("EBP", "Warning: CSS rules cache not found, attempting to parse CSS files"); // Rebuild CSS cache when missing or when cache version changed (loadFromCache removes stale file)
// to get CSS file list bool needCssRebuild = !cssParser->hasCache();
if (cssParser->hasCache() && !cssParser->loadFromCache()) {
needCssRebuild = true;
}
if (needCssRebuild) {
LOG_DBG("EBP", "CSS rules cache missing or stale, attempting to parse CSS files");
if (!parseContentOpf(bookMetadataCache->coreMetadata)) { if (!parseContentOpf(bookMetadataCache->coreMetadata)) {
LOG_ERR("EBP", "Could not parse content.opf from cached bookMetadata for CSS files"); LOG_ERR("EBP", "Could not parse content.opf from cached bookMetadata for CSS files");
// continue anyway - book will work without CSS and we'll still load any inline style CSS // continue anyway - book will work without CSS and we'll still load any inline style CSS
} }
parseCssFiles(); parseCssFiles();
// Invalidate section caches so they are rebuilt with the new CSS
Storage.removeDir((cachePath + "/sections").c_str());
}
} }
LOG_DBG("EBP", "Loaded ePub: %s", filepath.c_str()); LOG_DBG("EBP", "Loaded ePub: %s", filepath.c_str());
return true; return true;
@@ -447,6 +455,7 @@ bool Epub::load(const bool buildIfMissing, const bool skipLoadingCss) {
if (!skipLoadingCss) { if (!skipLoadingCss) {
// Parse CSS files after cache reload // Parse CSS files after cache reload
parseCssFiles(); parseCssFiles();
Storage.removeDir((cachePath + "/sections").c_str());
} }
LOG_DBG("EBP", "Loaded ePub: %s", filepath.c_str()); LOG_DBG("EBP", "Loaded ePub: %s", filepath.c_str());

View File

@@ -4,6 +4,7 @@
#include <Logging.h> #include <Logging.h>
#include <Serialization.h> #include <Serialization.h>
#include "Epub/css/CssParser.h"
#include "Page.h" #include "Page.h"
#include "hyphenation/Hyphenator.h" #include "hyphenation/Hyphenator.h"
#include "parsers/ChapterHtmlSlimParser.h" #include "parsers/ChapterHtmlSlimParser.h"

View File

@@ -189,10 +189,18 @@ CssTextDecoration CssParser::interpretDecoration(const std::string& val) {
} }
CssLength CssParser::interpretLength(const std::string& val) { CssLength CssParser::interpretLength(const std::string& val) {
const std::string v = normalized(val); CssLength result;
if (v.empty()) return CssLength{}; tryInterpretLength(val, result);
return result;
}
bool CssParser::tryInterpretLength(const std::string& val, CssLength& out) {
const std::string v = normalized(val);
if (v.empty()) {
out = CssLength{};
return false;
}
// Find where the number ends
size_t unitStart = v.size(); size_t unitStart = v.size();
for (size_t i = 0; i < v.size(); ++i) { for (size_t i = 0; i < v.size(); ++i) {
const char c = v[i]; const char c = v[i];
@@ -205,12 +213,13 @@ CssLength CssParser::interpretLength(const std::string& val) {
const std::string numPart = v.substr(0, unitStart); const std::string numPart = v.substr(0, unitStart);
const std::string unitPart = v.substr(unitStart); const std::string unitPart = v.substr(unitStart);
// Parse numeric value
char* endPtr = nullptr; char* endPtr = nullptr;
const float numericValue = std::strtof(numPart.c_str(), &endPtr); const float numericValue = std::strtof(numPart.c_str(), &endPtr);
if (endPtr == numPart.c_str()) return CssLength{}; // No number parsed if (endPtr == numPart.c_str()) {
out = CssLength{};
return false; // No number parsed (e.g. auto, inherit, initial)
}
// Determine unit type (preserve for deferred resolution)
auto unit = CssUnit::Pixels; auto unit = CssUnit::Pixels;
if (unitPart == "em") { if (unitPart == "em") {
unit = CssUnit::Em; unit = CssUnit::Em;
@@ -221,10 +230,11 @@ CssLength CssParser::interpretLength(const std::string& val) {
} else if (unitPart == "%") { } else if (unitPart == "%") {
unit = CssUnit::Percent; unit = CssUnit::Percent;
} }
// px and unitless default to Pixels
return CssLength{numericValue, unit}; out = CssLength{numericValue, unit};
return true;
} }
// Declaration parsing // Declaration parsing
void CssParser::parseDeclarationIntoStyle(const std::string& decl, CssStyle& style, std::string& propNameBuf, void CssParser::parseDeclarationIntoStyle(const std::string& decl, CssStyle& style, std::string& propNameBuf,
@@ -295,6 +305,18 @@ void CssParser::parseDeclarationIntoStyle(const std::string& decl, CssStyle& sty
style.defined.paddingTop = style.defined.paddingRight = style.defined.paddingBottom = style.defined.paddingLeft = style.defined.paddingTop = style.defined.paddingRight = style.defined.paddingBottom = style.defined.paddingLeft =
1; 1;
} }
} else if (propNameBuf == "height") {
CssLength len;
if (tryInterpretLength(propValueBuf, len)) {
style.imageHeight = len;
style.defined.imageHeight = 1;
}
} else if (propNameBuf == "width") {
CssLength len;
if (tryInterpretLength(propValueBuf, len)) {
style.imageWidth = len;
style.defined.imageWidth = 1;
}
} }
} }
@@ -561,8 +583,7 @@ CssStyle CssParser::parseInlineStyle(const std::string& styleValue) { return par
// Cache serialization // Cache serialization
// Cache format version - increment when format changes // Cache file name (version is CssParser::CSS_CACHE_VERSION)
constexpr uint8_t CSS_CACHE_VERSION = 2;
constexpr char rulesCache[] = "/css_rules.cache"; constexpr char rulesCache[] = "/css_rules.cache";
bool CssParser::hasCache() const { return Storage.exists((cachePath + rulesCache).c_str()); } bool CssParser::hasCache() const { return Storage.exists((cachePath + rulesCache).c_str()); }
@@ -578,7 +599,7 @@ bool CssParser::saveToCache() const {
} }
// Write version // Write version
file.write(CSS_CACHE_VERSION); file.write(CssParser::CSS_CACHE_VERSION);
// Write rule count // Write rule count
const auto ruleCount = static_cast<uint16_t>(rulesBySelector_.size()); const auto ruleCount = static_cast<uint16_t>(rulesBySelector_.size());
@@ -613,6 +634,8 @@ bool CssParser::saveToCache() const {
writeLength(style.paddingBottom); writeLength(style.paddingBottom);
writeLength(style.paddingLeft); writeLength(style.paddingLeft);
writeLength(style.paddingRight); writeLength(style.paddingRight);
writeLength(style.imageHeight);
writeLength(style.imageWidth);
// Write defined flags as uint16_t // Write defined flags as uint16_t
uint16_t definedBits = 0; uint16_t definedBits = 0;
@@ -629,6 +652,8 @@ bool CssParser::saveToCache() const {
if (style.defined.paddingBottom) definedBits |= 1 << 10; if (style.defined.paddingBottom) definedBits |= 1 << 10;
if (style.defined.paddingLeft) definedBits |= 1 << 11; if (style.defined.paddingLeft) definedBits |= 1 << 11;
if (style.defined.paddingRight) definedBits |= 1 << 12; if (style.defined.paddingRight) definedBits |= 1 << 12;
if (style.defined.imageHeight) definedBits |= 1 << 13;
if (style.defined.imageWidth) definedBits |= 1 << 14;
file.write(reinterpret_cast<const uint8_t*>(&definedBits), sizeof(definedBits)); file.write(reinterpret_cast<const uint8_t*>(&definedBits), sizeof(definedBits));
} }
@@ -652,9 +677,11 @@ bool CssParser::loadFromCache() {
// Read and verify version // Read and verify version
uint8_t version = 0; uint8_t version = 0;
if (file.read(&version, 1) != 1 || version != CSS_CACHE_VERSION) { if (file.read(&version, 1) != 1 || version != CssParser::CSS_CACHE_VERSION) {
LOG_DBG("CSS", "Cache version mismatch (got %u, expected %u)", version, CSS_CACHE_VERSION); LOG_DBG("CSS", "Cache version mismatch (got %u, expected %u), removing stale cache for rebuild", version,
CssParser::CSS_CACHE_VERSION);
file.close(); file.close();
Storage.remove((cachePath + rulesCache).c_str());
return false; return false;
} }
@@ -730,7 +757,8 @@ bool CssParser::loadFromCache() {
if (!readLength(style.textIndent) || !readLength(style.marginTop) || !readLength(style.marginBottom) || if (!readLength(style.textIndent) || !readLength(style.marginTop) || !readLength(style.marginBottom) ||
!readLength(style.marginLeft) || !readLength(style.marginRight) || !readLength(style.paddingTop) || !readLength(style.marginLeft) || !readLength(style.marginRight) || !readLength(style.paddingTop) ||
!readLength(style.paddingBottom) || !readLength(style.paddingLeft) || !readLength(style.paddingRight)) { !readLength(style.paddingBottom) || !readLength(style.paddingLeft) || !readLength(style.paddingRight) ||
!readLength(style.imageHeight) || !readLength(style.imageWidth)) {
rulesBySelector_.clear(); rulesBySelector_.clear();
file.close(); file.close();
return false; return false;
@@ -756,6 +784,8 @@ bool CssParser::loadFromCache() {
style.defined.paddingBottom = (definedBits & 1 << 10) != 0; style.defined.paddingBottom = (definedBits & 1 << 10) != 0;
style.defined.paddingLeft = (definedBits & 1 << 11) != 0; style.defined.paddingLeft = (definedBits & 1 << 11) != 0;
style.defined.paddingRight = (definedBits & 1 << 12) != 0; style.defined.paddingRight = (definedBits & 1 << 12) != 0;
style.defined.imageHeight = (definedBits & 1 << 13) != 0;
style.defined.imageWidth = (definedBits & 1 << 14) != 0;
rulesBySelector_[selector] = style; rulesBySelector_[selector] = style;
} }

View File

@@ -30,6 +30,9 @@
*/ */
class CssParser { class CssParser {
public: public:
// Bump when CSS cache format or rules change; section caches are invalidated when this changes
static constexpr uint8_t CSS_CACHE_VERSION = 3;
explicit CssParser(std::string cachePath) : cachePath(std::move(cachePath)) {} explicit CssParser(std::string cachePath) : cachePath(std::move(cachePath)) {}
~CssParser() = default; ~CssParser() = default;
@@ -113,6 +116,8 @@ class CssParser {
static CssFontWeight interpretFontWeight(const std::string& val); static CssFontWeight interpretFontWeight(const std::string& val);
static CssTextDecoration interpretDecoration(const std::string& val); static CssTextDecoration interpretDecoration(const std::string& val);
static CssLength interpretLength(const std::string& val); static CssLength interpretLength(const std::string& val);
/** Returns true only when a numeric length was parsed (e.g. 2em, 50%). False for auto/inherit/initial. */
static bool tryInterpretLength(const std::string& val, CssLength& out);
// String utilities // String utilities
static std::string normalized(const std::string& s); static std::string normalized(const std::string& s);

View File

@@ -69,6 +69,8 @@ struct CssPropertyFlags {
uint16_t paddingBottom : 1; uint16_t paddingBottom : 1;
uint16_t paddingLeft : 1; uint16_t paddingLeft : 1;
uint16_t paddingRight : 1; uint16_t paddingRight : 1;
uint16_t imageHeight : 1;
uint16_t imageWidth : 1;
CssPropertyFlags() CssPropertyFlags()
: textAlign(0), : textAlign(0),
@@ -83,17 +85,21 @@ struct CssPropertyFlags {
paddingTop(0), paddingTop(0),
paddingBottom(0), paddingBottom(0),
paddingLeft(0), paddingLeft(0),
paddingRight(0) {} paddingRight(0),
imageHeight(0),
imageWidth(0) {}
[[nodiscard]] bool anySet() const { [[nodiscard]] bool anySet() const {
return textAlign || fontStyle || fontWeight || textDecoration || textIndent || marginTop || marginBottom || return textAlign || fontStyle || fontWeight || textDecoration || textIndent || marginTop || marginBottom ||
marginLeft || marginRight || paddingTop || paddingBottom || paddingLeft || paddingRight; marginLeft || marginRight || paddingTop || paddingBottom || paddingLeft || paddingRight || imageHeight ||
imageWidth;
} }
void clearAll() { void clearAll() {
textAlign = fontStyle = fontWeight = textDecoration = textIndent = 0; textAlign = fontStyle = fontWeight = textDecoration = textIndent = 0;
marginTop = marginBottom = marginLeft = marginRight = 0; marginTop = marginBottom = marginLeft = marginRight = 0;
paddingTop = paddingBottom = paddingLeft = paddingRight = 0; paddingTop = paddingBottom = paddingLeft = paddingRight = 0;
imageHeight = imageWidth = 0;
} }
}; };
@@ -115,6 +121,8 @@ struct CssStyle {
CssLength paddingBottom; // Padding after CssLength paddingBottom; // Padding after
CssLength paddingLeft; // Padding left CssLength paddingLeft; // Padding left
CssLength paddingRight; // Padding right CssLength paddingRight; // Padding right
CssLength imageHeight; // Height for img (e.g. 2em) width derived from aspect ratio when only height set
CssLength imageWidth; // Width for img when both or only width set
CssPropertyFlags defined; // Tracks which properties were explicitly set CssPropertyFlags defined; // Tracks which properties were explicitly set
@@ -173,6 +181,14 @@ struct CssStyle {
paddingRight = base.paddingRight; paddingRight = base.paddingRight;
defined.paddingRight = 1; defined.paddingRight = 1;
} }
if (base.hasImageHeight()) {
imageHeight = base.imageHeight;
defined.imageHeight = 1;
}
if (base.hasImageWidth()) {
imageWidth = base.imageWidth;
defined.imageWidth = 1;
}
} }
[[nodiscard]] bool hasTextAlign() const { return defined.textAlign; } [[nodiscard]] bool hasTextAlign() const { return defined.textAlign; }
@@ -188,6 +204,8 @@ struct CssStyle {
[[nodiscard]] bool hasPaddingBottom() const { return defined.paddingBottom; } [[nodiscard]] bool hasPaddingBottom() const { return defined.paddingBottom; }
[[nodiscard]] bool hasPaddingLeft() const { return defined.paddingLeft; } [[nodiscard]] bool hasPaddingLeft() const { return defined.paddingLeft; }
[[nodiscard]] bool hasPaddingRight() const { return defined.paddingRight; } [[nodiscard]] bool hasPaddingRight() const { return defined.paddingRight; }
[[nodiscard]] bool hasImageHeight() const { return defined.imageHeight; }
[[nodiscard]] bool hasImageWidth() const { return defined.imageWidth; }
void reset() { void reset() {
textAlign = CssTextAlign::Left; textAlign = CssTextAlign::Left;
@@ -197,6 +215,7 @@ struct CssStyle {
textIndent = CssLength{}; textIndent = CssLength{};
marginTop = marginBottom = marginLeft = marginRight = CssLength{}; marginTop = marginBottom = marginLeft = marginRight = CssLength{};
paddingTop = paddingBottom = paddingLeft = paddingRight = CssLength{}; paddingTop = paddingBottom = paddingLeft = paddingRight = CssLength{};
imageHeight = imageWidth = CssLength{};
defined.clearAll(); defined.clearAll();
} }
}; };

View File

@@ -257,6 +257,81 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
if (decoder && decoder->getDimensions(cachedImagePath, dims)) { if (decoder && decoder->getDimensions(cachedImagePath, dims)) {
LOG_DBG("EHP", "Image dimensions: %dx%d", dims.width, dims.height); LOG_DBG("EHP", "Image dimensions: %dx%d", dims.width, dims.height);
int displayWidth = 0;
int displayHeight = 0;
const float emSize =
static_cast<float>(self->renderer.getLineHeight(self->fontId)) * self->lineCompression;
CssStyle imgStyle = self->cssParser ? self->cssParser->resolveStyle("img", classAttr) : CssStyle{};
// Merge inline style (e.g. style="height: 2em") so it overrides stylesheet rules
if (!styleAttr.empty()) {
imgStyle.applyOver(CssParser::parseInlineStyle(styleAttr));
}
const bool hasCssHeight = imgStyle.hasImageHeight();
const bool hasCssWidth = imgStyle.hasImageWidth();
if (hasCssHeight && hasCssWidth && dims.width > 0 && dims.height > 0) {
// Both CSS height and width set: resolve both, then clamp to viewport preserving requested ratio
displayHeight = static_cast<int>(
imgStyle.imageHeight.toPixels(emSize, static_cast<float>(self->viewportHeight)) + 0.5f);
displayWidth = static_cast<int>(
imgStyle.imageWidth.toPixels(emSize, static_cast<float>(self->viewportWidth)) + 0.5f);
if (displayHeight < 1) displayHeight = 1;
if (displayWidth < 1) displayWidth = 1;
if (displayWidth > self->viewportWidth || displayHeight > self->viewportHeight) {
float scaleX = (displayWidth > self->viewportWidth)
? static_cast<float>(self->viewportWidth) / displayWidth
: 1.0f;
float scaleY = (displayHeight > self->viewportHeight)
? static_cast<float>(self->viewportHeight) / displayHeight
: 1.0f;
float scale = (scaleX < scaleY) ? scaleX : scaleY;
displayWidth = static_cast<int>(displayWidth * scale + 0.5f);
displayHeight = static_cast<int>(displayHeight * scale + 0.5f);
if (displayWidth < 1) displayWidth = 1;
if (displayHeight < 1) displayHeight = 1;
}
LOG_DBG("EHP", "Display size from CSS height+width: %dx%d", displayWidth, displayHeight);
} else if (hasCssHeight && !hasCssWidth && dims.width > 0 && dims.height > 0) {
// Use CSS height (resolve % against viewport height) and derive width from aspect ratio
displayHeight = static_cast<int>(
imgStyle.imageHeight.toPixels(emSize, static_cast<float>(self->viewportHeight)) + 0.5f);
if (displayHeight < 1) displayHeight = 1;
displayWidth =
static_cast<int>(displayHeight * (static_cast<float>(dims.width) / dims.height) + 0.5f);
if (displayHeight > self->viewportHeight) {
displayHeight = self->viewportHeight;
// Rescale width to preserve aspect ratio when height is clamped
displayWidth =
static_cast<int>(displayHeight * (static_cast<float>(dims.width) / dims.height) + 0.5f);
if (displayWidth < 1) displayWidth = 1;
}
if (displayWidth > self->viewportWidth) {
displayWidth = self->viewportWidth;
// Rescale height to preserve aspect ratio when width is clamped
displayHeight =
static_cast<int>(displayWidth * (static_cast<float>(dims.height) / dims.width) + 0.5f);
if (displayHeight < 1) displayHeight = 1;
}
if (displayWidth < 1) displayWidth = 1;
LOG_DBG("EHP", "Display size from CSS height: %dx%d", displayWidth, displayHeight);
} else if (hasCssWidth && !hasCssHeight && dims.width > 0 && dims.height > 0) {
// Use CSS width (resolve % against viewport width) and derive height from aspect ratio
displayWidth = static_cast<int>(
imgStyle.imageWidth.toPixels(emSize, static_cast<float>(self->viewportWidth)) + 0.5f);
if (displayWidth > self->viewportWidth) displayWidth = self->viewportWidth;
if (displayWidth < 1) displayWidth = 1;
displayHeight =
static_cast<int>(displayWidth * (static_cast<float>(dims.height) / dims.width) + 0.5f);
if (displayHeight > self->viewportHeight) {
displayHeight = self->viewportHeight;
// Rescale width to preserve aspect ratio when height is clamped
displayWidth =
static_cast<int>(displayHeight * (static_cast<float>(dims.width) / dims.height) + 0.5f);
if (displayWidth < 1) displayWidth = 1;
}
if (displayHeight < 1) displayHeight = 1;
LOG_DBG("EHP", "Display size from CSS width: %dx%d", displayWidth, displayHeight);
} else {
// Scale to fit viewport while maintaining aspect ratio // Scale to fit viewport while maintaining aspect ratio
int maxWidth = self->viewportWidth; int maxWidth = self->viewportWidth;
int maxHeight = self->viewportHeight; int maxHeight = self->viewportHeight;
@@ -265,10 +340,10 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
float scale = (scaleX < scaleY) ? scaleX : scaleY; float scale = (scaleX < scaleY) ? scaleX : scaleY;
if (scale > 1.0f) scale = 1.0f; if (scale > 1.0f) scale = 1.0f;
int displayWidth = (int)(dims.width * scale); displayWidth = (int)(dims.width * scale);
int displayHeight = (int)(dims.height * scale); displayHeight = (int)(dims.height * scale);
LOG_DBG("EHP", "Display size: %dx%d (scale %.2f)", displayWidth, displayHeight, scale); LOG_DBG("EHP", "Display size: %dx%d (scale %.2f)", displayWidth, displayHeight, scale);
}
// Create page for image - only break if image won't fit remaining space // Create page for image - only break if image won't fit remaining space
if (self->currentPage && !self->currentPage->elements.empty() && if (self->currentPage && !self->currentPage->elements.empty() &&