Commit e5d4d080 authored by David Quiroz Marin's avatar David Quiroz Marin Committed by Commit Bot

Fix textBaseline alignment to match the spec for top/bottom/middle.

Our current implementation of canvas textBaseline alignment for top
middle and bottom doesn't do what the spec says it should, i.e. align
with the EM square.
https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-textbaseline
This has been a bug on FF side for quite some years:
https://bugzilla.mozilla.org/show_bug.cgi?id=737852
This change fixes this misalignment by using the EM square ascent
and descent from SimpleFontData instead of the font's max ascent/descent.
Also fixing the EM related metrics in canvas TextMetrics.

Bug:607053,277215

Change-Id: Id6374e6fcd4e46fa989a7e23a80dc1f5bf961b94
Reviewed-on: https://chromium-review.googlesource.com/1194494
Commit-Queue: David Quiroz Marin <davidqu@chromium.org>
Reviewed-by: default avatarFernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587855}
parent 32c9ffb4
......@@ -31,11 +31,13 @@ step_timeout(t.step_func_done(function () {
ctx.font = '50px CanvasTest';
ctx.direction = 'ltr';
ctx.align = 'left'
_assertSame(ctx.measureText('A').emHeightAscent, 124, "ctx.measureText('A').emHeightAscent", "124");
_assertSame(Math.abs(ctx.measureText('A').emHeightDescent), 0, "Math.abs(ctx.measureText('A').emHeightDescent)", "0");
_assertSame(ctx.measureText('A').emHeightAscent, 37.5, "ctx.measureText('A').emHeightAscent", "37.5");
_assertSame(ctx.measureText('A').emHeightDescent, 12.5, "ctx.measureText('A').emHeightDescent", "12.5");
_assertSame(ctx.measureText('A').emHeightDescent + ctx.measureText('A').emHeightAscent, 50, "ctx.measureText('A').emHeightDescent + ctx.measureText('A').emHeightAscent", "50");
_assertSame(ctx.measureText('ABCD').emHeightAscent, 124, "ctx.measureText('ABCD').emHeightAscent", "124");
_assertSame(Math.abs(ctx.measureText('ABCD').emHeightDescent), 0, "Math.abs(ctx.measureText('ABCD').emHeightDescent)", "0");
_assertSame(ctx.measureText('ABCD').emHeightAscent, 37.5, "ctx.measureText('ABCD').emHeightAscent", "37.5");
_assertSame(ctx.measureText('ABCD').emHeightDescent, 12.5, "ctx.measureText('ABCD').emHeightDescent", "12.5");
_assertSame(ctx.measureText('ABCD').emHeightDescent + ctx.measureText('ABCD').emHeightAscent, 50, "ctx.measureText('ABCD').emHeightDescent + ctx.measureText('ABCD').emHeightAscent", "50");
}), 500);
......
This is a testharness.js-based test.
FAIL textBaseline bottom is the bottom of the em square (not the bounding box) assert_approx_equals: Red channel of the pixel at (25, 25) expected 0 +/- 2 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL textBaseline middle is the middle of the em square (not the bounding box) assert_approx_equals: Red channel of the pixel at (5, 5) expected 0 +/- 2 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL textBaseline top is the top of the em square (not the bounding box) assert_approx_equals: Red channel of the pixel at (5, 5) expected 0 +/- 2 but got 255
Harness: the test ran to completion.
......@@ -9,7 +9,6 @@
2d.fillRect: "drawing-rectangles-to-the-canvas"
2d.strokeRect: "drawing-rectangles-to-the-canvas"
2d.text.draw: "drawing-text-to-the-canvas"
2d.text.draw.baseline.alphabetic: "drawing-text-to-the-canvas"
2d.text.draw.space.basic: "drawing-text-to-the-canvas"
2d.text.draw.space.collapse: "drawing-text-to-the-canvas"
2d.text.measure: "drawing-text-to-the-canvas"
......
......@@ -1103,11 +1103,13 @@
ctx.font = '50px CanvasTest';
ctx.direction = 'ltr';
ctx.align = 'left'
@assert ctx.measureText('A').emHeightAscent === 124;
@assert Math.abs(ctx.measureText('A').emHeightDescent) === 0;
@assert ctx.measureText('A').emHeightAscent === 37.5;
@assert ctx.measureText('A').emHeightDescent === 12.5;
@assert ctx.measureText('A').emHeightDescent + ctx.measureText('A').emHeightAscent === 50;
@assert ctx.measureText('ABCD').emHeightAscent === 124;
@assert Math.abs(ctx.measureText('ABCD').emHeightDescent) === 0;
@assert ctx.measureText('ABCD').emHeightAscent === 37.5;
@assert ctx.measureText('ABCD').emHeightDescent === 12.5;
@assert ctx.measureText('ABCD').emHeightDescent + ctx.measureText('ABCD').emHeightAscent === 50;
}), 500);
- name: 2d.text.measure.baselines
......
......@@ -25,7 +25,7 @@ function testTextMetrics(textMetrics, expected)
var aa = textMetrics.actualBoundingBoxAscent;
var ad = textMetrics.actualBoundingBoxDescent;
var epsilon = 2;
var epsilon = 3;
assert_approx_equals(w, expected[0], epsilon, "testing width");
assert_approx_equals(ba, expected[1], epsilon, "testing getBaselines().alphabetic");
assert_approx_equals(bh, expected[2], epsilon, "testing getBaselines().hanging");
......@@ -46,32 +46,32 @@ function measureMetrics(ctx)
ctx.textBaseline = "top";
var textMetrics = ctx.measureText(text);
var expected = [249, -44, -8.8, -57, 0, 57, 13, 44, -0, 249, -9, 46];
var expected = [249,-39,-3,-51,6,51,0,49,0,250,-4,41];
testTextMetrics(textMetrics, expected);
ctx.textBaseline = "hanging";
var textMetrics = ctx.measureText(text);
expected = [249, -35, 0.2, -48, 9, 48, 22, 35, -0, 249, 0, 37];
expected = [249,-36,0,-48,9,48,3,46,0,250,-1,38];
testTextMetrics(textMetrics, expected);
ctx.textBaseline = "middle";
var textMetrics = ctx.measureText(text);
expected = [249, -15, 20.2, -28, 29, 28, 42, 15, -0, 249, 20, 17];
expected = [249,-14,22,-26,31,26,25,24,0,250,21,16];
testTextMetrics(textMetrics, expected);
ctx.textBaseline = "alphabetic";
var textMetrics = ctx.measureText(text);
expected = [249, -0, 35.2, -13, 44, 13, 57, 0, -0, 249, 35, 2];
expected = [249,0,36,-12,45,12,39,10,0,250,35,2];
testTextMetrics(textMetrics, expected);
ctx.textBaseline = "ideographic";
var textMetrics = ctx.measureText(text);
expected = [249, 13, 48.2, 0, 57, 0, 70, -13, -0, 249, 48, -11];
expected = [249,12,48,0,57,0,51,-2,0,250,47,-10];
testTextMetrics(textMetrics, expected);
ctx.textBaseline = "bottom";
var textMetrics = ctx.measureText(text);
expected = [249, 13, 48.2, 0, 57, 0, 70, -13, -0, 249, 48, -11];
expected = [249,11,47,-1,56,1,50,-1,0,250,46,-9];
testTextMetrics(textMetrics, expected);
}
......
......@@ -11,19 +11,19 @@ namespace blink {
constexpr int kHangingAsPercentOfAscent = 80;
float TextMetrics::GetFontBaseline(const TextBaseline& text_baseline,
const FontMetrics& font_metrics) {
const SimpleFontData& font_data) {
FontMetrics font_metrics = font_data.GetFontMetrics();
// If the font is so tiny that the lroundf operations result in two
// different types of text baselines to return the same baseline, use
// floating point metrics (crbug.com/338908).
// If you changed the heuristic here, for consistency please also change it
// in SimpleFontData::platformInit().
// TODO(fserb): revisit this.
bool use_float_ascent_descent =
font_metrics.Ascent() < 3 || font_metrics.Height() < 2;
bool use_float_ascent_descent = font_data.EmHeightAscent().ToInt() < 4;
switch (text_baseline) {
case kTopTextBaseline:
return use_float_ascent_descent ? font_metrics.FloatAscent()
: font_metrics.Ascent();
return use_float_ascent_descent
? font_data.EmHeightAscent().ToFloat()
: lround(font_data.EmHeightAscent().ToFloat());
case kHangingTextBaseline:
// According to
// http://wiki.apache.org/xmlgraphics-fop/LineLayout/AlignmentHandling
......@@ -32,15 +32,21 @@ float TextMetrics::GetFontBaseline(const TextBaseline& text_baseline,
return use_float_ascent_descent
? font_metrics.FloatAscent() * kHangingAsPercentOfAscent / 100
: font_metrics.Ascent() * kHangingAsPercentOfAscent / 100;
case kBottomTextBaseline:
case kIdeographicTextBaseline:
return use_float_ascent_descent ? -font_metrics.FloatDescent()
: -font_metrics.Descent();
case kBottomTextBaseline:
return use_float_ascent_descent
? -font_data.EmHeightDescent().ToFloat()
: -lround(font_data.EmHeightDescent().ToFloat());
case kMiddleTextBaseline:
return use_float_ascent_descent
? -font_metrics.FloatDescent() +
font_metrics.FloatHeight() / 2.0f
: -font_metrics.Descent() + font_metrics.Height() / 2;
? (-font_data.EmHeightDescent() + font_data.EmHeightAscent())
.ToFloat() /
2.0f
: (-lround(font_data.EmHeightDescent().ToFloat()) +
lround(font_data.EmHeightAscent().ToFloat())) /
2;
case kAlphabeticTextBaseline:
default:
// Do nothing.
......@@ -86,18 +92,13 @@ void TextMetrics::Update(const Font& font,
// y direction
const float ascent = font_metrics.FloatAscent();
const float descent = font_metrics.FloatDescent();
const float baseline_y = GetFontBaseline(baseline, font_metrics);
const float baseline_y = GetFontBaseline(baseline, *font_data);
font_bounding_box_ascent_ = ascent - baseline_y;
font_bounding_box_descent_ = descent + baseline_y;
actual_bounding_box_ascent_ = -bbox.Y() - baseline_y;
actual_bounding_box_descent_ = bbox.MaxY() + baseline_y;
// it's not clear where the baseline for the em rect is.
// We could try to render a letter that has 1em height and try to figure out.
// But for now, just ignore descent for em.
em_height_ascent_ = font_metrics.Height() - baseline_y;
em_height_descent_ = baseline_y;
em_height_ascent_ = font_data->EmHeightAscent() - baseline_y;
em_height_descent_ = font_data->EmHeightDescent() + baseline_y;
// TODO(fserb): hanging/ideographic baselines are broken.
baselines_.setAlphabetic(-baseline_y);
......
......@@ -66,7 +66,7 @@ class CORE_EXPORT TextMetrics final : public ScriptWrappable {
void getBaselines(Baselines& baselines) const { baselines = baselines_; }
Baselines getBaselines() const { return baselines_; }
static float GetFontBaseline(const TextBaseline&, const FontMetrics&);
static float GetFontBaseline(const TextBaseline&, const SimpleFontData&);
private:
void Update(const Font&,
......
......@@ -1975,9 +1975,8 @@ void BaseRenderingContext2D::CheckOverdraw(
}
float BaseRenderingContext2D::GetFontBaseline(
const FontMetrics& font_metrics) const {
return TextMetrics::GetFontBaseline(GetState().GetTextBaseline(),
font_metrics);
const SimpleFontData& font_data) const {
return TextMetrics::GetFontBaseline(GetState().GetTextBaseline(), font_data);
}
String BaseRenderingContext2D::textAlign() const {
......
......@@ -372,7 +372,7 @@ class MODULES_EXPORT BaseRenderingContext2D : public GarbageCollectedMixin,
virtual void NeedsFinalizeFrame(){};
float GetFontBaseline(const FontMetrics&) const;
float GetFontBaseline(const SimpleFontData&) const;
static const char kDefaultFont[];
static const char kInheritDirectionString[];
......
......@@ -841,7 +841,7 @@ void CanvasRenderingContext2D::DrawTextInternal(
text_run.SetNormalizeSpace(true);
// Draw the item text at the correct point.
FloatPoint location(clampTo<float>(x),
clampTo<float>(y + GetFontBaseline(font_metrics)));
clampTo<float>(y + GetFontBaseline(*font_data)));
double font_width = font.Width(text_run);
bool use_max_width = (max_width && *max_width < font_width);
......
......@@ -418,7 +418,7 @@ void OffscreenCanvasRenderingContext2D::DrawTextInternal(
false);
text_run.SetNormalizeSpace(true);
// Draw the item text at the correct point.
FloatPoint location(x, y + GetFontBaseline(font_metrics));
FloatPoint location(x, y + GetFontBaseline(*font_data));
double font_width = font.Width(text_run);
bool use_max_width = (max_width && *max_width < font_width);
......
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