Closed Bug 1090168 Opened 10 years ago Closed 10 years ago

improve baseline handling for <canvas> text with vertical writing mode

Categories

(Core :: Graphics: Text, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 2 obsolete files)

Currently, Canvas2D's fillText in vertical writing mode will draw the text vertically using a centered baseline that starts at the given (x, y) position.

However, I don't think this is appropriate in all cases; it should depend on the text-orientation property. According to CSS Writing Modes, "In vertical writing mode, the central baseline is used as the dominant baseline when text-orientation is mixed or upright. Otherwise the alphabetic baseline is used."[1]

So when text-orientation is 'mixed' (the default) or 'upright', it's appropriate to use a centered baseline starting at (x, y). But when text-orientation is 'sideways-right', so that all glyphs are rotated 90° clockwise, we should place the *alphabetic* baseline origin at (x, y) instead.

[1] http://www.w3.org/TR/css3-writing-modes/#text-baselines
Incidentally, I think this will mean that rendering text with vertical writing mode and text-orientation:sideways-right will be exactly equivalent to rendering horizontal text at the same origin with a 90° rotation. Which will be nicely reftest-able.

(Whereas if text-orientation is mixed, these are not equivalent because the vertical mode rendering will align the center baseline to the origin location.)
This makes the effective baseline dependent on the text-orientation value, as per CSS Writing Modes. We'll need to do something similar for HTML text, too, but that's a bit more complex and will be in its own bug.
Attachment #8512633 - Flags: review?(jdaggett)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Unsurprisingly, subpixel discrepancies on the rotated text may show up, so this needs a fuzzy() annotation, but nevertheless it provides a reasonable test for the basic functionality.
Attachment #8512638 - Flags: review?(jdaggett)
(In reply to Jonathan Kew (:jfkthame) from comment #0)

> So when text-orientation is 'mixed' (the default) or 'upright', it's
> appropriate to use a centered baseline starting at (x, y). But when
> text-orientation is 'sideways-right', so that all glyphs are rotated 90°
> clockwise, we should place the *alphabetic* baseline origin at (x, y)
> instead.

In this case the spec we should be referencing is not the Writing Modes spec but the canvas spec, specifically the text preparation algorithm.[1] Unfortunately that algorithm is not "vertical aware". The canvas context has a 'textBaseline' attribute and the algorithm states specifically how the anchor point relates to that baseline attribute. I think we should simply infer from writing-mode/text-orientation what the default for textBaseline should be. But allow authors to explicitly specify a different one.

[1] https://html.spec.whatwg.org/multipage/scripting.html#text-preparation-algorithm
(In reply to John Daggett (:jtd) from comment #4)
> (In reply to Jonathan Kew (:jfkthame) from comment #0)
> 
> > So when text-orientation is 'mixed' (the default) or 'upright', it's
> > appropriate to use a centered baseline starting at (x, y). But when
> > text-orientation is 'sideways-right', so that all glyphs are rotated 90°
> > clockwise, we should place the *alphabetic* baseline origin at (x, y)
> > instead.
> 
> In this case the spec we should be referencing is not the Writing Modes spec
> but the canvas spec, specifically the text preparation algorithm.[1]
> Unfortunately that algorithm is not "vertical aware". The canvas context has
> a 'textBaseline' attribute and the algorithm states specifically how the
> anchor point relates to that baseline attribute. I think we should simply
> infer from writing-mode/text-orientation what the default for textBaseline
> should be. But allow authors to explicitly specify a different one.

Seems fair enough... except that the spec says "the textBaseline attribute must initially have the value alphabetic", which doesn't appear to leave room for making it depend on the w-m/t-o values. As you say, that spec is not "vertical aware".
Comment on attachment 8512633 [details] [diff] [review]
Use proper baseline for vertical text in <canvas>, depending on value of text-orientation.

(In reply to Jonathan Kew (:jfkthame) from comment #5)

> Seems fair enough... except that the spec says "the textBaseline attribute
> must initially have the value alphabetic", which doesn't appear to leave
> room for making it depend on the w-m/t-o values. As you say, that spec is
> not "vertical aware".

Yeah, I think we should just treat the spec as the requirements for horizontal text and simply come up with what makes sense for a default for vertical. To work in vertical text, there probably needs to be canvas writing-mode/text-orientation text attributes but for now I think we can simply assume a sensible default for 'textBaseline' based on the CSS properties.

For now, I'm going to clear the reviews.
Attachment #8512633 - Flags: review?(jdaggett)
Attachment #8512638 - Flags: review?(jdaggett)
Depends on: 1093165
OK, this makes vertical text respect the textBaseline attribute just like horizontal text does.
Attachment #8516236 - Flags: review?(jdaggett)
And this changes the initial value of textBaseline to 'middle' for vertical text, except when orientation is sideways. (This depends on bug 1093165.)
Attachment #8516237 - Flags: review?(jdaggett)
Attachment #8512638 - Attachment is obsolete: true
Attachment #8512633 - Attachment is obsolete: true
Simple testcase that can be used to see how the various textBaseline values work with vertical writing mode and different text-orientation values.
Attachment #8516236 - Flags: review?(jdaggett) → review+
Attachment #8516237 - Flags: review?(jdaggett) → review+
Comment on attachment 8516240 [details] [diff] [review]
Reftests for textBaseline support in <canvas> vertical writing-mode text.

Review of attachment 8516240 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with use of Math.PI

Just curious, what's the reason for the use of the fuzziness? By the looks of this, the origin placement should match exactly. Why the jitter?

::: layout/reftests/writing-mode/1090168-1-notref.html
@@ +31,5 @@
> +// Testcase: vertical text with orientation:sideways-right
> +// test(100,  50, 'Hello', 'writing-mode:vertical-lr;text-orientation:sideways-right', 0);
> +
> +// Reference: horizontal text with 90° rotation
> +// test(100,  50, 'Hello', 'writing-mode:horizontal-tb', 3.141592653589793/2);

No need to insert pi like this, use Math.PI instead. Ditto for all the other instances of this constant.
Attachment #8516240 - Flags: review?(jdaggett) → review+
(In reply to John Daggett (:jtd) from comment #11)
> Just curious, what's the reason for the use of the fuzziness? By the looks
> of this, the origin placement should match exactly. Why the jitter?

I don't know. It's the "cross-hair" that I'm drawing to show the origin, rather than the actual text, that exhibits the discrepancies. There's no visible shift in its position; just fractional variation in the color value of some of the antialiased edge pixels. The exact discrepancy (if any) depends on the graphics backend -- e.g. GDI vs D2D. Maybe the rotation on the context makes a difference to exactly where in the graphics system some pixel-snapping happens, or something like that.
(In reply to Jonathan Kew (:jfkthame) from comment #12)
> (In reply to John Daggett (:jtd) from comment #11)
> > Just curious, what's the reason for the use of the fuzziness? By the looks
> > of this, the origin placement should match exactly. Why the jitter?
> 
> I don't know. It's the "cross-hair" that I'm drawing to show the origin,
> rather than the actual text, that exhibits the discrepancies. There's no
> visible shift in its position; just fractional variation in the color value
> of some of the antialiased edge pixels.

Ok, sounds totally appropriate to apply fuzzy here then.
Actually, I think we can simply avoid the issue by drawing the origin cross-hair before rotating the context to draw the reference text. If that works as expected, I'll land it in that form and without the fuzz.
Depends on: 1099143
Backed out parts 1 and 2 (except the gfxFont.h change in part 1, which other bugs rely on) due to Nightly crashiness, see bug 1099143.

backout: https://hg.mozilla.org/mozilla-central/rev/a52bf59965a0

I didn't back out the reftest, as the writing-modes test directory isn't being run yet anyway.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/55957219134c
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Depends on: CVE-2018-12359
You need to log in before you can comment on or make changes to this bug.