Commit d37ff728 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Fix bounding box in ShapeResult::CopyRange

This patch fixes ShapeResult::CopyRange to compute more accurate
glyph bounding box.

There were 2 problems before this patch:
1. It always truncated the right edge. This is problematic when
   Copying (0, 1) of ". " in RTL, which resulted in an empty
   result.
2. It assumed no glyph overflow at the edge. This resulted in
   common glyph overflows such as "t" or "m" were cut off.

This patch extracts the logic to compute glyph bounding box from
ComputeGlyphPositions to a new helper class. CopyRange uses the
class to compute the line-left/right most glyph bounding box.

Bug: 636993, 714962
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I7d2068566308081727c996e3ac0fe23aae6558d1
Reviewed-on: https://chromium-review.googlesource.com/1023536Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554645}
parent c2ffec8a
......@@ -24,12 +24,12 @@
},
{
"object": "NGPaintFragment",
"rect": [8, 69, 369, 20],
"rect": [8, 69, 370, 20],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [8, 69, 369, 20],
"rect": [8, 69, 370, 20],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -324,12 +324,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -314,12 +314,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 581, 406, 19],
"rect": [14, 541, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 561, 406, 19],
"rect": [14, 521, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 541, 406, 19],
"rect": [14, 581, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 521, 406, 19],
"rect": [14, 561, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -319,12 +319,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 361, 49, 19],
"rect": [371, 361, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -329,12 +329,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -319,12 +319,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -324,12 +324,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......@@ -354,7 +354,7 @@
},
{
"object": "NGPaintFragment",
"rect": [350, 400, 19, 19],
"rect": [350, 400, 20, 19],
"reason": "appeared"
}
]
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -319,12 +319,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -319,12 +319,12 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -139,18 +139,18 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 480, 355, 19],
"rect": [14, 460, 356, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 480, 355, 19],
"reason": "disappeared"
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 460, 355, 19],
"reason": "appeared"
"rect": [14, 480, 355, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
......@@ -324,7 +324,7 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -29,42 +29,42 @@
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 580, 406, 19],
"rect": [14, 540, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 560, 406, 19],
"rect": [14, 520, 407, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 540, 406, 19],
"rect": [14, 580, 406, 19],
"reason": "disappeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [14, 520, 406, 19],
"rect": [14, 560, 406, 19],
"reason": "disappeared"
},
{
......@@ -314,7 +314,7 @@
},
{
"object": "NGPaintFragment",
"rect": [371, 360, 49, 19],
"rect": [371, 360, 50, 19],
"reason": "disappeared"
},
{
......
......@@ -19,12 +19,12 @@
"paintInvalidations": [
{
"object": "NGPaintFragment",
"rect": [8, 8, 742, 19],
"rect": [8, 8, 743, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [8, 8, 742, 19],
"rect": [8, 8, 743, 19],
"reason": "disappeared"
},
{
......
......@@ -19,12 +19,12 @@
"paintInvalidations": [
{
"object": "NGPaintFragment",
"rect": [8, 8, 762, 19],
"rect": [8, 8, 763, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [8, 8, 762, 19],
"rect": [8, 8, 763, 19],
"reason": "disappeared"
},
{
......
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"drawsContent": false,
"backgroundColor": "#FFFFFF"
},
{
"name": "Scrolling Layer",
"bounds": [800, 600],
"drawsContent": false
},
{
"name": "Scrolling Contents Layer",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF",
"paintInvalidations": [
{
"object": "NGPaintFragment",
"rect": [11, 35, 74, 19],
"reason": "selection"
},
{
"object": "NGPaintFragment",
"rect": [11, 11, 55, 19],
"reason": "selection"
}
]
}
],
"objectPaintInvalidations": [
{
"object": "NGPaintFragment",
"reason": "selection"
},
{
"object": "NGPaintFragment",
"reason": "selection"
}
]
}
......@@ -19,12 +19,12 @@
"paintInvalidations": [
{
"object": "NGPaintFragment",
"rect": [8, 16, 749, 19],
"rect": [8, 16, 750, 19],
"reason": "appeared"
},
{
"object": "NGPaintFragment",
"rect": [8, 16, 749, 19],
"rect": [8, 16, 750, 19],
"reason": "disappeared"
},
{
......
......@@ -907,7 +907,8 @@ TEST_F(HarfBuzzShaperTest, ShapeResultCopyRangeIntoArabicThaiHanLatin) {
EXPECT_EQ(result->NumCharacters(), composite_result->NumCharacters());
EXPECT_EQ(result->SnappedWidth(), composite_result->SnappedWidth());
EXPECT_EQ(result->Bounds(), composite_result->Bounds());
EXPECT_TRUE(composite_result->Bounds().Contains(result->Bounds()))
<< composite_result->Bounds() << "/" << result->Bounds();
EXPECT_EQ(result->SnappedStartPositionForOffset(0),
composite_result->SnappedStartPositionForOffset(0));
EXPECT_EQ(result->SnappedStartPositionForOffset(1),
......@@ -968,6 +969,30 @@ TEST_F(HarfBuzzShaperTest, ShapeResultCopyRangeSegmentGlyphBoundingBox) {
EXPECT_NEAR(result->Width(), result->Bounds().Width(), result->Width() * .1);
}
TEST_F(HarfBuzzShaperTest, ShapeResultCopyRangeBoundsLtr) {
String string(u". ");
TextDirection direction = TextDirection::kLtr;
HarfBuzzShaper shaper(string.Characters16(), string.length());
scoped_refptr<ShapeResult> result = shaper.Shape(&font, direction);
// Because a space character does not have ink, the bounds of "." should be
// the same as the bounds of ". ".
scoped_refptr<ShapeResult> sub_range = result->SubRange(0, 1);
EXPECT_EQ(sub_range->Bounds().Width(), result->Bounds().Width());
}
TEST_F(HarfBuzzShaperTest, ShapeResultCopyRangeBoundsRtl) {
String string(u". ");
TextDirection direction = TextDirection::kRtl;
HarfBuzzShaper shaper(string.Characters16(), string.length());
scoped_refptr<ShapeResult> result = shaper.Shape(&font, direction);
// Because a space character does not have ink, the bounds of "." should be
// the same as the bounds of ". ".
scoped_refptr<ShapeResult> sub_range = result->SubRange(0, 1);
EXPECT_EQ(sub_range->Bounds().Width(), result->Bounds().Width());
}
TEST_F(HarfBuzzShaperTest, SubRange) {
String string(u"Hello world");
TextDirection direction = TextDirection::kRtl;
......
......@@ -216,8 +216,7 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
void ComputeGlyphPositions(ShapeResult::RunInfo*,
unsigned start_glyph,
unsigned num_glyphs,
hb_buffer_t*,
FloatRect* glyph_bounding_box);
hb_buffer_t*);
void InsertRun(std::unique_ptr<ShapeResult::RunInfo>,
unsigned start_glyph,
unsigned num_glyphs,
......@@ -226,6 +225,9 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
void InsertRunForIndex(unsigned start_character_index);
void ReorderRtlRuns(unsigned run_size_before);
float LineLeftBounds() const;
float LineRightBounds() const;
float width_;
FloatRect glyph_bounding_box_;
Vector<std::unique_ptr<RunInfo>> runs_;
......
......@@ -48,6 +48,11 @@ struct HarfBuzzRunGlyphData {
uint16_t character_index;
float advance;
FloatSize offset;
void SetGlyphAndPositions(uint16_t glyph_id,
uint16_t character_index,
float advance,
const FloatSize& offset);
};
struct ShapeResult::RunInfo {
......@@ -88,11 +93,6 @@ struct ShapeResult::RunInfo {
float XPositionForVisualOffset(unsigned, AdjustMidCluster) const;
float XPositionForOffset(unsigned, AdjustMidCluster) const;
void CharacterIndexForXPosition(float, GlyphIndexResult*) const;
void SetGlyphAndPositions(unsigned index,
uint16_t glyph_id,
float advance,
float offset_x,
float offset_y);
size_t GlyphToCharacterIndex(size_t i) const {
return start_index_ + glyph_data_[i].character_index;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment