From f2aefe2f0196a5188ef1c7f74e639ff657f2d781 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 27 Aug 2019 22:08:02 +0000 Subject: [PATCH] Simplify PDFEngine::GetTextRunInfo(). Change it to return base::Optional, instead of returning the individual elements of PP_PrivateAccessibilityTextRunInfo as out parameters. Update the caller of GetTextRunInfo() to std::move() the text run into the vector that holds all the text runs, and simplify calculations for char indices in the caller. Change-Id: I719119935ea201f5aed7d1d41e706831a1696c90 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1769220 Commit-Queue: Lei Zhang Reviewed-by: Ian Prest Reviewed-by: Kevin Babbitt Cr-Commit-Position: refs/heads/master@{#690901} --- pdf/accessibility.cc | 22 ++++++++++----------- pdf/pdf_engine.h | 11 +++++------ pdf/pdfium/pdfium_engine.cc | 7 +++---- pdf/pdfium/pdfium_engine.h | 5 ++--- pdf/pdfium/pdfium_page.cc | 39 +++++++++++++++++-------------------- pdf/pdfium/pdfium_page.h | 9 +++------ 6 files changed, 42 insertions(+), 51 deletions(-) diff --git a/pdf/accessibility.cc b/pdf/accessibility.cc index 20b0bd8794eee8..7662c583766080 100644 --- a/pdf/accessibility.cc +++ b/pdf/accessibility.cc @@ -39,10 +39,12 @@ bool GetAccessibilityInfo( int char_index = 0; while (char_index < char_count) { - PP_PrivateAccessibilityTextRunInfo text_run_info; - engine->GetTextRunInfo(page_index, char_index, &text_run_info); - DCHECK_LE(char_index + text_run_info.len, - static_cast(char_count)); + base::Optional text_run_info_result = + engine->GetTextRunInfo(page_index, char_index); + DCHECK(text_run_info_result.has_value()); + const auto& text_run_info = text_run_info_result.value(); + uint32_t text_run_end = char_index + text_run_info.len; + DCHECK_LE(text_run_end, static_cast(char_count)); text_runs->push_back(text_run_info); // We need to provide enough information to draw a bounding box @@ -55,11 +57,10 @@ bool GetAccessibilityInfo( // can be computed from the bounds of the text run. // The same idea is used for RTL, TTB and BTT text direction. pp::FloatRect char_bounds = engine->GetCharBounds(page_index, char_index); - for (uint32_t i = 0; i < text_run_info.len - 1; i++) { - DCHECK_LT(char_index + i + 1, static_cast(char_count)); - pp::FloatRect next_char_bounds = - engine->GetCharBounds(page_index, char_index + i + 1); - double& char_width = (*chars)[char_index + i].char_width; + for (uint32_t i = char_index; i < text_run_end - 1; i++) { + DCHECK_LT(i + 1, static_cast(char_count)); + pp::FloatRect next_char_bounds = engine->GetCharBounds(page_index, i + 1); + double& char_width = (*chars)[i].char_width; switch (text_run_info.direction) { case PP_PRIVATEDIRECTION_NONE: case PP_PRIVATEDIRECTION_LTR: @@ -77,8 +78,7 @@ bool GetAccessibilityInfo( } char_bounds = next_char_bounds; } - double& char_width = - (*chars)[char_index + text_run_info.len - 1].char_width; + double& char_width = (*chars)[text_run_end - 1].char_width; if (text_run_info.direction == PP_PRIVATEDIRECTION_BTT || text_run_info.direction == PP_PRIVATEDIRECTION_TTB) { char_width = char_bounds.height(); diff --git a/pdf/pdf_engine.h b/pdf/pdf_engine.h index ab3f8cde865dbc..f438a8a87c5bf9 100644 --- a/pdf/pdf_engine.h +++ b/pdf/pdf_engine.h @@ -359,13 +359,12 @@ class PDFEngine { // Get a given unicode character on a given page. virtual uint32_t GetCharUnicode(int page_index, int char_index) = 0; // Given a start char index, find the longest continuous run of text that's - // in a single direction and with the same style and font size. Fill the - // |text_run_info| with the length of that sequence, text direction, bounding - // box and font size. - virtual void GetTextRunInfo( + // in a single direction and with the same style and font size. Return a + // filled out PP_PrivateAccessibilityTextRunInfo on success or base::nullopt + // on failure. e.g. When |start_char_index| is out of bounds. + virtual base::Optional GetTextRunInfo( int page_index, - int start_char_index, - PP_PrivateAccessibilityTextRunInfo* text_run_info) = 0; + int start_char_index) = 0; // Gets the PDF document's print scaling preference. True if the document can // be scaled to fit. virtual bool GetPrintScaling() = 0; diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc index 51fccde9ac0f7a..79f71747ff0834 100644 --- a/pdf/pdfium/pdfium_engine.cc +++ b/pdf/pdfium/pdfium_engine.cc @@ -2189,12 +2189,11 @@ uint32_t PDFiumEngine::GetCharUnicode(int page_index, int char_index) { return pages_[page_index]->GetCharUnicode(char_index); } -void PDFiumEngine::GetTextRunInfo( +base::Optional PDFiumEngine::GetTextRunInfo( int page_index, - int start_char_index, - PP_PrivateAccessibilityTextRunInfo* text_run_info) { + int start_char_index) { DCHECK(PageIndexInBounds(page_index)); - return pages_[page_index]->GetTextRunInfo(start_char_index, text_run_info); + return pages_[page_index]->GetTextRunInfo(start_char_index); } bool PDFiumEngine::GetPrintScaling() { diff --git a/pdf/pdfium/pdfium_engine.h b/pdf/pdfium/pdfium_engine.h index 0b5a1adcddfdb2..7309ddba5a503c 100644 --- a/pdf/pdfium/pdfium_engine.h +++ b/pdf/pdfium/pdfium_engine.h @@ -113,10 +113,9 @@ class PDFiumEngine : public PDFEngine, int GetCharCount(int page_index) override; pp::FloatRect GetCharBounds(int page_index, int char_index) override; uint32_t GetCharUnicode(int page_index, int char_index) override; - void GetTextRunInfo( + base::Optional GetTextRunInfo( int page_index, - int start_char_index, - PP_PrivateAccessibilityTextRunInfo* text_run_info) override; + int start_char_index) override; bool GetPrintScaling() override; int GetCopiesToPrint() override; int GetDuplexType() override; diff --git a/pdf/pdfium/pdfium_page.cc b/pdf/pdfium/pdfium_page.cc index 256a23919c0d46..f05926e17b60ca 100644 --- a/pdf/pdfium/pdfium_page.cc +++ b/pdf/pdfium/pdfium_page.cc @@ -251,20 +251,14 @@ FPDF_TEXTPAGE PDFiumPage::GetTextPage() { return text_page(); } -void PDFiumPage::GetTextRunInfo( - int start_char_index, - PP_PrivateAccessibilityTextRunInfo* text_run_info) { +base::Optional PDFiumPage::GetTextRunInfo( + int start_char_index) { FPDF_PAGE page = GetPage(); FPDF_TEXTPAGE text_page = GetTextPage(); int chars_count = FPDFText_CountChars(text_page); // Check to make sure |start_char_index| is within bounds. - if (start_char_index < 0 || start_char_index >= chars_count) { - text_run_info->len = 0; - text_run_info->font_size = 0; - text_run_info->bounds = pp::FloatRect(); - text_run_info->direction = PP_PRIVATEDIRECTION_NONE; - return; - } + if (start_char_index < 0 || start_char_index >= chars_count) + return base::nullopt; int actual_start_char_index = GetFirstNonUnicodeWhiteSpaceCharIndex( text_page, start_char_index, chars_count); @@ -274,11 +268,12 @@ void PDFiumPage::GetTextRunInfo( // If so, |text_run_info->len| needs to take the number of characters // iterated into account. DCHECK_GT(actual_start_char_index, start_char_index); - text_run_info->len = chars_count - start_char_index; - text_run_info->font_size = 0; - text_run_info->bounds = pp::FloatRect(); - text_run_info->direction = PP_PRIVATEDIRECTION_NONE; - return; + PP_PrivateAccessibilityTextRunInfo info; + info.len = chars_count - start_char_index; + info.font_size = 0; + info.bounds = pp::FloatRect(); + info.direction = PP_PRIVATEDIRECTION_NONE; + return info; } int char_index = actual_start_char_index; @@ -383,16 +378,18 @@ void PDFiumPage::GetTextRunInfo( text_run_font_size = estimated_font_size; } + PP_PrivateAccessibilityTextRunInfo info; + info.len = char_index - start_char_index; + info.font_size = text_run_font_size; + info.bounds = text_run_bounds; // Infer text direction from first and last character of the text run. We // can't base our decision on the character direction, since a character of a // RTL language will have an angle of 0 when not rotated, just like a // character in a LTR language. - text_run_info->direction = char_index - actual_start_char_index > 1 - ? GetDirectionFromAngle(text_run_angle) - : PP_PRIVATEDIRECTION_NONE; - text_run_info->len = char_index - start_char_index; - text_run_info->font_size = text_run_font_size; - text_run_info->bounds = text_run_bounds; + info.direction = char_index - actual_start_char_index > 1 + ? GetDirectionFromAngle(text_run_angle) + : PP_PRIVATEDIRECTION_NONE; + return info; } uint32_t PDFiumPage::GetCharUnicode(int char_index) { diff --git a/pdf/pdfium/pdfium_page.h b/pdf/pdfium/pdfium_page.h index 81315f2b6c1522..c7e347af8dc76d 100644 --- a/pdf/pdfium/pdfium_page.h +++ b/pdf/pdfium/pdfium_page.h @@ -44,12 +44,9 @@ class PDFiumPage { // Returns FPDF_TEXTPAGE for the page, loading and parsing it if necessary. FPDF_TEXTPAGE GetTextPage(); - // Given a start char index, find the longest continuous run of text that's - // in a single direction and with the same style and font size. Fill the - // |text_run_info| with the length of that sequence, text direction, bounding - // box and font size. - void GetTextRunInfo(int start_char_index, - PP_PrivateAccessibilityTextRunInfo* text_run_info); + // See definition of PDFEngine::GetTextRunInfo(). + base::Optional GetTextRunInfo( + int start_char_index); // Get a unicode character from the page. uint32_t GetCharUnicode(int char_index); // Get the bounds of a character in page pixels.