From 8b940db781ac2c6a0c708346062f8b98846406eb Mon Sep 17 00:00:00 2001 From: Zach Nelson Date: Thu, 16 Apr 2026 00:39:35 -0500 Subject: [PATCH] Combine approaches with #1454 --- lib/Epub/Epub/blocks/BlockStyle.h | 74 ++++++++++++------- .../Epub/parsers/ChapterHtmlSlimParser.cpp | 56 +++++++++----- 2 files changed, 82 insertions(+), 48 deletions(-) diff --git a/lib/Epub/Epub/blocks/BlockStyle.h b/lib/Epub/Epub/blocks/BlockStyle.h index a5a616bf9..93e15c7e1 100644 --- a/lib/Epub/Epub/blocks/BlockStyle.h +++ b/lib/Epub/Epub/blocks/BlockStyle.h @@ -23,42 +23,60 @@ struct BlockStyle { bool textIndentDefined = false; // true if text-indent was explicitly set in CSS bool textAlignDefined = false; // true if text-align was explicitly set in CSS - // Combined horizontal insets (margin + padding) + // Combined insets (margin + padding) [[nodiscard]] int16_t leftInset() const { return marginLeft + paddingLeft; } [[nodiscard]] int16_t rightInset() const { return marginRight + paddingRight; } [[nodiscard]] int16_t totalHorizontalInset() const { return leftInset() + rightInset(); } + [[nodiscard]] int16_t topInset() const { return marginTop + paddingTop; } + [[nodiscard]] int16_t bottomInset() const { return marginBottom + paddingBottom; } - // Combine with another block style. Useful for parent -> child styles, where the child style should be - // applied on top of the parent's style to get the combined style. - BlockStyle getCombinedBlockStyle(const BlockStyle& child) const { - BlockStyle combinedBlockStyle; + // Return a copy with bottom margins/padding zeroed out. + [[nodiscard]] BlockStyle withoutBottom() const { + BlockStyle result = *this; + result.marginBottom = 0; + result.paddingBottom = 0; + return result; + } - combinedBlockStyle.marginTop = static_cast(child.marginTop + marginTop); - combinedBlockStyle.marginBottom = static_cast(child.marginBottom + marginBottom); - combinedBlockStyle.marginLeft = static_cast(child.marginLeft + marginLeft); - combinedBlockStyle.marginRight = static_cast(child.marginRight + marginRight); + // Return a copy with the source's bottom margins/padding added to this style's. + [[nodiscard]] BlockStyle addBottom(const BlockStyle& source) const { + BlockStyle result = *this; + result.marginBottom = static_cast(marginBottom + source.marginBottom); + result.paddingBottom = static_cast(paddingBottom + source.paddingBottom); + return result; + } - combinedBlockStyle.paddingTop = static_cast(child.paddingTop + paddingTop); - combinedBlockStyle.paddingBottom = static_cast(child.paddingBottom + paddingBottom); - combinedBlockStyle.paddingLeft = static_cast(child.paddingLeft + paddingLeft); - combinedBlockStyle.paddingRight = static_cast(child.paddingRight + paddingRight); - // Text indent: use child's if defined - if (child.textIndentDefined) { - combinedBlockStyle.textIndent = child.textIndent; - combinedBlockStyle.textIndentDefined = true; + enum class CombineAxis : uint8_t { + Horizontal = 1, // margins left/right, padding left/right, text-align, text-indent + Vertical = 2, // margins top/bottom, padding top/bottom + }; + + // Combine this style's properties with a child style along the specified axis. + // Properties on the other axis are kept from the child unchanged. + [[nodiscard]] BlockStyle getCombinedBlockStyle(const BlockStyle& child, CombineAxis axis) const { + BlockStyle result = child; + + if (axis == CombineAxis::Horizontal) { + result.marginLeft = static_cast(child.marginLeft + marginLeft); + result.marginRight = static_cast(child.marginRight + marginRight); + result.paddingLeft = static_cast(child.paddingLeft + paddingLeft); + result.paddingRight = static_cast(child.paddingRight + paddingRight); + if (!child.textIndentDefined && textIndentDefined) { + result.textIndent = textIndent; + result.textIndentDefined = true; + } + if (!child.textAlignDefined && textAlignDefined) { + result.alignment = alignment; + result.textAlignDefined = true; + } } else { - combinedBlockStyle.textIndent = textIndent; - combinedBlockStyle.textIndentDefined = textIndentDefined; + result.marginTop = static_cast(child.marginTop + marginTop); + result.marginBottom = static_cast(child.marginBottom + marginBottom); + result.paddingTop = static_cast(child.paddingTop + paddingTop); + result.paddingBottom = static_cast(child.paddingBottom + paddingBottom); } - // Text align: use child's if defined - if (child.textAlignDefined) { - combinedBlockStyle.alignment = child.alignment; - combinedBlockStyle.textAlignDefined = true; - } else { - combinedBlockStyle.alignment = alignment; - combinedBlockStyle.textAlignDefined = textAlignDefined; - } - return combinedBlockStyle; + + return result; } // Create a BlockStyle from CSS style properties, resolving CssLength values to pixels diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index e75421eac..bdab57594 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -130,9 +130,13 @@ void ChapterHtmlSlimParser::startNewTextBlock(const BlockStyle& blockStyle) { if (currentTextBlock) { // already have a text block running and it is empty - just reuse it if (currentTextBlock->isEmpty()) { - // Set the block style directly. Callers from block/header element opens pass the - // fully accumulated style (parent stack + this element) so merging is not needed. - currentTextBlock->setBlockStyle(blockStyle); + // The stack accumulates horizontal margins and text properties from ancestors. + // Vertical margins are per-element and not inherited through the stack, but + // container elements deposit their vertical margins on the empty block when they + // open. Merge those into the new style so the first child in a container inherits + // the container's vertical spacing. + currentTextBlock->setBlockStyle( + currentTextBlock->getBlockStyle().getCombinedBlockStyle(blockStyle, BlockStyle::CombineAxis::Vertical)); if (!pendingAnchorId.empty()) { anchorData.push_back({std::move(pendingAnchorId), static_cast(completedPageCount)}); @@ -434,16 +438,18 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* self->startNewTextBlock(parentBlockStyle); } - // Apply vertical margins from the current (possibly empty) text block. - // When an image is inside a container like
, - // the div's margins live on the empty text block but are never flushed - // via makePages(). Apply them here so the image respects vertical spacing. + // Apply vertical margins from the container to the image. + // Top margin lives on the empty text block (deposited via vertical merge + // in startNewTextBlock). Bottom margin was stripped by withoutBottom() for + // deferred application at element close, so read it from the stack. int16_t imageMarginTop = 0; int16_t imageMarginBottom = 0; if (self->currentTextBlock && self->currentTextBlock->isEmpty()) { const auto& bs = self->currentTextBlock->getBlockStyle(); - imageMarginTop = static_cast(bs.marginTop + bs.paddingTop); - imageMarginBottom = static_cast(bs.marginBottom + bs.paddingBottom); + imageMarginTop = bs.topInset(); + if (self->blockStyleStack.size() > 1) { + imageMarginBottom = self->blockStyleStack.back().bottomInset(); + } } // Create page for image - only break if image won't fit remaining space @@ -500,7 +506,10 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Fallback to alt text if image processing fails if (!alt.empty()) { alt = "[Image: " + alt + "]"; - self->startNewTextBlock(self->blockStyleStack.back().getCombinedBlockStyle(centeredBlockStyle)); + self->startNewTextBlock( + self->blockStyleStack.back() + .getCombinedBlockStyle(centeredBlockStyle, BlockStyle::CombineAxis::Horizontal) + .withoutBottom()); self->italicUntilDepth = std::min(self->italicUntilDepth, self->depth); self->depth += 1; self->characterData(userData, alt.c_str(), alt.length()); @@ -590,9 +599,10 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* if (self->embeddedStyle && cssStyle.hasTextAlign()) { headerBlockStyle.alignment = cssStyle.textAlign; } - const auto accumulated = self->blockStyleStack.back().getCombinedBlockStyle(headerBlockStyle); + const auto accumulated = + self->blockStyleStack.back().getCombinedBlockStyle(headerBlockStyle, BlockStyle::CombineAxis::Horizontal); self->blockStyleStack.push_back(accumulated); - self->startNewTextBlock(accumulated); + self->startNewTextBlock(accumulated.withoutBottom()); self->boldUntilDepth = std::min(self->boldUntilDepth, self->depth); self->updateEffectiveInlineStyle(); } else if (matches(name, BLOCK_TAGS, NUM_BLOCK_TAGS)) { @@ -601,12 +611,13 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // flush word preceding
to currentTextBlock before calling startNewTextBlock self->flushPartWordBuffer(); } - self->startNewTextBlock(self->currentTextBlock->getBlockStyle()); + self->startNewTextBlock(self->blockStyleStack.back().withoutBottom()); } else { self->currentCssStyle = cssStyle; - const auto accumulated = self->blockStyleStack.back().getCombinedBlockStyle(userAlignmentBlockStyle); + const auto accumulated = + self->blockStyleStack.back().getCombinedBlockStyle(userAlignmentBlockStyle, BlockStyle::CombineAxis::Horizontal); self->blockStyleStack.push_back(accumulated); - self->startNewTextBlock(accumulated); + self->startNewTextBlock(accumulated.withoutBottom()); self->updateEffectiveInlineStyle(); if (strcmp(name, "li") == 0) { @@ -999,15 +1010,20 @@ void XMLCALL ChapterHtmlSlimParser::endElement(void* userData, const XML_Char* n self->currentCssStyle.reset(); self->updateEffectiveInlineStyle(); - // Pop this element's contribution from the block style stack and restore the - // parent's accumulated style on empty blocks. This prevents closed elements' - // styles (alignment, margins, padding) from bleeding into siblings while - // correctly preserving ancestor styles for subsequent children. // br is self-closing and not a container — it doesn't push/pop the stack. if (strcmp(name, "br") != 0 && self->blockStyleStack.size() > 1) { + // Apply closing element's bottom margin to the current text block so + // container spacing appears after the element's content (on the last child), + // not on the first child via the empty-block merge in startNewTextBlock. + if (self->currentTextBlock) { + self->currentTextBlock->setBlockStyle( + self->currentTextBlock->getBlockStyle().addBottom(self->blockStyleStack.back())); + } self->blockStyleStack.pop_back(); + // Restore parent's accumulated style on empty blocks to prevent the closed + // element's styles from bleeding into the next sibling. if (self->currentTextBlock && self->currentTextBlock->isEmpty()) { - self->currentTextBlock->setBlockStyle(self->blockStyleStack.back()); + self->currentTextBlock->setBlockStyle(self->blockStyleStack.back().withoutBottom()); } } }