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)
Core
Graphics: Text
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jfkthame, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 2 obsolete files)
(deleted),
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/html
|
Details |
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
Assignee | ||
Comment 1•10 years ago
|
||
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.)
Assignee | ||
Comment 2•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
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)
Comment 4•10 years ago
|
||
(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
Assignee | ||
Comment 5•10 years ago
|
||
(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 6•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8512638 -
Flags: review?(jdaggett)
Assignee | ||
Comment 7•10 years ago
|
||
OK, this makes vertical text respect the textBaseline attribute just like horizontal text does.
Attachment #8516236 -
Flags: review?(jdaggett)
Assignee | ||
Comment 8•10 years ago
|
||
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)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8516240 -
Flags: review?(jdaggett)
Assignee | ||
Updated•10 years ago
|
Attachment #8512638 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8512633 -
Attachment is obsolete: true
Assignee | ||
Comment 10•10 years ago
|
||
Simple testcase that can be used to see how the various textBaseline values work with vertical writing mode and different text-orientation values.
Updated•10 years ago
|
Attachment #8516236 -
Flags: review?(jdaggett) → review+
Updated•10 years ago
|
Attachment #8516237 -
Flags: review?(jdaggett) → review+
Comment 11•10 years ago
|
||
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+
Assignee | ||
Comment 12•10 years ago
|
||
(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.
Comment 13•10 years ago
|
||
(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.
Assignee | ||
Comment 14•10 years ago
|
||
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.
Assignee | ||
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fad675db4598
https://hg.mozilla.org/integration/mozilla-inbound/rev/618d8f44d9e2
https://hg.mozilla.org/integration/mozilla-inbound/rev/7abada342a01
Target Milestone: --- → mozilla36
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fad675db4598
https://hg.mozilla.org/mozilla-central/rev/618d8f44d9e2
https://hg.mozilla.org/mozilla-central/rev/7abada342a01
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 17•10 years ago
|
||
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 → ---
Assignee | ||
Comment 18•10 years ago
|
||
Relanded, with the fix from bug 1099143 folded in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/55957219134c
Comment 19•10 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Depends on: CVE-2018-12359
You need to log in
before you can comment on or make changes to this bug.
Description
•