From f88d0a04038320b9bf0906fc861202ff0d303c3f Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Thu, 18 Jun 2026 12:35:21 +1000 Subject: [PATCH 1/5] feat(utf8): extend utf8IsCjkBreakable and add utf8IsGraphemeExtender --- lib/Utf8/Utf8.h | 26 +++++++++++++++++++++++++- test/CMakeLists.txt | 1 + test/utf8/CMakeLists.txt | 12 ++++++++++++ test/utf8/Utf8PredicatesTest.cpp | 28 ++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 test/utf8/CMakeLists.txt create mode 100644 test/utf8/Utf8PredicatesTest.cpp diff --git a/lib/Utf8/Utf8.h b/lib/Utf8/Utf8.h index e7238f8557..7f25bc5668 100644 --- a/lib/Utf8/Utf8.h +++ b/lib/Utf8/Utf8.h @@ -32,7 +32,19 @@ inline bool utf8IsCjkBreakable(const uint32_t cp) { || (cp >= 0xFF01 && cp <= 0xFF60) // Fullwidth Latin / Punctuation || (cp >= 0xFF65 && cp <= 0xFFEF) // Halfwidth Katakana / Hangul || (cp >= 0x20000 && cp <= 0x2A6DF) // CJK Extension B - || (cp >= 0x2A700 && cp <= 0x2B73F); // CJK Extension C + || (cp >= 0x2A700 && cp <= 0x2B73F) // CJK Extension C + || (cp >= 0x1B000 && cp <= 0x1B0FF) // Kana Supplement + || (cp >= 0x1B100 && cp <= 0x1B12F) // Kana Extended-A + || (cp >= 0x2B740 && cp <= 0x2B81F) // CJK Extension D + || (cp >= 0x2B820 && cp <= 0x2CEAF) // CJK Extension E + || (cp >= 0x2CEB0 && cp <= 0x2EBEF) // CJK Extension F + || (cp >= 0x2F800 && cp <= 0x2FA1F) // CJK Compatibility Ideographs Supplement + || (cp >= 0x30000 && cp <= 0x3134F) // CJK Extension G + || (cp >= 0x1AFF0 && cp <= 0x1AFFF) // Kana Extended-B + || (cp >= 0x1B130 && cp <= 0x1B16F) // Small Kana Extension + || (cp >= 0x2EBF0 && cp <= 0x2EE5F) // CJK Extension I + || (cp >= 0x31350 && cp <= 0x323AF) // CJK Extension H + || (cp >= 0x323B0 && cp <= 0x3347F); // CJK Extension J } // Returns true for Unicode combining diacritical marks that should not advance the cursor. @@ -42,3 +54,15 @@ inline bool utf8IsCombiningMark(const uint32_t cp) { || (cp >= 0x20D0 && cp <= 0x20FF) // Combining Diacritical Marks for Symbols || (cp >= 0xFE20 && cp <= 0xFE2F); // Combining Half Marks } + +// Returns true for grapheme extenders that must stay glued to a preceding base +// codepoint in layout (combining marks, kana voicing marks, ideographic tone +// marks, variation selectors). +inline bool utf8IsGraphemeExtender(uint32_t cp) { + return utf8IsCombiningMark(cp) + || (cp >= 0x3099 && cp <= 0x309A) // Combining katakana-hiragana voicing + || (cp >= 0x302A && cp <= 0x302F) // Ideographic tone marks + || (cp >= 0xFF9E && cp <= 0xFF9F) // Halfwidth voicing marks + || (cp >= 0xFE00 && cp <= 0xFE0F) // Variation Selectors VS-1..VS-16 + || (cp >= 0xE0100 && cp <= 0xE01EF); // Variation Selectors Supplement +} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index fb7c535f79..2286f968ef 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -47,3 +47,4 @@ add_subdirectory(cjk_font_parity) add_subdirectory(font_resolver) add_subdirectory(font_boundary) add_subdirectory(parsed_text) +add_subdirectory(utf8) diff --git a/test/utf8/CMakeLists.txt b/test/utf8/CMakeLists.txt new file mode 100644 index 0000000000..105c68381f --- /dev/null +++ b/test/utf8/CMakeLists.txt @@ -0,0 +1,12 @@ +# test/utf8/CMakeLists.txt — dedicated predicate target, isolated from ParsedText / assembler. +add_executable(Utf8PredicatesTest + Utf8PredicatesTest.cpp + ${REPO_ROOT}/lib/Utf8/Utf8.cpp +) +target_include_directories(Utf8PredicatesTest PRIVATE + ${REPO_ROOT} + ${REPO_ROOT}/lib/Utf8 + ${REPO_ROOT}/test +) +target_link_libraries(Utf8PredicatesTest PRIVATE crosspoint_test_common GTest::gtest_main) +gtest_discover_tests(Utf8PredicatesTest) diff --git a/test/utf8/Utf8PredicatesTest.cpp b/test/utf8/Utf8PredicatesTest.cpp new file mode 100644 index 0000000000..c7d60af9ad --- /dev/null +++ b/test/utf8/Utf8PredicatesTest.cpp @@ -0,0 +1,28 @@ +#include + +#include + +TEST(Utf8Predicates, CjkBreakableSupplementaryRanges) { + EXPECT_TRUE(utf8IsCjkBreakable(0x30000)); // CJK Extension G first codepoint + EXPECT_TRUE(utf8IsCjkBreakable(0x1B000)); // Kana Supplement representative + EXPECT_TRUE(utf8IsCjkBreakable(0x1B100)); // Kana Extended-A first codepoint + EXPECT_TRUE(utf8IsCjkBreakable(0x2B740)); // CJK Extension D first codepoint + EXPECT_TRUE(utf8IsCjkBreakable(0x2F800)); // CJK Compatibility Ideographs Supplement + EXPECT_TRUE(utf8IsCjkBreakable(0x31350)); // CJK Extension H first codepoint (Unicode 15.0) + EXPECT_FALSE(utf8IsCjkBreakable(0x0061)); // ASCII 'a' — must not match +} + +TEST(Utf8Predicates, GraphemeExtenderCoversAllOverlapRanges) { + EXPECT_TRUE(utf8IsGraphemeExtender(0x3099)); // Combining hiragana-katakana voicing + EXPECT_TRUE(utf8IsGraphemeExtender(0x309A)); // Combining hiragana-katakana semi-voicing + EXPECT_TRUE(utf8IsGraphemeExtender(0x302A)); // Ideographic tone mark (left) + EXPECT_TRUE(utf8IsGraphemeExtender(0x302F)); // Ideographic tone mark range end + EXPECT_TRUE(utf8IsGraphemeExtender(0xFF9E)); // Halfwidth voicing + EXPECT_TRUE(utf8IsGraphemeExtender(0xFF9F)); // Halfwidth semi-voicing + EXPECT_TRUE(utf8IsGraphemeExtender(0xFE00)); // Variation Selector VS-1 + EXPECT_TRUE(utf8IsGraphemeExtender(0xFE0F)); // Variation Selector VS-16 + EXPECT_TRUE(utf8IsGraphemeExtender(0xE0100)); // Variation Selectors Supplement first + EXPECT_TRUE(utf8IsGraphemeExtender(0xE01EF)); // Variation Selectors Supplement last + EXPECT_FALSE(utf8IsGraphemeExtender(0x0061)); // ASCII 'a' — must not match + EXPECT_FALSE(utf8IsGraphemeExtender(0x65E5)); // 日 — base ideograph, must not match +} From ac6799818bee1f7b913bbeb7f2574e04efdb44df Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Thu, 18 Jun 2026 12:48:34 +1000 Subject: [PATCH 2/5] refactor: model word joins as 3-state WordJoin (space/glue/cjk-break) --- lib/Epub/Epub/ParsedText.cpp | 131 ++++++++++++------ lib/Epub/Epub/ParsedText.h | 13 +- lib/Epub/Epub/WordJoin.h | 14 ++ .../Epub/parsers/ChapterHtmlSlimParser.cpp | 38 ++--- lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h | 2 +- test/parsed_text/ParsedTextLayoutTest.cpp | 4 +- 6 files changed, 128 insertions(+), 74 deletions(-) create mode 100644 lib/Epub/Epub/WordJoin.h diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 219eb74bd2..51ba2a7fb6 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -119,12 +119,46 @@ bool isWordCharacter(uint32_t cp) { return true; } +// Returns true iff word is a single CJK grapheme cluster: one CJK base +// codepoint optionally followed by grapheme extenders (combining marks, +// voicing marks, ideographic tone marks, variation selectors). Parser +// emits these as single tokens via pendingCjkBase, so the focus tokenizer +// must treat the whole cluster as one unit. +static bool isSingleCjkGraphemeCluster(const std::string& s) { + const unsigned char* p = reinterpret_cast(s.c_str()); + uint32_t firstCp = utf8NextCodepoint(&p); + // `firstCp == 0` covers empty string (defensive); reject so the bypass only + // fires for genuine CJK clusters. + if (firstCp == 0 || !utf8IsCjkBreakable(firstCp)) return false; + // Everything after the first codepoint must be a grapheme extender + // (no second base codepoint allowed). + while (*p) { + uint32_t cp = utf8NextCodepoint(&p); + if (cp == 0) return false; + if (!utf8IsGraphemeExtender(cp)) return false; + } + return true; +} + } // namespace void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, const bool underline, - const bool attachToPrevious) { + const WordJoin join) { if (word.empty()) return; + // CJK grapheme-cluster tokens are pre-split by the parser into single + // breakable units. The focus-reading tokenizer would either bold the whole + // token or split base from extender — both wrong. Bypass focus split for any + // single CJK grapheme cluster regardless of `join` (which is a BOUNDARY + // descriptor, not a token-kind tag; the first CJK char of a run carries Space). + if (focusReadingEnabled && isSingleCjkGraphemeCluster(word)) { + words.emplace_back(std::move(word)); + wordStyles.push_back(fontStyle); + wordJoins.push_back(join); + wordIsFocusSuffix.push_back(false); + return; + } + EpdFontFamily::Style baseStyle = fontStyle; if (underline) { baseStyle = static_cast(baseStyle | EpdFontFamily::UNDERLINE); @@ -136,7 +170,7 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, if (!this->focusReadingEnabled || (baseStyle & EpdFontFamily::BOLD) != 0) { words.push_back(std::move(word)); wordStyles.push_back(baseStyle); - wordContinues.push_back(attachToPrevious); + wordJoins.push_back(join); wordIsFocusSuffix.push_back(false); if (wordStartsRtl) { hasRtlWord = true; @@ -165,18 +199,18 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, words.reserve(newCapacity); wordStyles.reserve(newCapacity); - wordContinues.reserve(newCapacity); + wordJoins.reserve(newCapacity); wordIsFocusSuffix.reserve(newCapacity); } // Lambda helper to process and push individual sub-segments of the string // Use std::string_view to avoid heap allocations when slicing - auto processSegment = [&](std::string_view segment, bool isWord, bool attach) { + auto processSegment = [&](std::string_view segment, bool isWord, WordJoin attach) { if (!isWord) { // Punctuation and Numbers stay regular words.emplace_back(segment); wordStyles.push_back(baseStyle); - wordContinues.push_back(attach); + wordJoins.push_back(attach); wordIsFocusSuffix.push_back(false); } else { size_t charCount = 0; @@ -197,7 +231,7 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, // Whole segment is bold - no suffix split needed words.emplace_back(segment); wordStyles.push_back(static_cast(baseStyle | EpdFontFamily::BOLD)); - wordContinues.push_back(attach); + wordJoins.push_back(attach); wordIsFocusSuffix.push_back(false); } else { countPtr = reinterpret_cast(segment.data()); @@ -209,13 +243,13 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, // Bold prefix words.emplace_back(segment.substr(0, splitByteOffset)); wordStyles.push_back(static_cast(baseStyle | EpdFontFamily::BOLD)); - wordContinues.push_back(attach); + wordJoins.push_back(attach); wordIsFocusSuffix.push_back(false); // Regular suffix - marked so extractLine can merge it back into single TextBlock entry words.emplace_back(segment.substr(splitByteOffset)); wordStyles.push_back(baseStyle); - wordContinues.push_back(true); + wordJoins.push_back(WordJoin::Glue); wordIsFocusSuffix.push_back(true); } } @@ -241,9 +275,9 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, size_t segmentLen = currentCpStart - segmentStart; std::string_view segment(reinterpret_cast(segmentStart), segmentLen); - // Only the very first segment inherits the original attachToPrevious flag. - // Every subsequent segment MUST attach=true so it glues seamlessly to the prefix. - processSegment(segment, inWordSegment, isFirstSegment ? attachToPrevious : true); + // Only the very first segment inherits the original join flag. + // Every subsequent segment MUST glue seamlessly to the prefix. + processSegment(segment, inWordSegment, isFirstSegment ? join : WordJoin::Glue); // Setup for the next segment segmentStart = currentCpStart; @@ -255,7 +289,7 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, // Process the final remaining segment size_t segmentLen = end - segmentStart; std::string_view segment(reinterpret_cast(segmentStart), segmentLen); - processSegment(segment, inWordSegment, isFirstSegment ? attachToPrevious : true); + processSegment(segment, inWordSegment, isFirstSegment ? join : WordJoin::Glue); if (wordStartsRtl) { hasRtlWord = true; } @@ -324,14 +358,14 @@ void ParsedText::layoutAndExtractLines(const ITextMetrics& renderer, const int f std::vector lineBreakIndices; if (hyphenationEnabled) { // Use greedy layout that can split words mid-loop when a hyphenated prefix fits. - lineBreakIndices = computeHyphenatedLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues); + lineBreakIndices = computeHyphenatedLineBreaks(renderer, fontId, pageWidth, wordWidths, wordJoins); } else { - lineBreakIndices = computeLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues); + lineBreakIndices = computeLineBreaks(renderer, fontId, pageWidth, wordWidths, wordJoins); } const size_t lineCount = includeLastLine ? lineBreakIndices.size() : lineBreakIndices.size() - 1; for (size_t i = 0; i < lineCount; ++i) { - extractLine(i, pageWidth, wordWidths, wordContinues, lineBreakIndices, processLine, renderer, fontId); + extractLine(i, pageWidth, wordWidths, wordJoins, lineBreakIndices, processLine, renderer, fontId); } // Remove consumed words so size() reflects only remaining words @@ -339,7 +373,7 @@ void ParsedText::layoutAndExtractLines(const ITextMetrics& renderer, const int f const size_t consumed = lineBreakIndices[lineCount - 1]; words.erase(words.begin(), words.begin() + consumed); wordStyles.erase(wordStyles.begin(), wordStyles.begin() + consumed); - wordContinues.erase(wordContinues.begin(), wordContinues.begin() + consumed); + wordJoins.erase(wordJoins.begin(), wordJoins.begin() + consumed); wordIsFocusSuffix.erase(wordIsFocusSuffix.begin(), wordIsFocusSuffix.begin() + consumed); } } @@ -356,7 +390,7 @@ std::vector ParsedText::calculateWordWidths(const ITextMetrics& render } std::vector ParsedText::computeLineBreaks(const ITextMetrics& renderer, const int fontId, const int pageWidth, - std::vector& wordWidths, std::vector& continuesVec) { + std::vector& wordWidths, std::vector& joinsVec) { if (words.empty()) { return {}; } @@ -395,10 +429,10 @@ std::vector ParsedText::computeLineBreaks(const ITextMetrics& renderer, for (size_t j = i; j < totalWordCount; ++j) { // Add space before word j, unless it's the first word on the line or a continuation int gap = 0; - if (j > static_cast(i) && !continuesVec[j]) { + if (j > static_cast(i) && needsSpaceBefore(joinsVec[j])) { gap = renderer.getSpaceAdvance(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]), wordStyles[j - 1]); - } else if (j > static_cast(i) && continuesVec[j]) { + } else if (j > static_cast(i) && !needsSpaceBefore(joinsVec[j])) { // Cross-boundary kerning for continuation words (e.g. nonbreaking spaces, attached punctuation) gap = renderer.getKerning(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]), wordStyles[j - 1]); } @@ -408,8 +442,9 @@ std::vector ParsedText::computeLineBreaks(const ITextMetrics& renderer, break; } - // Cannot break after word j if the next word attaches to it (continuation group) - if (j + 1 < totalWordCount && continuesVec[j + 1]) { + // Cannot break after word j if the next word attaches to it (continuation group). + // Only Glue forbids a break; CjkBreak permits it. + if (j + 1 < totalWordCount && !canBreakBefore(joinsVec[j + 1])) { continue; } @@ -470,7 +505,7 @@ std::vector ParsedText::computeLineBreaks(const ITextMetrics& renderer, // Builds break indices while opportunistically splitting the word that would overflow the current line. std::vector ParsedText::computeHyphenatedLineBreaks(const ITextMetrics& renderer, const int fontId, const int pageWidth, std::vector& wordWidths, - std::vector& continuesVec) { + std::vector& joinsVec) { const int firstLineIndent = resolveFirstLineIndent(true, renderer, fontId); std::vector lineBreakIndices; @@ -488,10 +523,10 @@ std::vector ParsedText::computeHyphenatedLineBreaks(const ITextMetrics& while (currentIndex < wordWidths.size()) { const bool isFirstWord = currentIndex == lineStart; int spacing = 0; - if (!isFirstWord && !continuesVec[currentIndex]) { + if (!isFirstWord && needsSpaceBefore(joinsVec[currentIndex])) { spacing = renderer.getSpaceAdvance(fontId, lastCodepoint(words[currentIndex - 1]), firstCodepoint(words[currentIndex]), wordStyles[currentIndex - 1]); - } else if (!isFirstWord && continuesVec[currentIndex]) { + } else if (!isFirstWord && !needsSpaceBefore(joinsVec[currentIndex])) { // Cross-boundary kerning for continuation words (e.g. nonbreaking spaces, attached punctuation) spacing = renderer.getKerning(fontId, lastCodepoint(words[currentIndex - 1]), firstCodepoint(words[currentIndex]), wordStyles[currentIndex - 1]); @@ -527,7 +562,8 @@ std::vector ParsedText::computeHyphenatedLineBreaks(const ITextMetrics& // Don't break before a continuation word (e.g., orphaned "?" after "question"). // Backtrack to the start of the continuation group so the whole group moves to the next line. - while (currentIndex > lineStart + 1 && currentIndex < wordWidths.size() && continuesVec[currentIndex]) { + // Only Glue forbids a break here; CjkBreak permits it. + while (currentIndex > lineStart + 1 && currentIndex < wordWidths.size() && !canBreakBefore(joinsVec[currentIndex])) { --currentIndex; } @@ -616,8 +652,8 @@ bool ParsedText::hyphenateWordAtIndex(const size_t wordIndex, const int availabl // // This lets the backtracking loop keep the entire prefix group ("200 Quadrat-") on one // line, while "kilometer" moves to the next line. - // wordContinues[wordIndex] is intentionally left unchanged — the prefix keeps its original attachment. - wordContinues.insert(wordContinues.begin() + wordIndex + 1, false); + // wordJoins[wordIndex] is intentionally left unchanged — the prefix keeps its original attachment. + wordJoins.insert(wordJoins.begin() + wordIndex + 1, WordJoin::Space); // Update cached widths to reflect the new prefix/remainder pairing. wordWidths[wordIndex] = static_cast(chosenWidth); @@ -627,7 +663,7 @@ bool ParsedText::hyphenateWordAtIndex(const size_t wordIndex, const int availabl } void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const std::vector& wordWidths, - const std::vector& continuesVec, const std::vector& lineBreakIndices, + const std::vector& joinsVec, const std::vector& lineBreakIndices, const std::function)>& processLine, const ITextMetrics& renderer, const int fontId) { const size_t lineBreak = lineBreakIndices[breakIndex]; @@ -660,11 +696,11 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const for (size_t wordIdx = 0; wordIdx < lineWordCount; wordIdx++) { lineWordWidthSum += wordWidths[lastBreakAt + wordIdx]; // Count gaps: each word after the first creates a gap, unless it's a continuation - if (wordIdx > 0 && !continuesVec[lastBreakAt + wordIdx]) { + if (wordIdx > 0 && needsSpaceBefore(joinsVec[lastBreakAt + wordIdx])) { actualGapCount++; totalNaturalGaps += renderer.getSpaceAdvance(fontId, lastCodepoint(lineWords[wordIdx - 1]), firstCodepoint(lineWords[wordIdx]), lineWordStyles[wordIdx - 1]); - } else if (wordIdx > 0 && continuesVec[lastBreakAt + wordIdx]) { + } else if (wordIdx > 0 && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx])) { // Non-breaking space tokens (" " with continues=true) are visible, stretchable spaces — // count them as justifiable gaps so justifyExtra is distributed to them too. if (lineWords[wordIdx] == " ") { @@ -708,12 +744,12 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const reorderedWordsScratch.clear(); reorderedStylesScratch.clear(); reorderedWidthsScratch.clear(); - reorderedContinuesScratch.clear(); + reorderedJoinsScratch.clear(); reorderedFocusSuffixScratch.clear(); reorderedWordsScratch.reserve(visualOrderScratch.size()); reorderedStylesScratch.reserve(visualOrderScratch.size()); reorderedWidthsScratch.reserve(visualOrderScratch.size()); - reorderedContinuesScratch.reserve(visualOrderScratch.size()); + reorderedJoinsScratch.reserve(visualOrderScratch.size()); reorderedFocusSuffixScratch.reserve(visualOrderScratch.size()); for (size_t i = 0; i < visualOrderScratch.size(); ++i) { @@ -726,20 +762,23 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const // Continuation means "no break/gap between two adjacent logical tokens". // After visual reordering (common in RTL), an adjacent logical pair can appear // as either (prev -> curr) or (curr -> prev) in visual order; preserve both. - bool continues = false; + // Carry the full WordJoin from the adjacent source index rather than collapsing + // to a bool, so CjkBreak survives reordering. The Space default for non-adjacent + // reorder pairs is unreachable in pure-Japanese and is explicitly out of scope. + WordJoin join = WordJoin::Space; if (i > 0) { const size_t prevSrc = visualOrderScratch[i - 1]; const size_t currSrc = src; const bool forwardAdjacent = currSrc == prevSrc + 1; const bool reverseAdjacent = prevSrc == currSrc + 1; - if (forwardAdjacent && continuesVec[lastBreakAt + currSrc]) { - continues = true; - } else if (reverseAdjacent && continuesVec[lastBreakAt + prevSrc]) { - continues = true; + if (forwardAdjacent) { + join = joinsVec[lastBreakAt + currSrc]; + } else if (reverseAdjacent) { + join = joinsVec[lastBreakAt + prevSrc]; } } - reorderedContinuesScratch.push_back(continues); + reorderedJoinsScratch.push_back(join); } int reorderedWordWidthSum = 0; @@ -747,12 +786,12 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const int reorderedNaturalGaps = 0; for (size_t wordIdx = 0; wordIdx < reorderedWidthsScratch.size(); wordIdx++) { reorderedWordWidthSum += reorderedWidthsScratch[wordIdx]; - if (wordIdx > 0 && !reorderedContinuesScratch[wordIdx]) { + if (wordIdx > 0 && needsSpaceBefore(reorderedJoinsScratch[wordIdx])) { reorderedGapCount++; reorderedNaturalGaps += renderer.getSpaceAdvance(fontId, lastCodepoint(reorderedWordsScratch[wordIdx - 1]), firstCodepoint(reorderedWordsScratch[wordIdx]), reorderedStylesScratch[wordIdx - 1]); - } else if (wordIdx > 0 && reorderedContinuesScratch[wordIdx]) { + } else if (wordIdx > 0 && !needsSpaceBefore(reorderedJoinsScratch[wordIdx])) { if (reorderedWordsScratch[wordIdx] == " ") { reorderedGapCount++; } @@ -794,12 +833,12 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const xpos += reorderedWidthsScratch[wordIdx]; const bool nextIsContinuation = - wordIdx + 1 < reorderedWidthsScratch.size() && reorderedContinuesScratch[wordIdx + 1]; + wordIdx + 1 < reorderedWidthsScratch.size() && !needsSpaceBefore(reorderedJoinsScratch[wordIdx + 1]); if (nextIsContinuation) { int advance = renderer.getKerning(fontId, lastCodepoint(reorderedWordsScratch[wordIdx]), firstCodepoint(reorderedWordsScratch[wordIdx + 1]), reorderedStylesScratch[wordIdx]); - if (reorderedWordsScratch[wordIdx] == " " && reorderedContinuesScratch[wordIdx] && + if (reorderedWordsScratch[wordIdx] == " " && !needsSpaceBefore(reorderedJoinsScratch[wordIdx]) && effectiveAlignment == CssTextAlign::Justify && !isLastLine) { advance += reorderedJustifyExtra; } @@ -834,12 +873,12 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const xpos -= wordWidths[lastBreakAt + wordIdx]; lineXPos.push_back(static_cast(xpos)); - const bool nextIsContinuation = wordIdx + 1 < lineWordCount && continuesVec[lastBreakAt + wordIdx + 1]; + const bool nextIsContinuation = wordIdx + 1 < lineWordCount && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx + 1]); if (nextIsContinuation) { // Cross-boundary kerning for continuation words int advance = renderer.getKerning(fontId, lastCodepoint(lineWords[wordIdx]), firstCodepoint(lineWords[wordIdx + 1]), lineWordStyles[wordIdx]); - if (lineWords[wordIdx] == " " && continuesVec[lastBreakAt + wordIdx] && + if (lineWords[wordIdx] == " " && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx]) && effectiveAlignment == CssTextAlign::Justify && !isLastLine) { advance += justifyExtra; } @@ -868,12 +907,12 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const for (size_t wordIdx = 0; wordIdx < lineWordCount; wordIdx++) { lineXPos.push_back(static_cast(xpos)); - const bool nextIsContinuation = wordIdx + 1 < lineWordCount && continuesVec[lastBreakAt + wordIdx + 1]; + const bool nextIsContinuation = wordIdx + 1 < lineWordCount && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx + 1]); if (nextIsContinuation) { int advance = wordWidths[lastBreakAt + wordIdx]; advance += renderer.getKerning(fontId, lastCodepoint(lineWords[wordIdx]), firstCodepoint(lineWords[wordIdx + 1]), lineWordStyles[wordIdx]); - if (lineWords[wordIdx] == " " && continuesVec[lastBreakAt + wordIdx] && + if (lineWords[wordIdx] == " " && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx]) && effectiveAlignment == CssTextAlign::Justify && !isLastLine) { advance += justifyExtra; } diff --git a/lib/Epub/Epub/ParsedText.h b/lib/Epub/Epub/ParsedText.h index 5f7f854c09..4fac612590 100644 --- a/lib/Epub/Epub/ParsedText.h +++ b/lib/Epub/Epub/ParsedText.h @@ -7,6 +7,7 @@ #include #include +#include "WordJoin.h" #include "blocks/BlockStyle.h" #include "blocks/TextBlock.h" @@ -15,7 +16,7 @@ class ITextMetrics; class ParsedText { std::vector words; std::vector wordStyles; - std::vector wordContinues; // true = word attaches to previous (no space before it) + std::vector wordJoins; // how each word joins the previous (space / glue / cjk-break) std::vector wordIsFocusSuffix; // true = token is the regular tail of a focus bold-prefix split BlockStyle blockStyle; bool extraParagraphSpacing; @@ -26,19 +27,19 @@ class ParsedText { std::vector reorderedWordsScratch; std::vector reorderedStylesScratch; std::vector reorderedWidthsScratch; - std::vector reorderedContinuesScratch; + std::vector reorderedJoinsScratch; std::vector reorderedFocusSuffixScratch; std::vector visualOrderScratch; int resolveFirstLineIndent(bool isFirstLine, const ITextMetrics& renderer, int fontId) const; std::vector computeLineBreaks(const ITextMetrics& renderer, int fontId, int pageWidth, - std::vector& wordWidths, std::vector& continuesVec); + std::vector& wordWidths, std::vector& joinsVec); std::vector computeHyphenatedLineBreaks(const ITextMetrics& renderer, int fontId, int pageWidth, - std::vector& wordWidths, std::vector& continuesVec); + std::vector& wordWidths, std::vector& joinsVec); bool hyphenateWordAtIndex(size_t wordIndex, int availableWidth, const ITextMetrics& renderer, int fontId, std::vector& wordWidths, bool allowFallbackBreaks); void extractLine(size_t breakIndex, int pageWidth, const std::vector& wordWidths, - const std::vector& continuesVec, const std::vector& lineBreakIndices, + const std::vector& joinsVec, const std::vector& lineBreakIndices, const std::function)>& processLine, const ITextMetrics& renderer, int fontId); std::vector calculateWordWidths(const ITextMetrics& renderer, int fontId); @@ -54,7 +55,7 @@ class ParsedText { hasRtlWord(false) {} ~ParsedText() = default; - void addWord(std::string word, EpdFontFamily::Style fontStyle, bool underline = false, bool attachToPrevious = false); + void addWord(std::string word, EpdFontFamily::Style fontStyle, bool underline = false, WordJoin join = WordJoin::Space); void setBlockStyle(const BlockStyle& blockStyle) { this->blockStyle = blockStyle; } BlockStyle& getBlockStyle() { return blockStyle; } size_t size() const { return words.size(); } diff --git a/lib/Epub/Epub/WordJoin.h b/lib/Epub/Epub/WordJoin.h new file mode 100644 index 0000000000..3f0d996108 --- /dev/null +++ b/lib/Epub/Epub/WordJoin.h @@ -0,0 +1,14 @@ +// lib/Epub/Epub/WordJoin.h +#pragma once + +#include + +// How the current word joins the previous one in a line: +// Space — a space precedes this word; the layout may break here. +// Glue — no space and no break (e.g. NBSP, hyphenation remainder bridge). +// CjkBreak — no space, but the layout MAY break here (the only state CJK needs). +// Predicates are free `constexpr` so call sites read naturally. +enum class WordJoin : uint8_t { Space, Glue, CjkBreak }; + +constexpr bool needsSpaceBefore(WordJoin j) { return j == WordJoin::Space; } +constexpr bool canBreakBefore(WordJoin j) { return j != WordJoin::Glue; } diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index c4b0ccd59e..ba80f9e1d5 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -185,14 +185,14 @@ void ChapterHtmlSlimParser::flushPartWordBuffer() { // flush the buffer partWordBuffer[partWordBufferIndex] = '\0'; - currentTextBlock->addWord(partWordBuffer, fontStyle, false, nextWordContinues); + currentTextBlock->addWord(partWordBuffer, fontStyle, false, nextJoin); partWordBufferIndex = 0; - nextWordContinues = false; + nextJoin = WordJoin::Space; } // start a new text block if needed void ChapterHtmlSlimParser::startNewTextBlock(const BlockStyle& blockStyle) { - nextWordContinues = false; // New block = new paragraph, no continuation + nextJoin = WordJoin::Space; // New block = new paragraph, no continuation if (currentTextBlock) { // already have a text block running and it is empty - just reuse it if (currentTextBlock->isEmpty()) { @@ -420,7 +420,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); } - self->nextWordContinues = false; + self->nextJoin = WordJoin::Space; self->inlineStyleStack.pop_back(); self->updateEffectiveInlineStyle(); @@ -743,7 +743,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Flush buffer before style change if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } self->insideFootnoteLink = true; self->footnoteLinkDepth = self->depth; @@ -827,7 +827,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Flush buffer before style change so preceding text gets current style if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } self->underlineUntilDepth = std::min(self->underlineUntilDepth, self->depth); // Push inline style entry for underline tag @@ -850,7 +850,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Flush buffer before style change so preceding text gets current style if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } self->boldUntilDepth = std::min(self->boldUntilDepth, self->depth); // Push inline style entry for bold tag @@ -873,7 +873,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Flush buffer before style change so preceding text gets current style if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } self->italicUntilDepth = std::min(self->italicUntilDepth, self->depth); // Push inline style entry for italic tag @@ -895,7 +895,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* } else if (strcmp(name, "sup") == 0 || strcmp(name, "sub") == 0) { if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } StyleStackEntry entry; entry.depth = self->depth; @@ -915,7 +915,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Flush buffer before style change so preceding text gets current style if (self->partWordBufferIndex > 0) { self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } StyleStackEntry entry; entry.depth = self->depth; // Track depth for matching pop @@ -998,7 +998,7 @@ void XMLCALL ChapterHtmlSlimParser::characterData(void* userData, const XML_Char self->flushPartWordBuffer(); } // Whitespace is a real word boundary — reset continuation state - self->nextWordContinues = false; + self->nextJoin = WordJoin::Space; // Skip the whitespace char continue; } @@ -1029,10 +1029,10 @@ void XMLCALL ChapterHtmlSlimParser::characterData(void* userData, const XML_Char self->partWordBuffer[0] = ' '; self->partWordBuffer[1] = '\0'; self->partWordBufferIndex = 1; - self->nextWordContinues = true; // Attach space to previous word (no break). + self->nextJoin = WordJoin::Glue; // Attach space to previous word (no break). self->flushPartWordBuffer(); - self->nextWordContinues = true; // Next real word attaches to this space (no break). + self->nextJoin = WordJoin::Glue; // Next real word attaches to this space (no break). i++; // Skip the second byte (0xA0) continue; @@ -1048,10 +1048,10 @@ void XMLCALL ChapterHtmlSlimParser::characterData(void* userData, const XML_Char self->partWordBuffer[0] = ' '; self->partWordBuffer[1] = '\0'; self->partWordBufferIndex = 1; - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; self->flushPartWordBuffer(); - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; i += 2; // Skip the remaining two bytes (0x80 0xAF) continue; @@ -1170,7 +1170,7 @@ void XMLCALL ChapterHtmlSlimParser::endElement(void* userData, const XML_Char* n self->flushPartWordBuffer(); // If closing an inline element, the next word fragment continues the same visual word if (isInlineTag) { - self->nextWordContinues = true; + self->nextJoin = WordJoin::Glue; } } } @@ -1198,18 +1198,18 @@ void XMLCALL ChapterHtmlSlimParser::endElement(void* userData, const XML_Char* n } if (self->tableDepth == 1 && (strcmp(name, "td") == 0 || strcmp(name, "th") == 0)) { - self->nextWordContinues = false; + self->nextJoin = WordJoin::Space; } if (self->tableDepth == 1 && (strcmp(name, "tr") == 0)) { - self->nextWordContinues = false; + self->nextJoin = WordJoin::Space; } if (self->tableDepth == 1 && strcmp(name, "table") == 0) { self->tableDepth -= 1; self->tableRowIndex = 0; self->tableColIndex = 0; - self->nextWordContinues = false; + self->nextJoin = WordJoin::Space; } // Leaving bold tag diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h index d2fd777806..cc9874ab04 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h @@ -36,7 +36,7 @@ class ChapterHtmlSlimParser { // leave one char at end for null pointer char partWordBuffer[MAX_WORD_SIZE + 1] = {}; int partWordBufferIndex = 0; - bool nextWordContinues = false; // true when next flushed word attaches to previous (inline element boundary) + WordJoin nextJoin = WordJoin::Space; // how the next flushed word joins the previous (inline element boundary) std::unique_ptr currentTextBlock = nullptr; std::unique_ptr currentPage = nullptr; int16_t currentPageNextY = 0; diff --git a/test/parsed_text/ParsedTextLayoutTest.cpp b/test/parsed_text/ParsedTextLayoutTest.cpp index 61285c8c90..c3297bd499 100644 --- a/test/parsed_text/ParsedTextLayoutTest.cpp +++ b/test/parsed_text/ParsedTextLayoutTest.cpp @@ -94,8 +94,8 @@ TEST(ParsedTextLayout, NoBreakSpaceGlueKeepsGroupAtomic) { leftAligned()); text.addWord("x", EpdFontFamily::REGULAR); // join=Space (default) text.addWord("200", EpdFontFamily::REGULAR); // join=Space — breakable before - text.addWord(" ", EpdFontFamily::REGULAR, /*underline=*/false, /*attachToPrevious=*/true); // NBSP — Glue - text.addWord("km", EpdFontFamily::REGULAR, /*underline=*/false, /*attachToPrevious=*/true); // Glue + text.addWord(" ", EpdFontFamily::REGULAR, /*underline=*/false, WordJoin::Glue); // NBSP — Glue + text.addWord("km", EpdFontFamily::REGULAR, /*underline=*/false, WordJoin::Glue); // Glue auto lines = linesOf(text, 6 * kCell); // Glue forbids breaking between "200", " ", and "km" — the trio must move as one // unit. The optimal layout puts "x" on line 0 and "200 km" on line 1. From 510ebb2c310fe510bab5145ed8bf0170dd42184f Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Thu, 18 Jun 2026 13:04:08 +1000 Subject: [PATCH 3/5] test: Japanese text wraps at viewport width, not at fixed chunk --- test/parsed_text/ParsedTextLayoutTest.cpp | 152 ++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/test/parsed_text/ParsedTextLayoutTest.cpp b/test/parsed_text/ParsedTextLayoutTest.cpp index c3297bd499..ddce228f13 100644 --- a/test/parsed_text/ParsedTextLayoutTest.cpp +++ b/test/parsed_text/ParsedTextLayoutTest.cpp @@ -171,3 +171,155 @@ TEST(ParsedTextLayout, BidiRtlRunLaysOut) { EXPECT_EQ(xpos[1], static_cast(40)); EXPECT_EQ(xpos[2], static_cast(70)); } + +// ───────────────────────────────────────────────────────────────────────── +// Phase 2 (PR E): Japanese / CJK layout characterization. +// ───────────────────────────────────────────────────────────────────────── + +// Count UTF-8 codepoints in a string (used to assert CJK line lengths). +static size_t countCodepoints(const std::string& s) { + size_t count = 0; + const auto* p = reinterpret_cast(s.c_str()); + while (utf8NextCodepoint(&p) != 0) ++count; + return count; +} + +// Add a CJK run: each char its own breakable, no-space word (mirrors the parser). +static void addCjkRun(ParsedText& text, int count, const char* ch) { + for (int i = 0; i < count; ++i) { + text.addWord(ch, EpdFontFamily::REGULAR, /*underline=*/false, + i == 0 ? WordJoin::Space : WordJoin::CjkBreak); + } +} + +TEST(ParsedTextLayout, JapaneseWrapsAtViewportNotAtChunk) { + ParsedText text(false, false, false, leftAligned()); + addCjkRun(text, 25, "\xE8\x87\xAA"); // 25 × U+81EA (自); viewport 10 → 10/10/5 + auto lines = linesOf(text, 10 * kCell); + ASSERT_EQ(lines.size(), 3u); + EXPECT_EQ(countCodepoints(lines[0]), 10u); + EXPECT_EQ(countCodepoints(lines[1]), 10u); + EXPECT_EQ(countCodepoints(lines[2]), 5u); // only the LAST line is short +} + +// "English 日本語" — the space after Latin must be preserved as Space before the +// first CJK char; only subsequent CJK chars use CjkBreak. +TEST(ParsedTextLayout, ParserSequenceEnglishSpaceCjkPreservesSpace) { + ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, + /*focusReadingEnabled=*/false, leftAligned()); + text.addWord("English", EpdFontFamily::REGULAR); // join=Space (default) + text.addWord("\xE6\x97\xA5", EpdFontFamily::REGULAR, false, WordJoin::Space); // 日 — first CJK after space + text.addWord("\xE6\x9C\xAC", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 本 + text.addWord("\xE8\xAA\x9E", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 語 + std::shared_ptr firstLine; + FakeTextMetrics metrics(kCell); + text.layoutAndExtractLines(metrics, kFontId, static_cast(100 * kCell), + [&](std::shared_ptr b) { if (!firstLine) firstLine = b; }); + ASSERT_TRUE(firstLine); + const auto& xpos = firstLine->getWordXpos(); + ASSERT_GE(xpos.size(), 2u); + // "English" is 7 codepoints × kCell = 70px wide. The 日 must sit kCell to the right + // of English's tail (Space adds 1 cell). Under CjkBreak the delta would be ≈ 0 + // (kerning only). Pin the Space-induced gap explicitly: + EXPECT_EQ(xpos[1] - xpos[0], 7 * kCell + kCell) + << "first CJK char must inherit Space gap from preceding Latin word, not CjkBreak"; +} + +// "日本語English" — Latin after a CJK run inherits CjkBreak from the last CJK emit, +// so it attaches without a space (intentional — Japanese typography norm). +TEST(ParsedTextLayout, ParserSequenceCjkLatinNoSpace) { + ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, + /*focusReadingEnabled=*/false, leftAligned()); + text.addWord("\xE6\x97\xA5", EpdFontFamily::REGULAR, false, WordJoin::Space); // 日 (paragraph start) + text.addWord("\xE6\x9C\xAC", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 本 + text.addWord("\xE8\xAA\x9E", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 語 + text.addWord("English", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // Latin after CJK + std::shared_ptr firstLine; + FakeTextMetrics metrics(kCell); + text.layoutAndExtractLines(metrics, kFontId, static_cast(100 * kCell), + [&](std::shared_ptr b) { if (!firstLine) firstLine = b; }); + ASSERT_TRUE(firstLine); + const auto& xpos = firstLine->getWordXpos(); + ASSERT_GE(xpos.size(), 4u); + // Each CJK char is kCell wide. "English" must sit flush against 語's tail (CjkBreak + // adds no space): xpos[3] - xpos[2] should equal kCell (just 語's width), not 2*kCell. + EXPECT_EQ(xpos[3] - xpos[2], kCell) + << "Latin word after CJK run must inherit CjkBreak (no space gap)"; +} + +// Layout-layer characterization for a CJK-Latin-CJK sequence (does NOT exercise the +// parser path — the parser's caller-append duty is verified by PR F's assembler unit +// tests + device QA). Guards the focus / xpos / cluster-token side of the contract. +TEST(ParsedTextLayout, JapaneseRunWithSingleLatinCharNotDropped) { + ParsedText text(false, false, false, leftAligned()); + text.addWord("\xE6\x97\xA5", EpdFontFamily::REGULAR, false, WordJoin::Space); // 日 + text.addWord("a", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // Latin 1 char + text.addWord("\xE6\x9C\xAC", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 本 + auto lines = linesOf(text, 100 * kCell); + ASSERT_EQ(lines.size(), 1u); + EXPECT_NE(lines[0].find("a"), std::string::npos); // 'a' must survive + EXPECT_EQ(countCodepoints(lines[0]), 3u); +} + +// NBSP glue around a CJK token: the CJK char inherits Glue from the NBSP, not +// CjkBreak, so the layout must not break between them. Leading breakable "x" forces a +// valid break BETWEEN "x" and "200", not inside the NBSP group. +TEST(ParsedTextLayout, ParserSequenceNbspGluesCjk) { + ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, + /*focusReadingEnabled=*/false, leftAligned()); + text.addWord("x", EpdFontFamily::REGULAR); // join=Space + text.addWord("200", EpdFontFamily::REGULAR); // join=Space + text.addWord(" ", EpdFontFamily::REGULAR, false, WordJoin::Glue); // NBSP word + text.addWord("\xE8\xAA\x9E", EpdFontFamily::REGULAR, false, WordJoin::Glue); // 語 inherits Glue + auto lines = linesOf(text, 6 * kCell); + ASSERT_EQ(lines.size(), 2u); + EXPECT_EQ(lines[0], "x"); + EXPECT_EQ(lines[1], "200 語"); // NBSP word " " is preserved as an explicit token in the join +} + +// Focus reading must NOT bold CJK 1-char tokens (Task 2.1 focus-bypass). The only +// observable difference is whether the CJK tokens carry the BOLD style flag, so this +// asserts on getWordStyles(), not on token count. +TEST(ParsedTextLayout, FocusReadingDoesNotBoldCjkTokens) { + ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, + /*focusReadingEnabled=*/true, leftAligned()); + addCjkRun(text, 25, "\xE8\x87\xAA"); // 25 × 自 with CjkBreak joins (first is Space) + std::vector styles; + text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, + [&](std::shared_ptr block) { + for (const auto& s : block->getWordStyles()) styles.push_back(s); + }); + ASSERT_EQ(styles.size(), 25u); // sanity: every CJK char emitted exactly one style entry + for (size_t i = 0; i < styles.size(); ++i) { + EXPECT_EQ(static_cast(styles[i]) & static_cast(EpdFontFamily::BOLD), 0) + << "CJK token " << i << " unexpectedly has BOLD set — Step 2.5 bypass did not fire"; + } +} + +// Multi-codepoint NFD cluster bypass: the parser emits 2-codepoint tokens like +// か (U+304B) + U+3099. A single-codepoint gate would split base from extender at +// render time. Assert the grapheme-cluster gate accepts them too. +TEST(ParsedTextLayout, FocusReadingDoesNotBoldCjkNfdClusters) { + ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, + /*focusReadingEnabled=*/true, leftAligned()); + // Build 25 NFD clusters: か + U+3099 = "\xE3\x81\x8B\xE3\x82\x99" (6 bytes, 2 codepoints). + const std::string kCluster = "\xE3\x81\x8B\xE3\x82\x99"; + for (int i = 0; i < 25; ++i) { + text.addWord(kCluster, EpdFontFamily::REGULAR, /*underline=*/false, + i == 0 ? WordJoin::Space : WordJoin::CjkBreak); + } + std::vector styles; + text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, + [&](std::shared_ptr block) { + for (const auto& s : block->getWordStyles()) styles.push_back(s); + }); + ASSERT_EQ(styles.size(), 25u); + for (size_t i = 0; i < styles.size(); ++i) { + EXPECT_EQ(static_cast(styles[i]) & static_cast(EpdFontFamily::BOLD), 0) + << "NFD cluster " << i << " unexpectedly has BOLD set — isSingleCjkGraphemeCluster missed it"; + } +} + +// Note: the empty-word focus test is deliberately omitted. ParsedText::addWord +// short-circuits with `if (word.empty()) return;` BEFORE the focus-bypass helper, +// so empty words never reach isSingleCjkGraphemeCluster — such a test would be vacuous. From 47ae9da0606c82ef5b1817e4dcb9e3b755984f32 Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Thu, 18 Jun 2026 13:20:55 +1000 Subject: [PATCH 4/5] fix(epub): preserve underline on CJK focus-bypass and guard embedded NUL --- lib/Epub/Epub/ParsedText.cpp | 26 ++++++++++++----------- test/parsed_text/ParsedTextLayoutTest.cpp | 18 ++++++++++++++++ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 51ba2a7fb6..02bb253c37 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -125,17 +125,18 @@ bool isWordCharacter(uint32_t cp) { // emits these as single tokens via pendingCjkBase, so the focus tokenizer // must treat the whole cluster as one unit. static bool isSingleCjkGraphemeCluster(const std::string& s) { - const unsigned char* p = reinterpret_cast(s.c_str()); + const unsigned char* p = reinterpret_cast(s.data()); + const unsigned char* end = p + s.size(); uint32_t firstCp = utf8NextCodepoint(&p); // `firstCp == 0` covers empty string (defensive); reject so the bypass only // fires for genuine CJK clusters. if (firstCp == 0 || !utf8IsCjkBreakable(firstCp)) return false; - // Everything after the first codepoint must be a grapheme extender - // (no second base codepoint allowed). - while (*p) { + // Everything after the first codepoint must be a grapheme extender. Bound by + // `end` (not a NUL terminator) so an embedded NUL followed by a non-extender + // is correctly rejected rather than silently truncating validation. + while (p < end) { uint32_t cp = utf8NextCodepoint(&p); - if (cp == 0) return false; - if (!utf8IsGraphemeExtender(cp)) return false; + if (cp == 0 || !utf8IsGraphemeExtender(cp)) return false; } return true; } @@ -146,23 +147,24 @@ void ParsedText::addWord(std::string word, const EpdFontFamily::Style fontStyle, const WordJoin join) { if (word.empty()) return; + EpdFontFamily::Style baseStyle = fontStyle; + if (underline) { + baseStyle = static_cast(baseStyle | EpdFontFamily::UNDERLINE); + } + // CJK grapheme-cluster tokens are pre-split by the parser into single // breakable units. The focus-reading tokenizer would either bold the whole // token or split base from extender — both wrong. Bypass focus split for any // single CJK grapheme cluster regardless of `join` (which is a BOUNDARY // descriptor, not a token-kind tag; the first CJK char of a run carries Space). + // Push baseStyle (not raw fontStyle) so an underlined CJK char keeps its underline. if (focusReadingEnabled && isSingleCjkGraphemeCluster(word)) { words.emplace_back(std::move(word)); - wordStyles.push_back(fontStyle); + wordStyles.push_back(baseStyle); wordJoins.push_back(join); wordIsFocusSuffix.push_back(false); return; } - - EpdFontFamily::Style baseStyle = fontStyle; - if (underline) { - baseStyle = static_cast(baseStyle | EpdFontFamily::UNDERLINE); - } const bool wordStartsRtl = !hasRtlWord && mayContainRtlBytes(word.c_str()) && BidiUtils::startsWithRtl(word.c_str(), RTL_PER_WORD_PROBE_DEPTH); diff --git a/test/parsed_text/ParsedTextLayoutTest.cpp b/test/parsed_text/ParsedTextLayoutTest.cpp index ddce228f13..8aa17d0c2d 100644 --- a/test/parsed_text/ParsedTextLayoutTest.cpp +++ b/test/parsed_text/ParsedTextLayoutTest.cpp @@ -323,3 +323,21 @@ TEST(ParsedTextLayout, FocusReadingDoesNotBoldCjkNfdClusters) { // Note: the empty-word focus test is deliberately omitted. ParsedText::addWord // short-circuits with `if (word.empty()) return;` BEFORE the focus-bypass helper, // so empty words never reach isSingleCjkGraphemeCluster — such a test would be vacuous. + +// Regression (Codex review): the focus-bypass must PRESERVE underline on a single +// CJK cluster. It previously pushed the raw fontStyle, dropping the UNDERLINE bit. +TEST(ParsedTextLayout, FocusReadingPreservesUnderlineOnCjkCluster) { + ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, + /*focusReadingEnabled=*/true, leftAligned()); + text.addWord("\xE6\x97\xA5", EpdFontFamily::REGULAR, /*underline=*/true, WordJoin::Space); // 日, underlined + std::vector styles; + text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, + [&](std::shared_ptr block) { + for (const auto& s : block->getWordStyles()) styles.push_back(s); + }); + ASSERT_EQ(styles.size(), 1u); + EXPECT_NE(static_cast(styles[0]) & static_cast(EpdFontFamily::UNDERLINE), 0) + << "underline must survive the CJK focus-bypass"; + EXPECT_EQ(static_cast(styles[0]) & static_cast(EpdFontFamily::BOLD), 0) + << "focus-bypass must still skip focus-bolding of a single CJK cluster"; +} From f2b6c5b23e4b90c9dd5f5a0722154cf34edfb336 Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Thu, 18 Jun 2026 13:42:40 +1000 Subject: [PATCH 5/5] style: apply clang-format-21 formatting --- lib/Epub/Epub/ParsedText.cpp | 9 ++-- lib/Epub/Epub/ParsedText.h | 3 +- lib/Utf8/Utf8.h | 11 +++-- test/parsed_text/ParsedTextLayoutTest.cpp | 54 +++++++++++------------ test/utf8/Utf8PredicatesTest.cpp | 17 ++++--- 5 files changed, 46 insertions(+), 48 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 02bb253c37..50662e0683 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -565,7 +565,8 @@ std::vector ParsedText::computeHyphenatedLineBreaks(const ITextMetrics& // Don't break before a continuation word (e.g., orphaned "?" after "question"). // Backtrack to the start of the continuation group so the whole group moves to the next line. // Only Glue forbids a break here; CjkBreak permits it. - while (currentIndex > lineStart + 1 && currentIndex < wordWidths.size() && !canBreakBefore(joinsVec[currentIndex])) { + while (currentIndex > lineStart + 1 && currentIndex < wordWidths.size() && + !canBreakBefore(joinsVec[currentIndex])) { --currentIndex; } @@ -875,7 +876,8 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const xpos -= wordWidths[lastBreakAt + wordIdx]; lineXPos.push_back(static_cast(xpos)); - const bool nextIsContinuation = wordIdx + 1 < lineWordCount && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx + 1]); + const bool nextIsContinuation = + wordIdx + 1 < lineWordCount && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx + 1]); if (nextIsContinuation) { // Cross-boundary kerning for continuation words int advance = renderer.getKerning(fontId, lastCodepoint(lineWords[wordIdx]), @@ -909,7 +911,8 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const for (size_t wordIdx = 0; wordIdx < lineWordCount; wordIdx++) { lineXPos.push_back(static_cast(xpos)); - const bool nextIsContinuation = wordIdx + 1 < lineWordCount && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx + 1]); + const bool nextIsContinuation = + wordIdx + 1 < lineWordCount && !needsSpaceBefore(joinsVec[lastBreakAt + wordIdx + 1]); if (nextIsContinuation) { int advance = wordWidths[lastBreakAt + wordIdx]; advance += renderer.getKerning(fontId, lastCodepoint(lineWords[wordIdx]), diff --git a/lib/Epub/Epub/ParsedText.h b/lib/Epub/Epub/ParsedText.h index 4fac612590..aa37fed244 100644 --- a/lib/Epub/Epub/ParsedText.h +++ b/lib/Epub/Epub/ParsedText.h @@ -55,7 +55,8 @@ class ParsedText { hasRtlWord(false) {} ~ParsedText() = default; - void addWord(std::string word, EpdFontFamily::Style fontStyle, bool underline = false, WordJoin join = WordJoin::Space); + void addWord(std::string word, EpdFontFamily::Style fontStyle, bool underline = false, + WordJoin join = WordJoin::Space); void setBlockStyle(const BlockStyle& blockStyle) { this->blockStyle = blockStyle; } BlockStyle& getBlockStyle() { return blockStyle; } size_t size() const { return words.size(); } diff --git a/lib/Utf8/Utf8.h b/lib/Utf8/Utf8.h index 7f25bc5668..5225935cb9 100644 --- a/lib/Utf8/Utf8.h +++ b/lib/Utf8/Utf8.h @@ -59,10 +59,9 @@ inline bool utf8IsCombiningMark(const uint32_t cp) { // codepoint in layout (combining marks, kana voicing marks, ideographic tone // marks, variation selectors). inline bool utf8IsGraphemeExtender(uint32_t cp) { - return utf8IsCombiningMark(cp) - || (cp >= 0x3099 && cp <= 0x309A) // Combining katakana-hiragana voicing - || (cp >= 0x302A && cp <= 0x302F) // Ideographic tone marks - || (cp >= 0xFF9E && cp <= 0xFF9F) // Halfwidth voicing marks - || (cp >= 0xFE00 && cp <= 0xFE0F) // Variation Selectors VS-1..VS-16 - || (cp >= 0xE0100 && cp <= 0xE01EF); // Variation Selectors Supplement + return utf8IsCombiningMark(cp) || (cp >= 0x3099 && cp <= 0x309A) // Combining katakana-hiragana voicing + || (cp >= 0x302A && cp <= 0x302F) // Ideographic tone marks + || (cp >= 0xFF9E && cp <= 0xFF9F) // Halfwidth voicing marks + || (cp >= 0xFE00 && cp <= 0xFE0F) // Variation Selectors VS-1..VS-16 + || (cp >= 0xE0100 && cp <= 0xE01EF); // Variation Selectors Supplement } diff --git a/test/parsed_text/ParsedTextLayoutTest.cpp b/test/parsed_text/ParsedTextLayoutTest.cpp index 8aa17d0c2d..d47cca58a0 100644 --- a/test/parsed_text/ParsedTextLayoutTest.cpp +++ b/test/parsed_text/ParsedTextLayoutTest.cpp @@ -92,8 +92,8 @@ TEST(ParsedTextLayout, EnglishWrapsAtWordBoundaries) { TEST(ParsedTextLayout, NoBreakSpaceGlueKeepsGroupAtomic) { ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, /*focusReadingEnabled=*/false, leftAligned()); - text.addWord("x", EpdFontFamily::REGULAR); // join=Space (default) - text.addWord("200", EpdFontFamily::REGULAR); // join=Space — breakable before + text.addWord("x", EpdFontFamily::REGULAR); // join=Space (default) + text.addWord("200", EpdFontFamily::REGULAR); // join=Space — breakable before text.addWord(" ", EpdFontFamily::REGULAR, /*underline=*/false, WordJoin::Glue); // NBSP — Glue text.addWord("km", EpdFontFamily::REGULAR, /*underline=*/false, WordJoin::Glue); // Glue auto lines = linesOf(text, 6 * kCell); @@ -187,8 +187,7 @@ static size_t countCodepoints(const std::string& s) { // Add a CJK run: each char its own breakable, no-space word (mirrors the parser). static void addCjkRun(ParsedText& text, int count, const char* ch) { for (int i = 0; i < count; ++i) { - text.addWord(ch, EpdFontFamily::REGULAR, /*underline=*/false, - i == 0 ? WordJoin::Space : WordJoin::CjkBreak); + text.addWord(ch, EpdFontFamily::REGULAR, /*underline=*/false, i == 0 ? WordJoin::Space : WordJoin::CjkBreak); } } @@ -207,14 +206,15 @@ TEST(ParsedTextLayout, JapaneseWrapsAtViewportNotAtChunk) { TEST(ParsedTextLayout, ParserSequenceEnglishSpaceCjkPreservesSpace) { ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, /*focusReadingEnabled=*/false, leftAligned()); - text.addWord("English", EpdFontFamily::REGULAR); // join=Space (default) + text.addWord("English", EpdFontFamily::REGULAR); // join=Space (default) text.addWord("\xE6\x97\xA5", EpdFontFamily::REGULAR, false, WordJoin::Space); // 日 — first CJK after space text.addWord("\xE6\x9C\xAC", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 本 text.addWord("\xE8\xAA\x9E", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // 語 std::shared_ptr firstLine; FakeTextMetrics metrics(kCell); - text.layoutAndExtractLines(metrics, kFontId, static_cast(100 * kCell), - [&](std::shared_ptr b) { if (!firstLine) firstLine = b; }); + text.layoutAndExtractLines(metrics, kFontId, static_cast(100 * kCell), [&](std::shared_ptr b) { + if (!firstLine) firstLine = b; + }); ASSERT_TRUE(firstLine); const auto& xpos = firstLine->getWordXpos(); ASSERT_GE(xpos.size(), 2u); @@ -236,15 +236,15 @@ TEST(ParsedTextLayout, ParserSequenceCjkLatinNoSpace) { text.addWord("English", EpdFontFamily::REGULAR, false, WordJoin::CjkBreak); // Latin after CJK std::shared_ptr firstLine; FakeTextMetrics metrics(kCell); - text.layoutAndExtractLines(metrics, kFontId, static_cast(100 * kCell), - [&](std::shared_ptr b) { if (!firstLine) firstLine = b; }); + text.layoutAndExtractLines(metrics, kFontId, static_cast(100 * kCell), [&](std::shared_ptr b) { + if (!firstLine) firstLine = b; + }); ASSERT_TRUE(firstLine); const auto& xpos = firstLine->getWordXpos(); ASSERT_GE(xpos.size(), 4u); // Each CJK char is kCell wide. "English" must sit flush against 語's tail (CjkBreak // adds no space): xpos[3] - xpos[2] should equal kCell (just 語's width), not 2*kCell. - EXPECT_EQ(xpos[3] - xpos[2], kCell) - << "Latin word after CJK run must inherit CjkBreak (no space gap)"; + EXPECT_EQ(xpos[3] - xpos[2], kCell) << "Latin word after CJK run must inherit CjkBreak (no space gap)"; } // Layout-layer characterization for a CJK-Latin-CJK sequence (does NOT exercise the @@ -267,10 +267,10 @@ TEST(ParsedTextLayout, JapaneseRunWithSingleLatinCharNotDropped) { TEST(ParsedTextLayout, ParserSequenceNbspGluesCjk) { ParsedText text(/*extraParagraphSpacing=*/false, /*hyphenationEnabled=*/false, /*focusReadingEnabled=*/false, leftAligned()); - text.addWord("x", EpdFontFamily::REGULAR); // join=Space - text.addWord("200", EpdFontFamily::REGULAR); // join=Space - text.addWord(" ", EpdFontFamily::REGULAR, false, WordJoin::Glue); // NBSP word - text.addWord("\xE8\xAA\x9E", EpdFontFamily::REGULAR, false, WordJoin::Glue); // 語 inherits Glue + text.addWord("x", EpdFontFamily::REGULAR); // join=Space + text.addWord("200", EpdFontFamily::REGULAR); // join=Space + text.addWord(" ", EpdFontFamily::REGULAR, false, WordJoin::Glue); // NBSP word + text.addWord("\xE8\xAA\x9E", EpdFontFamily::REGULAR, false, WordJoin::Glue); // 語 inherits Glue auto lines = linesOf(text, 6 * kCell); ASSERT_EQ(lines.size(), 2u); EXPECT_EQ(lines[0], "x"); @@ -285,10 +285,9 @@ TEST(ParsedTextLayout, FocusReadingDoesNotBoldCjkTokens) { /*focusReadingEnabled=*/true, leftAligned()); addCjkRun(text, 25, "\xE8\x87\xAA"); // 25 × 自 with CjkBreak joins (first is Space) std::vector styles; - text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, - [&](std::shared_ptr block) { - for (const auto& s : block->getWordStyles()) styles.push_back(s); - }); + text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, [&](std::shared_ptr block) { + for (const auto& s : block->getWordStyles()) styles.push_back(s); + }); ASSERT_EQ(styles.size(), 25u); // sanity: every CJK char emitted exactly one style entry for (size_t i = 0; i < styles.size(); ++i) { EXPECT_EQ(static_cast(styles[i]) & static_cast(EpdFontFamily::BOLD), 0) @@ -305,14 +304,12 @@ TEST(ParsedTextLayout, FocusReadingDoesNotBoldCjkNfdClusters) { // Build 25 NFD clusters: か + U+3099 = "\xE3\x81\x8B\xE3\x82\x99" (6 bytes, 2 codepoints). const std::string kCluster = "\xE3\x81\x8B\xE3\x82\x99"; for (int i = 0; i < 25; ++i) { - text.addWord(kCluster, EpdFontFamily::REGULAR, /*underline=*/false, - i == 0 ? WordJoin::Space : WordJoin::CjkBreak); + text.addWord(kCluster, EpdFontFamily::REGULAR, /*underline=*/false, i == 0 ? WordJoin::Space : WordJoin::CjkBreak); } std::vector styles; - text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, - [&](std::shared_ptr block) { - for (const auto& s : block->getWordStyles()) styles.push_back(s); - }); + text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, [&](std::shared_ptr block) { + for (const auto& s : block->getWordStyles()) styles.push_back(s); + }); ASSERT_EQ(styles.size(), 25u); for (size_t i = 0; i < styles.size(); ++i) { EXPECT_EQ(static_cast(styles[i]) & static_cast(EpdFontFamily::BOLD), 0) @@ -331,10 +328,9 @@ TEST(ParsedTextLayout, FocusReadingPreservesUnderlineOnCjkCluster) { /*focusReadingEnabled=*/true, leftAligned()); text.addWord("\xE6\x97\xA5", EpdFontFamily::REGULAR, /*underline=*/true, WordJoin::Space); // 日, underlined std::vector styles; - text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, - [&](std::shared_ptr block) { - for (const auto& s : block->getWordStyles()) styles.push_back(s); - }); + text.layoutAndExtractLines(FakeTextMetrics(kCell), kFontId, 100 * kCell, [&](std::shared_ptr block) { + for (const auto& s : block->getWordStyles()) styles.push_back(s); + }); ASSERT_EQ(styles.size(), 1u); EXPECT_NE(static_cast(styles[0]) & static_cast(EpdFontFamily::UNDERLINE), 0) << "underline must survive the CJK focus-bypass"; diff --git a/test/utf8/Utf8PredicatesTest.cpp b/test/utf8/Utf8PredicatesTest.cpp index c7d60af9ad..a56e0c7541 100644 --- a/test/utf8/Utf8PredicatesTest.cpp +++ b/test/utf8/Utf8PredicatesTest.cpp @@ -1,15 +1,14 @@ -#include - #include +#include TEST(Utf8Predicates, CjkBreakableSupplementaryRanges) { - EXPECT_TRUE(utf8IsCjkBreakable(0x30000)); // CJK Extension G first codepoint - EXPECT_TRUE(utf8IsCjkBreakable(0x1B000)); // Kana Supplement representative - EXPECT_TRUE(utf8IsCjkBreakable(0x1B100)); // Kana Extended-A first codepoint - EXPECT_TRUE(utf8IsCjkBreakable(0x2B740)); // CJK Extension D first codepoint - EXPECT_TRUE(utf8IsCjkBreakable(0x2F800)); // CJK Compatibility Ideographs Supplement - EXPECT_TRUE(utf8IsCjkBreakable(0x31350)); // CJK Extension H first codepoint (Unicode 15.0) - EXPECT_FALSE(utf8IsCjkBreakable(0x0061)); // ASCII 'a' — must not match + EXPECT_TRUE(utf8IsCjkBreakable(0x30000)); // CJK Extension G first codepoint + EXPECT_TRUE(utf8IsCjkBreakable(0x1B000)); // Kana Supplement representative + EXPECT_TRUE(utf8IsCjkBreakable(0x1B100)); // Kana Extended-A first codepoint + EXPECT_TRUE(utf8IsCjkBreakable(0x2B740)); // CJK Extension D first codepoint + EXPECT_TRUE(utf8IsCjkBreakable(0x2F800)); // CJK Compatibility Ideographs Supplement + EXPECT_TRUE(utf8IsCjkBreakable(0x31350)); // CJK Extension H first codepoint (Unicode 15.0) + EXPECT_FALSE(utf8IsCjkBreakable(0x0061)); // ASCII 'a' — must not match } TEST(Utf8Predicates, GraphemeExtenderCoversAllOverlapRanges) {