Commit 751a970a authored by Yoshifumi Inoue's avatar Yoshifumi Inoue Committed by Commit Bot

Make NGInlineNodeEditor to produce proper NGInlineItem and ShapeResult

This patch changes |NGInlineNodeEditor| to produce |NGInlineItem| with
|ShapeResult| having same start and end offset for valid text painting.

Before this patch, we see wrong painting, e.g. "AxXX" instead of "AxVX"
when insert "x" before "X" of "AVX". This is cause by kerning between
"A" and "V".

Bug: 1121885
Change-Id: I8242356ef6b865f2ad8b2e47de62d3200286e3f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391701
Commit-Queue: Koji Ishii <kojii@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804257}
parent 7d66f340
...@@ -598,8 +598,14 @@ class NGInlineNodeDataEditor final { ...@@ -598,8 +598,14 @@ class NGInlineNodeDataEditor final {
DCHECK_EQ(it->layout_object_, layout_text_); DCHECK_EQ(it->layout_object_, layout_text_);
// Copy part of item before replaced range. // Copy part of item before replaced range.
if (it->start_offset_ < start_offset) if (it->start_offset_ < start_offset) {
items.push_back(CopyItemBefore(*it, start_offset)); const NGInlineItem& new_item = CopyItemBefore(*it, start_offset);
items.push_back(new_item);
if (new_item.EndOffset() < start_offset) {
items.push_back(
NGInlineItem(*it, new_item.EndOffset(), start_offset, nullptr));
}
}
// Skip items in replaced range. // Skip items in replaced range.
while (it->end_offset_ < end_offset) while (it->end_offset_ < end_offset)
...@@ -621,7 +627,13 @@ class NGInlineNodeDataEditor final { ...@@ -621,7 +627,13 @@ class NGInlineNodeDataEditor final {
// Copy part of item after replaced range. // Copy part of item after replaced range.
if (end_offset < it->end_offset_) { if (end_offset < it->end_offset_) {
items.push_back(CopyItemAfter(*it, end_offset)); const NGInlineItem& new_item = CopyItemAfter(*it, end_offset);
if (end_offset < new_item.StartOffset()) {
items.push_back(
NGInlineItem(*it, end_offset, new_item.StartOffset(), nullptr));
ShiftItem(&items.back(), diff);
}
items.push_back(new_item);
ShiftItem(&items.back(), diff); ShiftItem(&items.back(), diff);
} }
...@@ -676,7 +688,7 @@ class NGInlineNodeDataEditor final { ...@@ -676,7 +688,7 @@ class NGInlineNodeDataEditor final {
if (end_offset == safe_start_offset) if (end_offset == safe_start_offset)
return NGInlineItem(item, start_offset, end_offset, nullptr); return NGInlineItem(item, start_offset, end_offset, nullptr);
return NGInlineItem( return NGInlineItem(
item, start_offset, end_offset, item, safe_start_offset, end_offset,
item.shape_result_->SubRange(safe_start_offset, end_offset)); item.shape_result_->SubRange(safe_start_offset, end_offset));
} }
...@@ -700,7 +712,7 @@ class NGInlineNodeDataEditor final { ...@@ -700,7 +712,7 @@ class NGInlineNodeDataEditor final {
if (start_offset == safe_end_offset) if (start_offset == safe_end_offset)
return NGInlineItem(item, start_offset, end_offset, nullptr); return NGInlineItem(item, start_offset, end_offset, nullptr);
return NGInlineItem( return NGInlineItem(
item, start_offset, end_offset, item, start_offset, safe_end_offset,
item.shape_result_->SubRange(start_offset, safe_end_offset)); item.shape_result_->SubRange(start_offset, safe_end_offset));
} }
...@@ -715,28 +727,24 @@ class NGInlineNodeDataEditor final { ...@@ -715,28 +727,24 @@ class NGInlineNodeDataEditor final {
item->shape_result_->CopyAdjustedOffset(item->start_offset_); item->shape_result_->CopyAdjustedOffset(item->start_offset_);
} }
// TODO(yosin): Once we can reproduce invalid |ShapeResult| offsets, we
// should make this function works only for |DCHECK_IS_ON()|.
void VerifyItems(const Vector<NGInlineItem>& items) const { void VerifyItems(const Vector<NGInlineItem>& items) const {
#if DCHECK_IS_ON()
if (items.IsEmpty()) if (items.IsEmpty())
return; return;
unsigned last_offset = items.front().start_offset_; unsigned last_offset = items.front().start_offset_;
for (const NGInlineItem& item : items) { for (const NGInlineItem& item : items) {
CHECK_LE(item.start_offset_, item.end_offset_); DCHECK_LE(item.start_offset_, item.end_offset_);
CHECK_EQ(last_offset, item.start_offset_); DCHECK_EQ(last_offset, item.start_offset_);
last_offset = item.end_offset_; last_offset = item.end_offset_;
if (!item.shape_result_ || item.layout_object_ != layout_text_) if (!item.shape_result_ || item.layout_object_ != layout_text_)
continue; continue;
CHECK_LT(item.start_offset_, item.end_offset_); DCHECK_LT(item.start_offset_, item.end_offset_);
if (item.shape_result_->StartIndex() == item.start_offset_) { DCHECK_EQ(item.shape_result_->StartIndex(), item.start_offset_);
CHECK_LE(item.shape_result_->EndIndex(), item.end_offset_); DCHECK_EQ(item.shape_result_->EndIndex(), item.end_offset_);
} else {
CHECK_LE(item.start_offset_, item.shape_result_->StartIndex());
CHECK_EQ(item.end_offset_, item.shape_result_->EndIndex());
}
} }
CHECK_EQ(last_offset, DCHECK_EQ(last_offset,
block_flow_->GetNGInlineNodeData()->text_content.length()); block_flow_->GetNGInlineNodeData()->text_content.length());
#endif
} }
std::unique_ptr<NGInlineNodeData> data_; std::unique_ptr<NGInlineNodeData> data_;
......
<!doctype html>
<div id="target">AVX</div>
<script>
document.body.offsetHeight;
const text = target.firstChild;
text.insertData(1, 'x');
</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