make gfxFont::Measure always lay out glyphs left to right
Categories
(Core :: Layout: Text and Fonts, enhancement, P5)
Tracking
()
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: heycam, Assigned: heycam)
References
Details
Attachments
(2 files, 1 obsolete file)
Assignee | ||
Comment 1•6 years ago
|
||
Updated•6 years ago
|
Comment 2•6 years ago
|
||
The trouble here is that while it's fine to conceptually lay out the glyphs LTR for the purpose of accumulating advances to figure out the total width of the glyph run, so we wouldn't need to negate advances when updating the current position, etc., this doesn't work for the other thing gfxFont::Measure does, which is to accumulate the overall bounding box.
Consider two glyphs [x] and [y]. Suppose they each have an advance of 100px. The "ink" of each glyph entirely fills its advance width; glyph [x] also has a swash tail that extends a further 50px beyond its advance.
When we iterate LTR over the glyph sequence [x][y], accumulating widths and bounding box, the total advance will be 200px, and the bounding box will extend from 0 to 200px; the "tail" of [x] extends half-way across the width of [y], but is subsumed within its overall bbox when we Union the rects.
If the glyphs are laid out RTL, the total advance will still be 200px (now measured in the leftward direction, so the x-coordinate will become -200px during the iteration; we then reverse the sign again to return the positive width of the run). But the bounding box should now be 250px wide, because the swash tail of [x] projects to the right of the origin, while the advances of the glyphs move to the left. It no longer overlaps the advance of [y]; instead, it extends the overall bbox to the other side of the origin.
So it doesn't work to simply do the bounding-box computation in LTR mode; running RTL affects not only the origin of the bounds (which we then reset, as noted) but also the overall width.
To handle this, we basically need to reflect the bounding box of each glyph in runs that are really RTL (although we're measuring as LTR), so that the left and right sidebearings are swapped; then the merging of glyph bounding boxes to form the overall bounds will be correct (modulo reflection); at the end, we can reverse this transformation to get the correct final bounds.
Pushed a new try job to see how this fares:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc3576dcf9940497a5d54476019f74a70a2b5674
Comment 3•6 years ago
|
||
Tryserver says... nice effort, but not quite.
Take #2: https://treeherder.mozilla.org/#/jobs?repo=try&revision=480406965d1290f755713eaea913ac1df21b9f92
Comment 4•6 years ago
|
||
OK, I've created a couple of extended versions of bugs/1116480-1-fakeitalic-overflow, which tends to fail if we get the RTL bounding box wrong; the new tests add a bunch of delta-positioned DetailedGlyph components, as well as a new reftest based on the Awami example from https://phabricator.services.mozilla.com/D14227#355901. This should give us some confidence we're not breaking things here.
With the new patch, we eliminate all the direction * ...
multiplications in Measure; the only bidi-related work left is a couple of short if (isRTL) ...
fragments to account for the flipping of bounding boxes as discussed in comment 2.
Comment 5•6 years ago
|
||
Comment 6•6 years ago
|
||
Depends on D38298
Updated•6 years ago
|
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c949fbdda08f
https://hg.mozilla.org/mozilla-central/rev/6b47fb297b39
Description
•