cottongin/crosspoint-reader-mod
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Hanging indent (negative text-indent) and em-unit sizing (#1229)

Browse Source 
## Summary

* **What is the goal of this PR?** Fixing two independent CSS rendering
bugs combined to make hanging-indent list styles
(e.g. margin-left:3em; text-indent:-1em) render incorrectly:

* **What changes are included?**
  1. Negative text-indent was silently ignored

Three guards in ParsedText.cpp (computeLineBreaks,
computeHyphenatedLineBreaks,
extractLine) conditioned firstLineIndent on blockStyle.textIndent > 0,
so any
negative value collapsed to zero. Additionally, wordXpos was uint16_t,
which
cannot represent negative offsets — a cast of e.g. −18 would wrap to
65518 and
      render the word far off-screen.

   2. extraParagraphSpacing suppressed hanging indents

Even after removing the > 0 guard, the existing !extraParagraphSpacing
condition
would still suppress all text-indent when that setting is on (its
default). Positive
text-indent is a decorative paragraph indent that the user can
reasonably replace with
vertical spacing — negative text-indent is structural (it positions the
list marker)
      and must always apply.

   3. em unit was calibrated against line height, not font size

emSize was computed as getLineHeight() * lineCompression (the full line
advance).
CSS em units are defined relative to the font-size, which corresponds to
the
ascender height — not the line height. Using line height makes every
em-based
margin/indent ~20–30% wider than a browser would render it, and is
especially
noticeable for CSS that uses font-size: small (which we do not
implement).

## Additional Context

Test case
```
.lsl1 { margin-left: 3em; text-indent: -1em; }

<div class="lsl1">• First list item that wraps across lines</div>
<div class="lsl1">• Short item</div>
```
Before: all lines of all items started at 3 em from the left edge
(indent ignored).

After: the bullet marker hangs at 2 em; continuation lines align at 3
em.

<img width="240" alt="before"
src="https://github.com/user-attachments/assets/9dcbf3e0-fcd9-4af8-b451-a90ba4d2fb75"
/>
<img width="240" alt="after"
src="https://github.com/user-attachments/assets/1ffdcf56-a180-4267-9590-c60d7ac44707"
/>

---

### 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**_
This commit is contained in:
jpirnay
2026-03-02 12:02:09 +01:00
committed by GitHub
parent f0a549b680
commit aff93f1dc0
5 changed files with 25 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
lib/Epub/Epub/ParsedText.cpp
View File

@@ -144,9 +144,12 @@ std::vector<size_t> ParsedText::computeLineBreaks(const GfxRenderer& renderer, c
return {};
}
// Calculate first line indent (only for left/justified text without extra paragraph spacing)
// Calculate first line indent (only for left/justified text).
// Positive text-indent (paragraph indent) is suppressed when extraParagraphSpacing is on.
// Negative text-indent (hanging indent, e.g. margin-left:3em; text-indent:-1em) always applies —
// it is structural (positions the bullet/marker), not decorative.
const int firstLineIndent =
blockStyle.textIndent > 0 && !extraParagraphSpacing &&
blockStyle.textIndentDefined && (blockStyle.textIndent < 0 || !extraParagraphSpacing) &&
(blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left)
? blockStyle.textIndent
: 0;
@@ -275,9 +278,12 @@ std::vector<size_t> ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& r
const int pageWidth, const int spaceWidth,
std::vector<uint16_t>& wordWidths,
std::vector<bool>& continuesVec) {
// Calculate first line indent (only for left/justified text without extra paragraph spacing)
// Calculate first line indent (only for left/justified text).
// Positive text-indent (paragraph indent) is suppressed when extraParagraphSpacing is on.
// Negative text-indent (hanging indent, e.g. margin-left:3em; text-indent:-1em) always applies —
// it is structural (positions the bullet/marker), not decorative.
const int firstLineIndent =
blockStyle.textIndent > 0 && !extraParagraphSpacing &&
blockStyle.textIndentDefined && (blockStyle.textIndent < 0 || !extraParagraphSpacing) &&
(blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left)
? blockStyle.textIndent
: 0;
@@ -443,10 +449,13 @@ 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)
// Calculate first line indent (only for left/justified text).
// Positive text-indent (paragraph indent) is suppressed when extraParagraphSpacing is on.
// Negative text-indent (hanging indent, e.g. margin-left:3em; text-indent:-1em) always applies —
// it is structural (positions the bullet/marker), not decorative.
const bool isFirstLine = breakIndex == 0;
const int firstLineIndent =
isFirstLine && blockStyle.textIndent > 0 && !extraParagraphSpacing &&
isFirstLine && blockStyle.textIndentDefined && (blockStyle.textIndent < 0 || !extraParagraphSpacing) &&
(blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left)
? blockStyle.textIndent
: 0;
@@ -485,8 +494,9 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const
? spareSpace / static_cast<int>(actualGapCount)
: 0;
// Calculate initial x position (first line starts at indent for left/justified text)
auto xpos = static_cast<uint16_t>(firstLineIndent);
// Calculate initial x position (first line starts at indent for left/justified text;
// may be negative for hanging indents, e.g. margin-left:3em; text-indent:-1em).
auto xpos = static_cast<int16_t>(firstLineIndent);
if (blockStyle.alignment == CssTextAlign::Right) {
xpos = effectivePageWidth - lineWordWidthSum - totalNaturalGaps;
} else if (blockStyle.alignment == CssTextAlign::Center) {
@@ -495,7 +505,7 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const
// Pre-calculate X positions for words
// Continuation words attach to the previous word with no space before them
std::vector<uint16_t> lineXPos;
std::vector<int16_t> lineXPos;
lineXPos.reserve(lineWordCount);
for (size_t wordIdx = 0; wordIdx < lineWordCount; wordIdx++) {

2
lib/Epub/Epub/Section.cpp
View File

@@ -10,7 +10,7 @@
#include "parsers/ChapterHtmlSlimParser.h"
namespace {
constexpr uint8_t SECTION_FILE_VERSION = 14;
constexpr uint8_t SECTION_FILE_VERSION = 16;
constexpr uint32_t HEADER_SIZE = sizeof(uint8_t) + sizeof(int) + sizeof(float) + sizeof(bool) + sizeof(uint8_t) +
sizeof(uint16_t) + sizeof(uint16_t) + sizeof(uint16_t) + sizeof(bool) + sizeof(bool) +
sizeof(uint32_t);

2
lib/Epub/Epub/blocks/TextBlock.cpp
View File

@@ -74,7 +74,7 @@ bool TextBlock::serialize(FsFile& file) const {
std::unique_ptr<TextBlock> TextBlock::deserialize(FsFile& file) {
uint16_t wc;
std::vector<std::string> words;
std::vector<uint16_t> wordXpos;
std::vector<int16_t> wordXpos;
std::vector<EpdFontFamily::Style> wordStyles;
BlockStyle blockStyle;

4
lib/Epub/Epub/blocks/TextBlock.h
View File

@@ -13,12 +13,12 @@
class TextBlock final : public Block {
private:
std::vector<std::string> words;
std::vector<uint16_t> wordXpos;
std::vector<int16_t> wordXpos;
std::vector<EpdFontFamily::Style> wordStyles;
BlockStyle blockStyle;
public:
explicit TextBlock(std::vector<std::string> words, std::vector<uint16_t> word_xpos,
explicit TextBlock(std::vector<std::string> words, std::vector<int16_t> word_xpos,
std::vector<EpdFontFamily::Style> word_styles, const BlockStyle& blockStyle = BlockStyle())
: words(std::move(words)),
wordXpos(std::move(word_xpos)),

5
lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp
View File

@@ -278,8 +278,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
int displayWidth = 0;
int displayHeight = 0;
const float emSize =
static_cast<float>(self->renderer.getLineHeight(self->fontId)) * self->lineCompression;
const float emSize = static_cast<float>(self->renderer.getFontAscenderSize(self->fontId));
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()) {
@@ -505,7 +504,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
}
}
const float emSize = static_cast<float>(self->renderer.getLineHeight(self->fontId)) * self->lineCompression;
const float emSize = static_cast<float>(self->renderer.getFontAscenderSize(self->fontId));
const auto userAlignmentBlockStyle = BlockStyle::fromCssStyle(
cssStyle, emSize, static_cast<CssTextAlign>(self->paragraphAlignment), self->viewportWidth);