Skip to content

Commit

Permalink
libtxt: some cleanup (flutter#4268)
Browse files Browse the repository at this point in the history 
* rename glyph_position_x to glyph_lines
* use round instead of roundf
* return a range start/end struct in Paragraph::GetWordBoundary
  • Loading branch information
@jason-simmons
jason-simmons authored Oct 24, 2017
1 parent 91071f8 commit 054a2cc
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 61 deletions.
6 changes: 3 additions & 3 deletions lib/ui/text/paragraph_impl_txt.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ Dart_Handle ParagraphImplTxt::getPositionForOffset(double dx, double dy) {
}

Dart_Handle ParagraphImplTxt::getWordBoundary(unsigned offset) {
SkIPoint point = m_paragraph->GetWordBoundary(offset);
txt::Paragraph::Range point = m_paragraph->GetWordBoundary(offset);
Dart_Handle result = Dart_NewList(2);
Dart_ListSetAt(result, 0, ToDart(point.x()));
Dart_ListSetAt(result, 1, ToDart(point.y()));
Dart_ListSetAt(result, 0, ToDart(point.start));
Dart_ListSetAt(result, 1, ToDart(point.end));
return result;
}

Expand Down
71 changes: 32 additions & 39 deletions third_party/txt/src/txt/paragraph.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <algorithm>
#include <limits>
#include <numeric>
#include <tuple>
#include <utility>
#include <vector>

Expand All @@ -42,11 +41,6 @@
namespace txt {
namespace {

struct Range {
Range(size_t s, size_t e) : start(s), end(e) {}
size_t start, end;
};

const sk_sp<SkTypeface>& GetTypefaceForGlyph(const minikin::Layout& layout,
size_t index) {
const FontSkia* font = static_cast<const FontSkia*>(layout.getFont(index));
Expand Down Expand Up @@ -123,7 +117,7 @@ void GetPaint(const TextStyle& style, SkPaint* paint) {
void FindWords(const std::vector<uint16_t>& text,
size_t start,
size_t end,
std::vector<Range>* words) {
std::vector<Paragraph::Range>* words) {
bool in_word = false;
size_t word_start;
for (size_t i = start; i < end; ++i) {
Expand Down Expand Up @@ -274,7 +268,7 @@ void Paragraph::Layout(double width, bool force) {

records_.clear();
line_heights_.clear();
glyph_position_x_.clear();
glyph_lines_.clear();

minikin::Layout layout;
SkTextBlobBuilder builder;
Expand Down Expand Up @@ -316,7 +310,7 @@ void Paragraph::Layout(double width, bool force) {
run_index++;
}

std::vector<GlyphPosition> glyph_single_line_position_x;
std::vector<GlyphPosition> glyph_positions;
double run_x_offset = GetLineXOffset(line_number);
double justify_x_offset = 0;
std::vector<PaintRecord> paint_records;
Expand Down Expand Up @@ -436,14 +430,14 @@ void Paragraph::Layout(double width, bool force) {
}
float subglyph_advance =
glyph_advance / subglyph_code_unit_counts.size();
glyph_single_line_position_x.emplace_back(
run_x_offset + glyph_x_offset, subglyph_advance,
subglyph_code_unit_counts[0]);
glyph_positions.emplace_back(run_x_offset + glyph_x_offset,
subglyph_advance,
subglyph_code_unit_counts[0]);

// Compute positions for the additional characters in the ligature.
for (size_t i = 1; i < subglyph_code_unit_counts.size(); ++i) {
glyph_single_line_position_x.emplace_back(
glyph_single_line_position_x.back().start + subglyph_advance,
glyph_positions.emplace_back(
glyph_positions.back().start + subglyph_advance,
subglyph_advance, subglyph_code_unit_counts[i]);
}

Expand All @@ -455,8 +449,7 @@ void Paragraph::Layout(double width, bool force) {

if (!isnan(word_start_position)) {
double word_width =
glyph_single_line_position_x.back().glyph_end() -
word_start_position;
glyph_positions.back().glyph_end() - word_start_position;
max_word_width = std::max(word_width, max_word_width);
word_start_position = std::numeric_limits<double>::quiet_NaN();
}
Expand Down Expand Up @@ -498,8 +491,8 @@ void Paragraph::Layout(double width, bool force) {
}

line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) +
roundf(max_line_spacing + max_descent));
y_offset += roundf(max_line_spacing + prev_max_descent);
round(max_line_spacing + max_descent));
y_offset += round(max_line_spacing + prev_max_descent);
prev_max_descent = max_descent;

for (PaintRecord& paint_record : paint_records) {
Expand All @@ -511,8 +504,8 @@ void Paragraph::Layout(double width, bool force) {
size_t next_line_start = (line_number < line_ranges_.size() - 1)
? line_ranges_[line_number + 1].start
: text_.size();
glyph_position_x_.emplace_back(std::move(glyph_single_line_position_x),
next_line_start - line_range.start);
glyph_lines_.emplace_back(std::move(glyph_positions),
next_line_start - line_range.start);
}

max_intrinsic_width_ = 0;
Expand Down Expand Up @@ -767,34 +760,34 @@ std::vector<SkRect> Paragraph::GetRectsForRange(size_t start,

size_t pos = 0;
size_t line;
for (line = 0; line < glyph_position_x_.size(); ++line) {
if (start < pos + glyph_position_x_[line].total_code_units)
for (line = 0; line < glyph_lines_.size(); ++line) {
if (start < pos + glyph_lines_[line].total_code_units)
break;
pos += glyph_position_x_[line].total_code_units;
pos += glyph_lines_[line].total_code_units;
}
if (line == glyph_position_x_.size())
if (line == glyph_lines_.size())
return rects;

if (end <= pos + glyph_position_x_[line].total_code_units) {
if (end <= pos + glyph_lines_[line].total_code_units) {
rects.push_back(GetRectForLineRange(line, start - pos, end - pos));
return rects;
}

rects.push_back(GetRectForLineRange(
line, start - pos, glyph_position_x_[line].total_code_units));
rects.push_back(GetRectForLineRange(line, start - pos,
glyph_lines_[line].total_code_units));

while (true) {
pos += glyph_position_x_[line].total_code_units;
pos += glyph_lines_[line].total_code_units;
line++;
if (line == glyph_position_x_.size())
if (line == glyph_lines_.size())
break;

if (end <= pos + glyph_position_x_[line].total_code_units) {
if (end <= pos + glyph_lines_[line].total_code_units) {
rects.push_back(GetRectForLineRange(line, 0, end - pos));
break;
} else {
rects.push_back(GetRectForLineRange(
line, 0, glyph_position_x_[line].total_code_units));
rects.push_back(
GetRectForLineRange(line, 0, glyph_lines_[line].total_code_units));
}
}

Expand All @@ -804,8 +797,8 @@ std::vector<SkRect> Paragraph::GetRectsForRange(size_t start,
SkRect Paragraph::GetRectForLineRange(size_t line,
size_t start,
size_t end) const {
FXL_DCHECK(line < glyph_position_x_.size());
const GlyphLine& glyph_line = glyph_position_x_[line];
FXL_DCHECK(line < glyph_lines_.size());
const GlyphLine& glyph_line = glyph_lines_[line];
if (glyph_line.positions.empty())
return SkRect::MakeEmpty();

Expand All @@ -830,11 +823,11 @@ Paragraph::PositionWithAffinity Paragraph::GetGlyphPositionAtCoordinate(
for (y_index = 0; y_index < line_heights_.size() - 1; ++y_index) {
if (dy < line_heights_[y_index])
break;
offset += glyph_position_x_[y_index].total_code_units;
offset += glyph_lines_[y_index].total_code_units;
}

const std::vector<GlyphPosition>& line_glyph_position =
glyph_position_x_[y_index].positions;
glyph_lines_[y_index].positions;
size_t x_index;
for (x_index = 0; x_index < line_glyph_position.size(); ++x_index) {
double glyph_end = (x_index < line_glyph_position.size() - 1)
Expand All @@ -855,11 +848,11 @@ Paragraph::PositionWithAffinity Paragraph::GetGlyphPositionAtCoordinate(
return PositionWithAffinity(offset, x_index > 0 ? UPSTREAM : DOWNSTREAM);
}

SkIPoint Paragraph::GetWordBoundary(size_t offset) const {
Paragraph::Range Paragraph::GetWordBoundary(size_t offset) const {
// TODO(garyq): Consider punctuation as separate words.
if (text_.size() == 0)
return SkIPoint::Make(0, 0);
return SkIPoint::Make(
return Range(0, 0);
return Range(
minikin::getPrevWordBreakForCache(text_.data(), offset + 1, text_.size()),
minikin::getNextWordBreakForCache(text_.data(), offset, text_.size()));
}
Expand Down
14 changes: 11 additions & 3 deletions third_party/txt/src/txt/paragraph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#define LIB_TXT_SRC_PARAGRAPH_H_

#include <set>
#include <tuple>
#include <utility>
#include <vector>

#include "font_collection.h"
Expand Down Expand Up @@ -63,6 +63,14 @@ class Paragraph {
PositionWithAffinity(size_t p, Affinity a) : position(p), affinity(a) {}
};

struct Range {
Range(size_t s, size_t e) : start(s), end(e) {}
size_t start, end;
bool operator==(const Range& other) const {
return start == other.start && end == other.end;
}
};

// Minikin Layout doLayout() and LineBreaker addStyleRun() has an
// O(N^2) (according to benchmarks) time complexity where N is the total
// number of characters. However, this is not significant for reasonably sized
Expand Down Expand Up @@ -133,7 +141,7 @@ class Paragraph {

// Finds the first and last glyphs that define a word containing the glyph at
// index offset.
SkIPoint GetWordBoundary(size_t offset) const;
Range GetWordBoundary(size_t offset) const;

// Returns the number of lines the paragraph takes up. If the text exceeds the
// amount width and maxlines provides, Layout() truncates the extra text from
Expand Down Expand Up @@ -218,7 +226,7 @@ class Paragraph {
};

// Holds the laid out x positions of each glyph.
std::vector<GlyphLine> glyph_position_x_;
std::vector<GlyphLine> glyph_lines_;

// The max width of the paragraph as provided in the most recent Layout()
// call.
Expand Down
32 changes: 16 additions & 16 deletions third_party/txt/tests/paragraph_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1075,35 +1075,35 @@ TEST_F(ParagraphTest, GetWordBoundaryParagraph) {
SkRect rect = GetCoordinatesForGlyphPosition(*paragraph, 0);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(0), SkIPoint::Make(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(1), SkIPoint::Make(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(2), SkIPoint::Make(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(3), SkIPoint::Make(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(4), SkIPoint::Make(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(0), txt::Paragraph::Range(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(1), txt::Paragraph::Range(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(2), txt::Paragraph::Range(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(3), txt::Paragraph::Range(0, 5));
EXPECT_EQ(paragraph->GetWordBoundary(4), txt::Paragraph::Range(0, 5));
rect = GetCoordinatesForGlyphPosition(*paragraph, 5);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(5), SkIPoint::Make(5, 6));
EXPECT_EQ(paragraph->GetWordBoundary(5), txt::Paragraph::Range(5, 6));
rect = GetCoordinatesForGlyphPosition(*paragraph, 6);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(6), SkIPoint::Make(6, 7));
EXPECT_EQ(paragraph->GetWordBoundary(6), txt::Paragraph::Range(6, 7));
rect = GetCoordinatesForGlyphPosition(*paragraph, 7);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(7), SkIPoint::Make(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(8), SkIPoint::Make(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(9), SkIPoint::Make(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(10), SkIPoint::Make(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(11), SkIPoint::Make(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(7), txt::Paragraph::Range(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(8), txt::Paragraph::Range(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(9), txt::Paragraph::Range(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(10), txt::Paragraph::Range(7, 12));
EXPECT_EQ(paragraph->GetWordBoundary(11), txt::Paragraph::Range(7, 12));
rect = GetCoordinatesForGlyphPosition(*paragraph, 12);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(12), SkIPoint::Make(12, 13));
EXPECT_EQ(paragraph->GetWordBoundary(12), txt::Paragraph::Range(12, 13));
rect = GetCoordinatesForGlyphPosition(*paragraph, 13);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(13), SkIPoint::Make(13, 18));
EXPECT_EQ(paragraph->GetWordBoundary(13), txt::Paragraph::Range(13, 18));
rect = GetCoordinatesForGlyphPosition(*paragraph, 18);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

Expand All @@ -1119,15 +1119,15 @@ TEST_F(ParagraphTest, GetWordBoundaryParagraph) {
rect = GetCoordinatesForGlyphPosition(*paragraph, 30);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(30), SkIPoint::Make(30, 31));
EXPECT_EQ(paragraph->GetWordBoundary(30), txt::Paragraph::Range(30, 31));
rect = GetCoordinatesForGlyphPosition(*paragraph, 31);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

rect = GetCoordinatesForGlyphPosition(*paragraph, icu_text.length() - 5);
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

EXPECT_EQ(paragraph->GetWordBoundary(icu_text.length() - 1),
SkIPoint::Make(icu_text.length() - 5, icu_text.length()));
txt::Paragraph::Range(icu_text.length() - 5, icu_text.length()));
rect = GetCoordinatesForGlyphPosition(*paragraph, icu_text.length());
GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);

Expand Down

0 comments on commit 054a2cc

Please sign in to comment.