Merge 9dac5bf27e into da4d3b5ea5

- margin, padding, and text-indent now all support ems, rems, and px values
- shorthand margin/padding CSS is also supported
- margin/padding/indent values of 0 should no longer erroneously produce additional spacing

lib/Epub/Epub/ParsedText.cpp

@@ -93,20 +93,9 @@ std::vector<uint16_t> ParsedText::calculateWordWidths(const GfxRenderer& rendere
 
   auto wordsIt = words.begin();
   auto wordStylesIt = wordStyles.begin();
-  bool isFirst = true;
 
   while (wordsIt != words.end()) {
     uint16_t width = measureWordWidth(renderer, fontId, *wordsIt, *wordStylesIt);
-
-    // Add CSS text-indent to first word width
-    if (isFirst && blockStyle.textIndent > 0 && (style == TextBlock::JUSTIFIED || style == TextBlock::LEFT_ALIGN) &&
-        !extraParagraphSpacing) {
-      width += static_cast<uint16_t>(blockStyle.textIndent);
-      isFirst = false;
-    } else {
-      isFirst = false;
-    }
-
     wordWidths.push_back(width);
 
     std::advance(wordsIt, 1);
@@ -122,10 +111,18 @@ std::vector<size_t> ParsedText::computeLineBreaks(const GfxRenderer& renderer, c
     return {};
   }
 
+  // Calculate first line indent (only for left/justified text without extra paragraph spacing)
+  const int firstLineIndent = blockStyle.textIndent > 0 && !extraParagraphSpacing &&
+                                      (style == TextBlock::JUSTIFIED || style == TextBlock::LEFT_ALIGN)
+                                  ? blockStyle.textIndent
+                                  : 0;
+
   // Ensure any word that would overflow even as the first entry on a line is split using fallback hyphenation.
   for (size_t i = 0; i < wordWidths.size(); ++i) {
-    while (wordWidths[i] > pageWidth) {
-      if (!hyphenateWordAtIndex(i, pageWidth, renderer, fontId, wordWidths, /*allowFallbackBreaks=*/true)) {
+    // First word needs to fit in reduced width if there's an indent
+    const int effectiveWidth = i == 0 ? pageWidth - firstLineIndent : pageWidth;
+    while (wordWidths[i] > effectiveWidth) {
+      if (!hyphenateWordAtIndex(i, effectiveWidth, renderer, fontId, wordWidths, /*allowFallbackBreaks=*/true)) {
         break;
       }
     }
@@ -146,11 +143,14 @@ std::vector<size_t> ParsedText::computeLineBreaks(const GfxRenderer& renderer, c
     int currlen = -spaceWidth;
     dp[i] = MAX_COST;
 
+    // First line has reduced width due to text-indent
+    const int effectivePageWidth = i == 0 ? pageWidth - firstLineIndent : pageWidth;
+
     for (size_t j = i; j < totalWordCount; ++j) {
       // Current line length: previous width + space + current word width
       currlen += wordWidths[j] + spaceWidth;
 
-      if (currlen > pageWidth) {
+      if (currlen > effectivePageWidth) {
         break;
       }
 
@@ -158,7 +158,7 @@ std::vector<size_t> ParsedText::computeLineBreaks(const GfxRenderer& renderer, c
       if (j == totalWordCount - 1) {
         cost = 0;  // Last line
       } else {
-        const int remainingSpace = pageWidth - currlen;
+        const int remainingSpace = effectivePageWidth - currlen;
         // Use long long for the square to prevent overflow
         const long long cost_ll = static_cast<long long>(remainingSpace) * remainingSpace + dp[j + 1];
 
@@ -213,10 +213,11 @@ void ParsedText::applyParagraphIndent() {
     return;
   }
 
-  if (blockStyle.textIndent > 0) {
-    // CSS text-indent is handled via first word width adjustment
-    // We'll add the indent value directly to the first word's width
+  if (blockStyle.textIndentDefined) {
+    // CSS text-indent is explicitly set (even if 0) - don't use fallback EmSpace
+    // The actual indent positioning is handled in extractLine()
   } else if (style == TextBlock::JUSTIFIED || style == TextBlock::LEFT_ALIGN) {
+    // No CSS text-indent defined - use EmSpace fallback for visual indent
     words.front().insert(0, "\xe2\x80\x83");
   }
 }
@@ -225,13 +226,23 @@ void ParsedText::applyParagraphIndent() {
 std::vector<size_t> ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& renderer, const int fontId,
                                                             const int pageWidth, const int spaceWidth,
                                                             std::vector<uint16_t>& wordWidths) {
+  // Calculate first line indent (only for left/justified text without extra paragraph spacing)
+  const int firstLineIndent = blockStyle.textIndent > 0 && !extraParagraphSpacing &&
+                                      (style == TextBlock::JUSTIFIED || style == TextBlock::LEFT_ALIGN)
+                                  ? blockStyle.textIndent
+                                  : 0;
+
   std::vector<size_t> lineBreakIndices;
   size_t currentIndex = 0;
+  bool isFirstLine = true;
 
   while (currentIndex < wordWidths.size()) {
     const size_t lineStart = currentIndex;
     int lineWidth = 0;
 
+    // First line has reduced width due to text-indent
+    const int effectivePageWidth = isFirstLine ? pageWidth - firstLineIndent : pageWidth;
+
     // Consume as many words as possible for current line, splitting when prefixes fit
     while (currentIndex < wordWidths.size()) {
       const bool isFirstWord = currentIndex == lineStart;
@@ -239,14 +250,14 @@ std::vector<size_t> ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& r
       const int candidateWidth = spacing + wordWidths[currentIndex];
 
       // Word fits on current line
-      if (lineWidth + candidateWidth <= pageWidth) {
+      if (lineWidth + candidateWidth <= effectivePageWidth) {
         lineWidth += candidateWidth;
         ++currentIndex;
         continue;
       }
 
       // Word would overflow — try to split based on hyphenation points
-      const int availableWidth = pageWidth - lineWidth - spacing;
+      const int availableWidth = effectivePageWidth - lineWidth - spacing;
       const bool allowFallbackBreaks = isFirstWord;  // Only for first word on line
 
       if (availableWidth > 0 &&
@@ -266,6 +277,7 @@ std::vector<size_t> ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& r
     }
 
     lineBreakIndices.push_back(currentIndex);
+    isFirstLine = false;
   }
 
   return lineBreakIndices;
@@ -350,14 +362,22 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const
   const size_t lastBreakAt = breakIndex > 0 ? lineBreakIndices[breakIndex - 1] : 0;
   const size_t lineWordCount = lineBreak - lastBreakAt;
 
+  // Calculate first line indent (only for left/justified text without extra paragraph spacing)
+  const bool isFirstLine = breakIndex == 0;
+  const int firstLineIndent = isFirstLine && blockStyle.textIndent > 0 && !extraParagraphSpacing &&
+                                      (style == TextBlock::JUSTIFIED || style == TextBlock::LEFT_ALIGN)
+                                  ? blockStyle.textIndent
+                                  : 0;
+
   // Calculate total word width for this line
   int lineWordWidthSum = 0;
   for (size_t i = lastBreakAt; i < lineBreak; i++) {
     lineWordWidthSum += wordWidths[i];
   }
 
-  // Calculate spacing
-  const int spareSpace = pageWidth - lineWordWidthSum;
+  // Calculate spacing (account for indent reducing effective page width on first line)
+  const int effectivePageWidth = pageWidth - firstLineIndent;
+  const int spareSpace = effectivePageWidth - lineWordWidthSum;
 
   int spacing = spaceWidth;
   const bool isLastLine = breakIndex == lineBreakIndices.size() - 1;
@@ -366,8 +386,8 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const
     spacing = spareSpace / (lineWordCount - 1);
   }
 
-  // Calculate initial x position
-  uint16_t xpos = 0;
+  // Calculate initial x position (first line starts at indent for left/justified text)
+  auto xpos = static_cast<uint16_t>(firstLineIndent);
   if (style == TextBlock::RIGHT_ALIGN) {
     xpos = spareSpace - (lineWordCount - 1) * spaceWidth;
   } else if (style == TextBlock::CENTER_ALIGN) {

lib/Epub/Epub/blocks/BlockStyle.h

@@ -9,9 +9,19 @@
  * Padding is treated similarly to margins for rendering purposes.
  */
 struct BlockStyle {
-  int8_t marginTop = 0;      // 0-2 lines
-  int8_t marginBottom = 0;   // 0-2 lines
-  int8_t paddingTop = 0;     // 0-2 lines (treated same as margin)
-  int8_t paddingBottom = 0;  // 0-2 lines (treated same as margin)
-  int16_t textIndent = 0;    // pixels
+  int16_t marginTop = 0;           // pixels
+  int16_t marginBottom = 0;        // pixels
+  int16_t marginLeft = 0;          // pixels
+  int16_t marginRight = 0;         // pixels
+  int16_t paddingTop = 0;          // pixels (treated same as margin)
+  int16_t paddingBottom = 0;       // pixels (treated same as margin)
+  int16_t paddingLeft = 0;         // pixels (treated same as margin)
+  int16_t paddingRight = 0;        // pixels (treated same as margin)
+  int16_t textIndent = 0;          // pixels
+  bool textIndentDefined = false;  // true if text-indent was explicitly set in CSS
+
+  // Combined horizontal 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(); }
 };

lib/Epub/Epub/blocks/TextBlock.cpp

@@ -89,9 +89,14 @@ bool TextBlock::serialize(FsFile& file) const {
   // Block style (margins/padding/indent)
   serialization::writePod(file, blockStyle.marginTop);
   serialization::writePod(file, blockStyle.marginBottom);
+  serialization::writePod(file, blockStyle.marginLeft);
+  serialization::writePod(file, blockStyle.marginRight);
   serialization::writePod(file, blockStyle.paddingTop);
   serialization::writePod(file, blockStyle.paddingBottom);
+  serialization::writePod(file, blockStyle.paddingLeft);
+  serialization::writePod(file, blockStyle.paddingRight);
   serialization::writePod(file, blockStyle.textIndent);
+  serialization::writePod(file, blockStyle.textIndentDefined);
 
   return true;
 }
@@ -141,9 +146,14 @@ std::unique_ptr<TextBlock> TextBlock::deserialize(FsFile& file) {
   // Block style (margins/padding/indent)
   serialization::readPod(file, blockStyle.marginTop);
   serialization::readPod(file, blockStyle.marginBottom);
+  serialization::readPod(file, blockStyle.marginLeft);
+  serialization::readPod(file, blockStyle.marginRight);
   serialization::readPod(file, blockStyle.paddingTop);
   serialization::readPod(file, blockStyle.paddingBottom);
+  serialization::readPod(file, blockStyle.paddingLeft);
+  serialization::readPod(file, blockStyle.paddingRight);
   serialization::readPod(file, blockStyle.textIndent);
+  serialization::readPod(file, blockStyle.textIndentDefined);
 
   return std::unique_ptr<TextBlock>(new TextBlock(std::move(words), std::move(wordXpos), std::move(wordStyles), style,
                                                   blockStyle, std::move(wordUnderlines)));

lib/Epub/Epub/css/CssParser.cpp

@@ -370,28 +370,57 @@ CssStyle CssParser::parseDeclarations(const std::string& declBlock) {
       style.indentPixels = interpretLength(propValue);
       style.defined.indent = 1;
     } else if (propName == "margin-top") {
-      const int8_t spacing = interpretSpacing(propValue);
-      if (spacing > 0) {
-        style.marginTop = spacing;
-        style.defined.marginTop = 1;
-      }
+      style.marginTop = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.marginTop = 1;
     } else if (propName == "margin-bottom") {
-      const int8_t spacing = interpretSpacing(propValue);
-      if (spacing > 0) {
-        style.marginBottom = spacing;
-        style.defined.marginBottom = 1;
+      style.marginBottom = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.marginBottom = 1;
+    } else if (propName == "margin-left") {
+      style.marginLeft = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.marginLeft = 1;
+    } else if (propName == "margin-right") {
+      style.marginRight = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.marginRight = 1;
+    } else if (propName == "margin") {
+      // Shorthand: 1-4 values for top, right, bottom, left
+      const auto values = splitWhitespace(propValue);
+      if (!values.empty()) {
+        const auto top = static_cast<int16_t>(interpretLength(values[0]));
+        const int16_t right = values.size() >= 2 ? static_cast<int16_t>(interpretLength(values[1])) : top;
+        const int16_t bottom = values.size() >= 3 ? static_cast<int16_t>(interpretLength(values[2])) : top;
+        const int16_t left = values.size() >= 4 ? static_cast<int16_t>(interpretLength(values[3])) : right;
+        style.marginTop = top;
+        style.marginRight = right;
+        style.marginBottom = bottom;
+        style.marginLeft = left;
+        style.defined.marginTop = style.defined.marginRight = style.defined.marginBottom = style.defined.marginLeft = 1;
       }
     } else if (propName == "padding-top") {
-      const int8_t spacing = interpretSpacing(propValue);
-      if (spacing > 0) {
-        style.paddingTop = spacing;
-        style.defined.paddingTop = 1;
-      }
+      style.paddingTop = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.paddingTop = 1;
     } else if (propName == "padding-bottom") {
-      const int8_t spacing = interpretSpacing(propValue);
-      if (spacing > 0) {
-        style.paddingBottom = spacing;
-        style.defined.paddingBottom = 1;
+      style.paddingBottom = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.paddingBottom = 1;
+    } else if (propName == "padding-left") {
+      style.paddingLeft = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.paddingLeft = 1;
+    } else if (propName == "padding-right") {
+      style.paddingRight = static_cast<int16_t>(interpretLength(propValue));
+      style.defined.paddingRight = 1;
+    } else if (propName == "padding") {
+      // Shorthand: 1-4 values for top, right, bottom, left
+      const auto values = splitWhitespace(propValue);
+      if (!values.empty()) {
+        const auto top = static_cast<int16_t>(interpretLength(values[0]));
+        const int16_t right = values.size() >= 2 ? static_cast<int16_t>(interpretLength(values[1])) : top;
+        const int16_t bottom = values.size() >= 3 ? static_cast<int16_t>(interpretLength(values[2])) : top;
+        const int16_t left = values.size() >= 4 ? static_cast<int16_t>(interpretLength(values[3])) : right;
+        style.paddingTop = top;
+        style.paddingRight = right;
+        style.paddingBottom = bottom;
+        style.paddingLeft = left;
+        style.defined.paddingTop = style.defined.paddingRight = style.defined.paddingBottom =
+            style.defined.paddingLeft = 1;
       }
     }
   }

lib/Epub/Epub/css/CssStyle.h

@@ -23,9 +23,13 @@ struct CssPropertyFlags {
   uint16_t indent : 1;
   uint16_t marginTop : 1;
   uint16_t marginBottom : 1;
+  uint16_t marginLeft : 1;
+  uint16_t marginRight : 1;
   uint16_t paddingTop : 1;
   uint16_t paddingBottom : 1;
-  uint16_t reserved : 7;
+  uint16_t paddingLeft : 1;
+  uint16_t paddingRight : 1;
+  uint16_t reserved : 3;
 
   CssPropertyFlags()
       : alignment(0),
@@ -35,18 +39,23 @@ struct CssPropertyFlags {
         indent(0),
         marginTop(0),
         marginBottom(0),
+        marginLeft(0),
+        marginRight(0),
         paddingTop(0),
         paddingBottom(0),
+        paddingLeft(0),
+        paddingRight(0),
         reserved(0) {}
 
   [[nodiscard]] bool anySet() const {
-    return alignment || fontStyle || fontWeight || decoration || indent || marginTop || marginBottom || paddingTop ||
-           paddingBottom;
+    return alignment || fontStyle || fontWeight || decoration || indent || marginTop || marginBottom || marginLeft ||
+           marginRight || paddingTop || paddingBottom || paddingLeft || paddingRight;
   }
 
   void clearAll() {
     alignment = fontStyle = fontWeight = decoration = indent = 0;
-    marginTop = marginBottom = paddingTop = paddingBottom = 0;
+    marginTop = marginBottom = marginLeft = marginRight = 0;
+    paddingTop = paddingBottom = paddingLeft = paddingRight = 0;
   }
 };
 
@@ -59,10 +68,14 @@ struct CssStyle {
   CssTextDecoration decoration = CssTextDecoration::None;
 
   float indentPixels = 0.0f;  // First-line indent in pixels
-  int8_t marginTop = 0;       // Vertical spacing before block (in lines, 0-2)
-  int8_t marginBottom = 0;    // Vertical spacing after block (in lines, 0-2)
-  int8_t paddingTop = 0;      // Padding before (in lines, 0-2)
-  int8_t paddingBottom = 0;   // Padding after (in lines, 0-2)
+  int16_t marginTop = 0;      // Vertical spacing before block (in pixels)
+  int16_t marginBottom = 0;   // Vertical spacing after block (in pixels)
+  int16_t marginLeft = 0;     // Horizontal spacing left of block (in pixels)
+  int16_t marginRight = 0;    // Horizontal spacing right of block (in pixels)
+  int16_t paddingTop = 0;     // Padding before (in pixels)
+  int16_t paddingBottom = 0;  // Padding after (in pixels)
+  int16_t paddingLeft = 0;    // Padding left (in pixels)
+  int16_t paddingRight = 0;   // Padding right (in pixels)
 
   CssPropertyFlags defined;  // Tracks which properties were explicitly set
 
@@ -97,6 +110,14 @@ struct CssStyle {
       marginBottom = base.marginBottom;
       defined.marginBottom = 1;
     }
+    if (base.defined.marginLeft) {
+      marginLeft = base.marginLeft;
+      defined.marginLeft = 1;
+    }
+    if (base.defined.marginRight) {
+      marginRight = base.marginRight;
+      defined.marginRight = 1;
+    }
     if (base.defined.paddingTop) {
       paddingTop = base.paddingTop;
       defined.paddingTop = 1;
@@ -105,6 +126,14 @@ struct CssStyle {
       paddingBottom = base.paddingBottom;
       defined.paddingBottom = 1;
     }
+    if (base.defined.paddingLeft) {
+      paddingLeft = base.paddingLeft;
+      defined.paddingLeft = 1;
+    }
+    if (base.defined.paddingRight) {
+      paddingRight = base.paddingRight;
+      defined.paddingRight = 1;
+    }
   }
 
   // Compatibility accessors for existing code that uses hasX pattern
@@ -115,8 +144,12 @@ struct CssStyle {
   [[nodiscard]] bool hasTextIndent() const { return defined.indent; }
   [[nodiscard]] bool hasMarginTop() const { return defined.marginTop; }
   [[nodiscard]] bool hasMarginBottom() const { return defined.marginBottom; }
+  [[nodiscard]] bool hasMarginLeft() const { return defined.marginLeft; }
+  [[nodiscard]] bool hasMarginRight() const { return defined.marginRight; }
   [[nodiscard]] bool hasPaddingTop() const { return defined.paddingTop; }
   [[nodiscard]] bool hasPaddingBottom() const { return defined.paddingBottom; }
+  [[nodiscard]] bool hasPaddingLeft() const { return defined.paddingLeft; }
+  [[nodiscard]] bool hasPaddingRight() const { return defined.paddingRight; }
 
   // Merge another style (alias for applyOver for compatibility)
   void merge(const CssStyle& other) { applyOver(other); }
@@ -127,7 +160,8 @@ struct CssStyle {
     fontWeight = CssFontWeight::Normal;
     decoration = CssTextDecoration::None;
     indentPixels = 0.0f;
-    marginTop = marginBottom = paddingTop = paddingBottom = 0;
+    marginTop = marginBottom = marginLeft = marginRight = 0;
+    paddingTop = paddingBottom = paddingLeft = paddingRight = 0;
     defined.clearAll();
   }
 };

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp

@@ -46,11 +46,19 @@ bool matches(const char* tag_name, const char* possible_tags[], const int possib
 // Create a BlockStyle from CSS style properties
 BlockStyle createBlockStyleFromCss(const CssStyle& cssStyle) {
   BlockStyle blockStyle;
-  blockStyle.marginTop = static_cast<int8_t>(cssStyle.marginTop + cssStyle.paddingTop);
-  blockStyle.marginBottom = static_cast<int8_t>(cssStyle.marginBottom + cssStyle.paddingBottom);
+  // Vertical: combine margin and padding for top/bottom spacing
+  blockStyle.marginTop = static_cast<int16_t>(cssStyle.marginTop + cssStyle.paddingTop);
+  blockStyle.marginBottom = static_cast<int16_t>(cssStyle.marginBottom + cssStyle.paddingBottom);
   blockStyle.paddingTop = cssStyle.paddingTop;
   blockStyle.paddingBottom = cssStyle.paddingBottom;
+  // Horizontal: store margin and padding separately for layout calculations
+  blockStyle.marginLeft = cssStyle.marginLeft;
+  blockStyle.marginRight = cssStyle.marginRight;
+  blockStyle.paddingLeft = cssStyle.paddingLeft;
+  blockStyle.paddingRight = cssStyle.paddingRight;
+  // Text indent
   blockStyle.textIndent = static_cast<int16_t>(cssStyle.indentPixels);
+  blockStyle.textIndentDefined = cssStyle.defined.indent;
   return blockStyle;
 }
 
@@ -570,7 +578,9 @@ void ChapterHtmlSlimParser::addLineToPage(std::shared_ptr<TextBlock> line) {
     currentPageNextY = 0;
   }
 
-  currentPage->elements.push_back(std::make_shared<PageLine>(line, 0, currentPageNextY));
+  // Apply horizontal left inset (margin + padding) as x position offset
+  const int16_t xOffset = line->getBlockStyle().leftInset();
+  currentPage->elements.push_back(std::make_shared<PageLine>(line, xOffset, currentPageNextY));
   currentPageNextY += lineHeight;
 }
 
@@ -587,19 +597,24 @@ void ChapterHtmlSlimParser::makePages() {
 
   const int lineHeight = renderer.getLineHeight(fontId) * lineCompression;
 
-  // Apply marginTop before the paragraph
+  // Apply marginTop before the paragraph (stored in pixels)
   const BlockStyle& blockStyle = currentTextBlock->getBlockStyle();
   if (blockStyle.marginTop > 0) {
-    currentPageNextY += lineHeight * blockStyle.marginTop;
+    currentPageNextY += blockStyle.marginTop;
   }
 
+  // Calculate effective width accounting for horizontal margins/padding
+  const int horizontalInset = blockStyle.totalHorizontalInset();
+  const uint16_t effectiveWidth =
+      (horizontalInset < viewportWidth) ? static_cast<uint16_t>(viewportWidth - horizontalInset) : viewportWidth;
+
   currentTextBlock->layoutAndExtractLines(
-      renderer, fontId, viewportWidth,
+      renderer, fontId, effectiveWidth,
       [this](const std::shared_ptr<TextBlock>& textBlock) { addLineToPage(textBlock); });
 
-  // Apply marginBottom after the paragraph
+  // Apply marginBottom after the paragraph (stored in pixels)
   if (blockStyle.marginBottom > 0) {
-    currentPageNextY += lineHeight * blockStyle.marginBottom;
+    currentPageNextY += blockStyle.marginBottom;
   }
 
   // Extra paragraph spacing if enabled (default behavior)