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

Make NGInlineNode::ShapeText() not to reuse ShapeResult for letter-spacing

This patch changes |NGInlineNode::ShapeText()| not to reuse
|ShapeResult| for text with CSS property "letter-spacing" because we
modify |ShapeResult| directly to apply letter spacing. Thus, letter-
spacing is applied more than once on reused portion of |ShapeResult|.

This is reland of original commit. There are two changes to fix
revert resones:
 1. Size of ShapeResult is increased => reduce bit size of
    |num_glyphs_|[1]
 2. Newly added member is not initialized in copy constructor[2]


[1] http://crrev.com/c/2390615
[2] http://crrev.com/c/2392577

Bug: 1124446, 1124740
Change-Id: Ifef96781528b03944be752303b1ce1edca5ff92e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2397105
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@{#804918}
parent 98dabd9c
......@@ -231,6 +231,8 @@ class ReusingTextShaper final {
continue;
if (!item->TextShapeResult() || item->Direction() != direction)
continue;
if (item->TextShapeResult()->IsAppliedSpacing())
continue;
shape_results.push_back(item->TextShapeResult());
}
return shape_results;
......
......@@ -380,7 +380,8 @@ ShapeResult::ShapeResult(scoped_refptr<const SimpleFontData> font_data,
num_characters_(num_characters),
num_glyphs_(0),
direction_(static_cast<unsigned>(direction)),
has_vertical_offsets_(0) {}
has_vertical_offsets_(false),
is_applied_spacing_(false) {}
ShapeResult::ShapeResult(const Font* font,
unsigned start_index,
......@@ -396,7 +397,8 @@ ShapeResult::ShapeResult(const ShapeResult& other)
num_characters_(other.num_characters_),
num_glyphs_(other.num_glyphs_),
direction_(other.direction_),
has_vertical_offsets_(other.has_vertical_offsets_) {
has_vertical_offsets_(other.has_vertical_offsets_),
is_applied_spacing_(other.is_applied_spacing_) {
runs_.ReserveCapacity(other.runs_.size());
for (const auto& run : other.runs_)
runs_.push_back(run->Create(*run.get()));
......@@ -902,6 +904,7 @@ void ShapeResult::ApplySpacingImpl(
void ShapeResult::ApplySpacing(ShapeResultSpacing<String>& spacing,
int text_start_offset) {
is_applied_spacing_ = true;
ApplySpacingImpl(spacing, text_start_offset);
}
......
......@@ -185,6 +185,9 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
// have vertical offsets.
bool HasVerticalOffsets() const { return has_vertical_offsets_; }
// Note: We should not reuse |ShapeResult| if we call |ApplySpacing()|.
bool IsAppliedSpacing() const { return is_applied_spacing_; }
// For memory reporting.
size_t ByteSize() const;
......@@ -490,7 +493,7 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
unsigned start_index_;
unsigned num_characters_;
unsigned num_glyphs_ : 30;
unsigned num_glyphs_ : 29;
// Overall direction for the TextRun, dictates which order each individual
// sub run (represented by RunInfo structs in the m_runs vector) can have a
......@@ -500,6 +503,12 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
// Tracks whether any runs contain glyphs with a y-offset != 0.
unsigned has_vertical_offsets_ : 1;
// True once called |ApplySpacing()|.
unsigned is_applied_spacing_ : 1;
// Note: When you add more bit flags, please consider to reduce size of
// |num_glyphs_| or |num_characters_|.
private:
friend class HarfBuzzShaper;
friend class ShapeResultBuffer;
......
<!doctype html>
<script src="../../resources/ahem.js"></script>
<style>
#target {
font: 10px/15px Ahem;
letter-spacing: 20px;
}
</style>
<div id="target">abcx</div>
<!doctype html>
<script src="../../resources/ahem.js"></script>
<style>
#target {
font: 10px/15px Ahem;
letter-spacing: 20px;
}
</style>
<div id="target">x</div>
<script>
const text = target.firstChild;
function typing(character) {
text.replaceData(0, 0, character);
document.body.offsetHeight;
}
typing('a');
typing('b');
typing('c');
</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