Commit e680566d authored by Yoshifumi Inoue's avatar Yoshifumi Inoue Committed by Commit Bot

Make NGInlineNode::SetTextWithOffset() not to reuse last glyph of run

This patch changes |NGInlineNode::SetTextWithOffset()| not to reuse last
glyph in run to handle kergning, e.g. "AV".

When inserting character "V" after "A", before this patch, we have glyph
run for "A" and "V" without adjusting spaces between space "A" and "V"
because we reuse shaping result of "A" then concatenating shape result
of "V". After this patch, we does shaping "AV" once.

Bug: 1129710
Change-Id: I2ecf2ce04945dba18ed9622a9f6745ea4060ce87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423802
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811919}
parent 8f52ca70
......@@ -701,24 +701,33 @@ class NGInlineNodeDataEditor final {
DCHECK_LT(item.start_offset_, end_offset);
DCHECK_LE(end_offset, item.end_offset_);
DCHECK_EQ(item.layout_object_, layout_text_);
const unsigned safe_end_offset = GetLastSafeToReuse(item, end_offset);
const unsigned start_offset = item.start_offset_;
if (!item.shape_result_ || item.shape_result_->IsAppliedSpacing())
return NGInlineItem(item, start_offset, end_offset, nullptr);
if (item.end_offset_ == end_offset)
return item;
// TODO(yosin): We should handle |shape_result| doesn't have safe-to-break
// at start and end, because of |ShapeText()| splits |ShapeResult| ignoring
// safe-to-break offset.
item.shape_result_->EnsurePositionData();
const unsigned safe_end_offset =
item.shape_result_->CachedPreviousSafeToBreakOffset(end_offset);
if (start_offset == safe_end_offset)
return NGInlineItem(item, start_offset, end_offset, nullptr);
// To handle kerning, e.g. "AV", we should not reuse last glyph.
// See http://crbug.com/1129710
DCHECK_LT(safe_end_offset, item.end_offset_);
return NGInlineItem(
item, start_offset, safe_end_offset,
item.shape_result_->SubRange(start_offset, safe_end_offset));
}
unsigned GetLastSafeToReuse(const NGInlineItem& item,
unsigned end_offset) const {
DCHECK_LT(item.start_offset_, end_offset);
DCHECK_LE(end_offset, item.end_offset_);
DCHECK_EQ(item.layout_object_, layout_text_);
const unsigned start_offset = item.start_offset_;
if (!item.shape_result_ || item.shape_result_->IsAppliedSpacing() ||
end_offset - start_offset <= 1)
return start_offset;
item.shape_result_->EnsurePositionData();
// Note: Because |CachedPreviousSafeToBreakOffset()| assumes |end_offset|
// is always safe to break offset, we try to search before |end_offset|.
return item.shape_result_->CachedPreviousSafeToBreakOffset(end_offset - 1);
}
static void ShiftItem(NGInlineItem* item, int delta) {
if (delta == 0)
return;
......
<!doctype html>
<div>XA W AV A W AV A W AV A W AV</div>
<div>XA W AV A W AV A W AV A W AV</div>
<!doctype html>
<div id="target">X</div>
<div>XA W AV A W AV A W AV A W AV</div>
<script>
function type(code) {
target.firstChild.appendData(code);
document.body.offsetHeight;
}
for (let i = 0; i < 4; ++i) {
type('A');
type(' ');
type('W');
type(' ');
type('A');
type('V');
type(' ');
}
</script>
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