581092 - [dwrite] REFTEST TEST-UNEXPECTED-FAIL | canvas/text-bidi-(ltr|rtl)-test.html |

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(Since those other D2D-reftest bugs all had Win7 file URIs in the testnames, I assume the correct platform for these two is also Win7.)

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: simplified testcase showing canvas bidi positioning discrepancy

The reftest failure here occurs because the canvas bidi code rounds the width of text segments to whole pixels, but with directwrite enabled, text run widths and glyph positions may be fractional.

The attached testcase illustrates the problem; there are two versions of "hello world" drawn to a canvas element, one as LTR and the other as RTL. Flipping between them in tabs, a half-pixel difference in positioning can clearly be seen.

Checking values in nsCanvasBidiProcessor::GetWidth(), we see that with GDI, the textRun's mAdvanceWidth is 5640 appunits, which is exactly 94 pixels; thus, the appunits-to-integer device pixels conversion at http://mxr.mozilla.org/mozilla-central/source/content/canvas/src/nsCanvasRenderingContext2D.cpp#2594 is precise, and the drawing of the right-to-left run is as expected.

With DWrite enabled, the textRun returns an advance width of 5731 appunits, which represents 95.516667 device pixels. But nsCanvasBidiProcessor::GetWidth() returns this as a truncated value of 95, and therefore when the run is drawn from right to left, it appears approximately half a pixel to the left of its LTR position.

To fix this, I think we should probably modify the canvas bidi code to use fractional pixel values for textrun widths and positions throughout the line layout process. An alternative might be to paint RTL text runs by reversing the glyphs and then painting in LTR order, so that the rounding to integer device pixels is consistently happening at the left end of text runs.

Comment 3

[Bas Schouten (:bas.schouten)]

Sounds like we need a modest change to canvas.

Comment 4

[Jonathan Kew [:jfkthame]]

It turns out that OS X suffers from this exact same problem, as Quartz also does subpixel text positioning, and these two reftests are currently marked as failing on OS X. The fix should resolve the test failure for both platforms.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1 - use non-rounded width when advancing to draw RTL glyph run

This seems to be the least-intrusive way to resolve the reftest failure on both OS X and Windows with DWrite.

We might want to consider using true subpixel metrics throughout the text layout process, but that would require a much more extensive patch and might have wider implications for canvas; I think if we want to look into that, we should treat it as a separate enhancement.

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 462154 [details] [diff] [review]
patch, v1 - use non-rounded width when advancing to draw RTL glyph run

Requesting approval to land on trunk as we need this to pass reftests on Windows-D2D. (It also fixes a current known-failure on OS X.) The patch seems pretty safe as the change is extremely local in its effect.

Comment 7

[Joe Drew (not getting mail)]

Heck, this blocks if we need it for reftests on D2D.

Comment 8

[Jonathan Kew [:jfkthame]]

http://hg.mozilla.org/mozilla-central/rev/4f4c7262afb5