diff --git a/core/fpdftext/cpdf_textpage.cpp b/core/fpdftext/cpdf_textpage.cpp index 97dd8a251..5492ada28 100644 --- a/core/fpdftext/cpdf_textpage.cpp +++ b/core/fpdftext/cpdf_textpage.cpp @@ -321,6 +321,33 @@ bool DrawsOnlySpaces(const CPDF_TextObject* text_obj) { return true; } +// EmbedPDF(bug-fix): whether `text_obj` draws only combining marks: harakat, +// Hebrew points, accents. A shaping engine places a mark above or below its +// letter, so a producer that draws each glyph as its own object draws the mark +// off the line's baseline, though it belongs to the line. +bool DrawsOnlyMarks(const CPDF_TextObject* text_obj) { + RetainPtr font = text_obj->GetFont(); + if (!font || text_obj->CharCount() == 0) { + return false; + } + for (size_t i = 0; i < text_obj->CharCount(); ++i) { + const uint32_t char_code = text_obj->GetCharCode(i); + if (char_code == CPDF_Font::kInvalidCharCode) { + return false; + } + const WideString unicode = font->UnicodeFromCharCode(char_code); + if (unicode.IsEmpty()) { + return false; + } + for (wchar_t c : unicode) { + if (pdfium::unicode::GetBidiClass(c) != BidiClass::kNSM) { + return false; + } + } + } + return true; +} + // EmbedPDF(text-geometry): a character's loose and tight boxes in its text // object's space. The tight box is upstream's char box, moved here from // ProcessTextObjectItems() so the text page and EPDFText_GetCharGeometry @@ -431,22 +458,14 @@ CFX_FloatRect GetLooseBounds(const CPDF_TextPage::CharInfo& charinfo) { return charinfo.char_box(); } -// EmbedPDF(bug-fix): whether `chars`, the line so far, ends in right-to-left -// text that isn't /ActualText. Spaces and other characters without a strong -// direction are skipped. -bool FollowsLiteralRightToLeftText( - pdfium::span chars) { - for (size_t i = chars.size(); i > 0; --i) { - const CPDF_TextPage::CharInfo& info = chars[i - 1]; - const BidiClass bidi = pdfium::unicode::GetBidiClass(info.unicode()); - if (bidi == BidiClass::kL) { - return false; - } - if (bidi == BidiClass::kR || bidi == BidiClass::kAL) { - return info.char_type() != CPDF_TextPage::CharType::kActualText; - } - } - return false; +// EmbedPDF(bug-fix): whether `a` and `b` come from one /ActualText span. +// ProcessMarkedContent() gives a span's characters the text object it is read +// from, and a text object is in one span at most. +bool InSameActualTextSpan(const CPDF_TextPage::CharInfo& a, + const CPDF_TextPage::CharInfo& b) { + return a.char_type() == CPDF_TextPage::CharType::kActualText && + b.char_type() == CPDF_TextPage::CharType::kActualText && + a.text_object() == b.text_object(); } // EmbedPDF(bug-fix): the text a line's bidi segments are cut from. An @@ -482,9 +501,7 @@ WideString BidiTextOf(const WideString& str, begin = end; continue; } - while (end < count && - chars[end].char_type() == CPDF_TextPage::CharType::kActualText && - chars[end].text_object() == first.text_object()) { + while (end < count && InSameActualTextSpan(chars[end], first)) { ++end; } @@ -1105,19 +1122,24 @@ void CPDF_TextPage::CloseTempLine() { (segment.direction == CFX_BidiChar::Direction::kNeutral && current_direction == CFX_BidiChar::Direction::kRight)) { current_direction = CFX_BidiChar::Direction::kRight; - const bool is_actual_text = - !char_span.empty() && - char_span[0].char_type() == CharType::kActualText; - if (is_actual_text) { - // Since /ActualText strings are already in logical order, pass their - // characters in forward order instead of reversing them. - for (auto [c, info] : fxcrt::Zip(str_span, char_span)) { - AddCharInfo(c, info, /*is_rtl=*/true); + // EmbedPDF(bug-fix): the segment is in visual order, but an /ActualText + // span in it is logical text. So the segment is reversed unit by unit: a + // span is one unit, kept as written, and any other character is a unit + // of its own. Upstream passed a segment that starts with a span forward + // whole, and reversed each span ahead of time where it followed other + // right-to-left text; spans drawn in visual order, one per letter, then + // came out in visual order. + size_t end = segment.count; + while (end > 0) { + size_t begin = end - 1; + while (begin > 0 && + InSameActualTextSpan(char_span[begin - 1], char_span[end - 1])) { + --begin; } - } else { - for (size_t i = segment.count; i > 0; --i) { - AddCharInfo(str_span[i - 1], char_span[i - 1], /*is_rtl=*/true); + for (size_t i = begin; i < end; ++i) { + AddCharInfo(str_span[i], char_span[i], /*is_rtl=*/true); } + end = begin; } } else { if (segment.direction != CFX_BidiChar::Direction::kLeftWeak) { @@ -1157,7 +1179,15 @@ void CPDF_TextPage::ProcessTextObject( } // Explicitly copy in case `text_objects_` gets cleared. + // EmbedPDF(bug-fix): the line's last glyph that isn't a mark; a mark sits + // off the baseline (DrawsOnlyMarks). TransformedTextObject prev_obj = text_objects_.back(); + for (size_t i = text_objects_.size(); i > 0; --i) { + if (!DrawsOnlyMarks(text_objects_[i - 1].text_obj_)) { + prev_obj = text_objects_[i - 1]; + break; + } + } size_t nItem = prev_obj.text_obj_->CountItems(); if (nItem == 0) { return; @@ -1184,7 +1214,18 @@ void CPDF_TextPage::ProcessTextObject( prev_obj.form_matrix_.Transform(prev_obj.text_obj_->GetPos())); CFX_PointF this_pos = display_matrix_.Transform(form_matrix.Transform(text_obj->GetPos())); - if (fabs(this_pos.y - prev_pos.y) > threshold * 2) { + // EmbedPDF(bug-fix): a mark, or a line of only marks so far, belongs to the + // line within a font size of its baseline: marks are placed that far above + // or below their letters, and a mark has no width to measure by. + float line_tolerance = threshold * 2; + if (DrawsOnlyMarks(text_obj) || DrawsOnlyMarks(prev_obj.text_obj_)) { + const float this_size = + this_matrix.TransformDistance(fabs(text_obj->GetFontSize())); + const float prev_size = + prev_matrix.TransformDistance(fabs(prev_obj.text_obj_->GetFontSize())); + line_tolerance = std::max({line_tolerance, this_size, prev_size}); + } + if (fabs(this_pos.y - prev_pos.y) > line_tolerance) { ProcessTransformedTextObjects(); text_objects_.clear(); text_objects_.push_back(new_obj); @@ -1258,7 +1299,9 @@ CPDF_TextPage::MarkedContentState CPDF_TextPage::PreMarkedContent( return MarkedContentState::kDelay; } -bool CPDF_TextPage::ProcessMarkedContent(const TransformedTextObject& obj) { +// EmbedPDF(bug-fix): returns nothing, as the span is never reversed here (see +// CloseTempLine()). +void CPDF_TextPage::ProcessMarkedContent(const TransformedTextObject& obj) { CPDF_TextObject* const text_obj = obj.text_obj_; const CPDF_ContentMarks* marks = text_obj->GetContentMarks(); const size_t content_marks_count = marks->CountItems(); @@ -1271,7 +1314,7 @@ bool CPDF_TextPage::ProcessMarkedContent(const TransformedTextObject& obj) { } } if (actual_text.IsEmpty()) { - return false; + return; } const bool is_rtl = @@ -1317,7 +1360,6 @@ bool CPDF_TextPage::ProcessMarkedContent(const TransformedTextObject& obj) { CharType::kActualText, wc, form_matrix.Transform(text_obj->GetPos()), form_matrix.TransformRect(char_box), matrix, text_obj)); } - return is_rtl; } void CPDF_TextPage::FindPreviousTextObject() { @@ -1386,19 +1428,9 @@ void CPDF_TextPage::ProcessTransformedTextObjects() { const size_t orig_char_list_index = temp_char_list_.size(); const size_t orig_buf_index = temp_text_buf_.GetLength(); if (ePreMKC == MarkedContentState::kDelay) { - // EmbedPDF(bug-fix): a right-to-left /ActualText run is reversed here - // only after literal right-to-left text, whose segment CloseTempLine() - // reverses as a whole. A run that starts its segment stays in logical - // order, because CloseTempLine() passes such a segment forward: reversing - // it here turned Acrobat's /ActualText around each shaped Arabic or - // Hebrew run back to front. - if (ProcessMarkedContent(obj) && - FollowsLiteralRightToLeftText( - pdfium::span(temp_char_list_).first(orig_char_list_index))) { - // TODO(thestig): It would be nicer if there was a way to avoid doing - // this second reversal. - ReverseTempTextBufs(orig_char_list_index, orig_buf_index); - } + // EmbedPDF(bug-fix): the span stays in logical order; CloseTempLine() + // reverses a right-to-left segment span by span. + ProcessMarkedContent(obj); prev_text_obj_ = text_obj; prev_matrix_ = form_matrix; continue; diff --git a/core/fpdftext/cpdf_textpage.h b/core/fpdftext/cpdf_textpage.h index bb4ae8cd5..bfe976ef3 100644 --- a/core/fpdftext/cpdf_textpage.h +++ b/core/fpdftext/cpdf_textpage.h @@ -210,9 +210,9 @@ class CPDF_TextPage { MarkedContentState PreMarkedContent(const CPDF_TextObject* text_obj); // Processes `/ActualText` marked content for `obj` and appends character info // and text to `temp_char_list_` and `temp_text_buf_`. - // Returns true if the caller needs to call ReverseTempTextBufs() because the - // text is RTL, requiring the newly appended character order to be reversed. - bool ProcessMarkedContent(const TransformedTextObject& obj); + // EmbedPDF(bug-fix): the text is appended as written, in logical order, and + // never reversed here; CloseTempLine() orders a line's spans. + void ProcessMarkedContent(const TransformedTextObject& obj); void FindPreviousTextObject(); void AddCharInfo(wchar_t wc, const CharInfo& info, bool is_rtl); TextOrientation GetTextObjectWritingMode( diff --git a/fpdfsdk/fpdf_text_embeddertest.cpp b/fpdfsdk/fpdf_text_embeddertest.cpp index bf901f899..5164d4c90 100644 --- a/fpdfsdk/fpdf_text_embeddertest.cpp +++ b/fpdfsdk/fpdf_text_embeddertest.cpp @@ -2635,19 +2635,13 @@ TEST_F(FPDFTextEmbedderTest, ArabicActualTextOrdering) { // TODO(crbug.com/525093248): Text extraction for the remaining variants // currently produces differing results with unintended linebreaks/spaces // and reversed characters within spans. - // EmbedPDF(bug-fix): what the fork extracts. It reverses an /ActualText - // span only after literal right-to-left text and reads marks with their - // base (Acrobat's shaped runs come out right), so variant 2, spans in - // visual order, keeps that order where upstream reverses it. Ordering a - // line's spans by position would fix both; until then this records it. - const std::vector kExpectedPages[] = { - {0x0645, 0x0650, '\r', '\n', 0x0633, 0x0652, 0x0628, 0x0650, '\0'}, - {0x0645, 0x0650, 0x0633, 0x0652, 0x0628, 0x0650, '\0'}, - {0x0633, 0x0652, 0x0628, 0x0650, ' ', 0x0628, 0x0650, '\r', '\n', 0x0645, - 0x0650, '\0'}, - {0x0633, 0x0652, 0x0628, 0x0650, ' ', 0x0628, 0x0650, '\r', '\n', 0x0645, - 0x0650, 0x0633, 0x0652, '\r', '\n', 0x0645, 0x0650, '\0'}, - }; + // EmbedPDF(bug-fix): the fork extracts all four as typed. A line's marks + // stay on it, and a right-to-left run is reversed span by span, each + // /ActualText span kept as written. + const std::vector kWord = {0x0628, 0x0650, 0x0633, 0x0652, + 0x0645, 0x0650, '\0'}; + const std::vector kExpectedPages[] = {kWord, kWord, kWord, + kWord}; int page_index = 0; for (const std::vector& expected_page : kExpectedPages) {