Commit 70ab0a94 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Fix gfx_unittests flakiness caused by negative x-offset

This CL is fixing a source of flakiness we observed acrross different
computer. The gfx_unittests are using SetGlyphWidth to avoid relying
on the underlying font information for testing some API. Unfortunately,
we often ovserved inconsistency accross different platform and blocked
many CL on incorrect behavior based on the following bug:

Within the ShapeRunWithFont(...) code, the x_offset may not be 0 even
if the glyph size is zero. It can also be negative for some fonts.

Looking to the failing bot, we got these values:
  Font: ".SF NS Text"
  Offset: is -0.0952148
  Glyph: 1216

To fix the issue, we proposed to keep the x_offset to zero when a
glyph_size is used. This only affects tests, otherwise no functionnal
change.

Bug: 1056220
Change-Id: I486722a5e64c1e4ee85aa373343a19c1a1ac1331
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2075079Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744782}
parent c83e0ed7
...@@ -1297,15 +1297,12 @@ void ShapeRunWithFont(const ShapeRunWithFontInput& in, ...@@ -1297,15 +1297,12 @@ void ShapeRunWithFont(const ShapeRunWithFontInput& in,
out->positions.resize(out->glyph_count); out->positions.resize(out->glyph_count);
out->width = 0.0f; out->width = 0.0f;
#if defined(OS_MACOSX) // Font on MAC like ".SF NS Text" may have a negative x_offset. Positive
// Mac 10.9 and 10.10 give a quirky offset for whitespace glyphs in RTL, // x_offset are also found on Windows (e.g. "Segoe UI"). It requires tests
// which requires tests relying on the behavior of |glyph_width_for_test_| // relying on the behavior of |glyph_width_for_test_| to also be given a zero
// to also be given a zero x_offset, otherwise expectations get thrown off. // x_offset, otherwise expectations get thrown off
const bool force_zero_offset = // (see: http://crbug.com/1056220).
in.glyph_width_for_test > 0 && base::mac::IsAtMostOS10_10(); const bool force_zero_offset = in.glyph_width_for_test > 0;
#else
constexpr bool force_zero_offset = false;
#endif
constexpr uint16_t kMissingGlyphId = 0; constexpr uint16_t kMissingGlyphId = 0;
out->missing_glyph_count = 0; out->missing_glyph_count = 0;
......
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