Commit 1f84e158 authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Fix subrange shaping logic when small caps feature is active

The existing logic was to exclude segmenter output that does not overlap
with range start and range end (HarfBuzzShaper::Shape) and to restrict
shaping to the character indexes of the queue items that contribute to
the desired range. However, the latter restriction only worked if the
case mapping for small caps did not chop up the queue items further, due
to case change segmentation. Adding an early-out if the result of case
mapping chopping produces queue items that fall outside the desired
range.

Thanks to kojii@ for the report and initial test case.

Bug: 817271
Test: RangeShapeSmallCaps
Change-Id: If8c0b00343e7b1c9a342305e5f44fa515ba3576c
Reviewed-on: https://chromium-review.googlesource.com/964445Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543464}
parent 2754ffaa
......@@ -835,6 +835,13 @@ void HarfBuzzShaper::ShapeSegment(RangeData* range_data,
if (caps_support.NeedsRunCaseSplitting()) {
SplitUntilNextCaseChange(text_, &range_data->reshape_queue,
current_queue_item, small_caps_behavior);
// Skip queue items generated by SplitUntilNextCaseChange that do not
// contribute to the shape result if the range_data restricts shaping to
// a substring.
if (range_data->start >= current_queue_item.start_index_ +
current_queue_item.num_characters_ ||
range_data->end <= current_queue_item.start_index_)
continue;
}
}
......@@ -856,6 +863,7 @@ void HarfBuzzShaper::ShapeSegment(RangeData* range_data,
unsigned shape_end =
std::min(range_data->end, current_queue_item.start_index_ +
current_queue_item.num_characters_);
DCHECK_GT(shape_end, shape_start);
CaseMappingHarfBuzzBufferFiller(
case_map_intend, font_description.LocaleOrDefault(), range_data->buffer,
......
......@@ -391,6 +391,47 @@ TEST_F(HarfBuzzShaperTest, ShapeVerticalUpright) {
EXPECT_EQ(result->Bounds(), composite_result->Bounds());
}
TEST_F(HarfBuzzShaperTest, RangeShapeSmallCaps) {
// Test passes if no assertion is hit of the ones below, but also the newly
// introduced one in HarfBuzzShaper::ShapeSegment: DCHECK_GT(shape_end,
// shape_start) is not hit.
FontDescription font_description;
font_description.SetVariantCaps(FontDescription::kSmallCaps);
font_description.SetComputedSize(12.0);
Font font(font_description);
font.Update(nullptr);
// Shaping index 2 to 3 means that case splitting for small caps splits before
// character index 2 since the initial 'a' needs to be uppercased, but the
// space character does not need to be uppercased. This triggered
// crbug.com/817271.
String string(u"a aa");
HarfBuzzShaper shaper(string.Characters16(), string.length());
scoped_refptr<ShapeResult> result =
shaper.Shape(&font, TextDirection::kLtr, 2, 3);
EXPECT_EQ(1u, result->NumCharacters());
string = u"aa a";
HarfBuzzShaper shaper_two(string.Characters16(), string.length());
result = shaper_two.Shape(&font, TextDirection::kLtr, 3, 4);
EXPECT_EQ(1u, result->NumCharacters());
string = u"a aa";
HarfBuzzShaper shaper_three(string.Characters16(), string.length());
result = shaper_three.Shape(&font, TextDirection::kLtr, 1, 2);
EXPECT_EQ(1u, result->NumCharacters());
string = u"aa aa aa aa aa aa aa aa aa aa";
HarfBuzzShaper shaper_four(string.Characters16(), string.length());
result = shaper_four.Shape(&font, TextDirection::kLtr, 21, 23);
EXPECT_EQ(2u, result->NumCharacters());
string = u"aa aa aa aa aa aa aa aa aa aa";
HarfBuzzShaper shaper_five(string.Characters16(), string.length());
result = shaper_five.Shape(&font, TextDirection::kLtr, 27, 29);
EXPECT_EQ(2u, result->NumCharacters());
}
TEST_F(HarfBuzzShaperTest, ShapeVerticalMixed) {
font_description.SetOrientation(FontOrientation::kVerticalMixed);
font = Font(font_description);
......
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