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

Stop using |Font::BoundingBox| in |TextMetrics::Update|

As part of the work to update |TextMetrics::Update| for issue
1010893, the call to |Font::BoundingBox| turned out to be
unnecessary:
* |Width()| is the sum of advances, which is already comptued
  as |real_width|.
* |Y()| and |MaxY()| are the same as |glyph_bounds|, except
  that |Font::BoundingBox| clamps them to 0.

This patch replaces the call to |Font::BoundingBox| with the
equivalent, already computed values.

This is to help supporting bidi in |TextMetrics::Update|.
WIP: https://crbug.com/c/2196086
but also reduces call to CachingWordShaper and improves the
performance.

Note that the clamping behavior is not copied. It is most
likely unintentional and unlikely to cause compat problems.

Currently, this function is the only caller of
|Font::BoundingBox|. We can remove it once this change is
stabilized.

Bug: 1010893
Change-Id: I79d255e1a308e5da501e65a95981c6f13148a57f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2196624Reviewed-by: default avatarYi Xu <yiyix@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771497}
parent d6e67531
...@@ -68,15 +68,25 @@ void TextMetrics::Update(const Font& font, ...@@ -68,15 +68,25 @@ void TextMetrics::Update(const Font& font,
TextRun::kAllowTrailingExpansion | TextRun::kForbidLeadingExpansion, TextRun::kAllowTrailingExpansion | TextRun::kForbidLeadingExpansion,
direction, false); direction, false);
text_run.SetNormalizeSpace(true); text_run.SetNormalizeSpace(true);
FloatRect bbox = font.BoundingBox(text_run);
const FontMetrics& font_metrics = font_data->GetFontMetrics(); const FontMetrics& font_metrics = font_data->GetFontMetrics();
advances_ = font.IndividualCharacterAdvances(text_run); advances_ = font.IndividualCharacterAdvances(text_run);
// x direction // x direction
width_ = bbox.Width();
FloatRect glyph_bounds; FloatRect glyph_bounds;
double real_width = font.Width(text_run, nullptr, &glyph_bounds); double real_width = font.Width(text_run, nullptr, &glyph_bounds);
#if DCHECK_IS_ON()
// This DCHECK is for limited time only; to use |glyph_bounds| instead of
// |BoundingBox| and make sure they are compatible.
FloatRect bbox = font.BoundingBox(text_run);
// |GetCharacterRange|, the underlying function of |BoundingBox|, clamps
// negative |MaxY| to 0. This is unintentional, and that we are not copying
// the behavior.
DCHECK_EQ(bbox.Y(), std::min(glyph_bounds.Y(), .0f));
DCHECK_EQ(bbox.MaxY(), std::max(glyph_bounds.MaxY(), .0f));
DCHECK_EQ(bbox.Width(), real_width);
#endif
width_ = real_width;
float dx = 0.0f; float dx = 0.0f;
if (align == kCenterTextAlign) if (align == kCenterTextAlign)
...@@ -94,8 +104,8 @@ void TextMetrics::Update(const Font& font, ...@@ -94,8 +104,8 @@ void TextMetrics::Update(const Font& font,
const float baseline_y = GetFontBaseline(baseline, *font_data); const float baseline_y = GetFontBaseline(baseline, *font_data);
font_bounding_box_ascent_ = ascent - baseline_y; font_bounding_box_ascent_ = ascent - baseline_y;
font_bounding_box_descent_ = descent + baseline_y; font_bounding_box_descent_ = descent + baseline_y;
actual_bounding_box_ascent_ = -bbox.Y() - baseline_y; actual_bounding_box_ascent_ = -glyph_bounds.Y() - baseline_y;
actual_bounding_box_descent_ = bbox.MaxY() + baseline_y; actual_bounding_box_descent_ = glyph_bounds.MaxY() + baseline_y;
em_height_ascent_ = font_data->EmHeightAscent() - baseline_y; em_height_ascent_ = font_data->EmHeightAscent() - baseline_y;
em_height_descent_ = font_data->EmHeightDescent() + baseline_y; em_height_descent_ = font_data->EmHeightDescent() + baseline_y;
......
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