915938 - Inconsistent baseline for text in canvases

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

Attachment: device-2013-09-12-172134.png

The baseline of the text in this canvas is inconsistent. I have seen this in other canvases as well.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e3c19a339b36&tochange=5ceea82a79c7

Pref on SkiaGL looks suspicious. Though there are a number of Graphics changes in that regression range. Will do some additional bisecting.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Suspicion confirmed.

The first bad revision is:
changeset:   139475:bdd1a266176f
user:        James Willcox <[e-mail]>
date:        Fri Jul 19 14:03:46 2013 -0400
summary:     Bug 895409 - Enable SkiaGL canvas on Android r=blassey

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

On Galaxy Nexus I don't get any text at all in the top right box. Strange. We should fix this and get a mochitest in that covers it.

George can you see if this is reproducible with the rebase. If so, please fix :)

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

This is on a Nexus 7 2013 (Adreno 320) and a HTC One X+ (nVIDIA Tegra 3) both running Android 4.3.

Comment 5

[George Wright (:gw280) (needinfo me!)]

Attachment: honour-hinting.patch

This bug is due to hinting mismatches between layout and rendering. Basically, layout thinks we're unhinted (which we are), and skia thinks we're hinted (the default if no hint options are explicitly set). Grab our GlyphRenderingOptions from gfxPlatform in our canvas rendering context and pass it through to the DrawTarget in order to resolve this mismatch.

Comment 6

[George Wright (:gw280) (needinfo me!)]

Try run here: https://tbpl.mozilla.org/?tree=Try&rev=6d1cf6324942

Comment 7

[Jonathan Kew [:jfkthame]]

Comment on attachment 817199 [details] [diff] [review]
honour-hinting.patch

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

Looks reasonable to me. :)

Comment 8

[George Wright (:gw280) (needinfo me!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/48b08b13a29f

Comment 9

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 817199 [details] [diff] [review]
honour-hinting.patch

[Approval Request Comment]
Regression caused by (bug #): 848491
User impact if declined: Canvas text won't render correctly on Android
Testing completed (on m-c, etc.): Full try run, m-c
Risk to taking this patch (and alternatives if risky): Low risk, only synchronises hinting preferences in different parts of the codebase
String or IDL/UUID changes made by this patch: None

Comment 10

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 817199 [details] [diff] [review]
honour-hinting.patch

See previous comment. Accidentally flagged for release instead of aurora

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/48b08b13a29f

Comment 12

[Alex Keybl [:akeybl]]

Comment on attachment 817199 [details] [diff] [review]
honour-hinting.patch

Since this is low risk and a tracked regression, we'll take on branches. Thanks for getting this fix in.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/88ba0bbae1ce
https://hg.mozilla.org/releases/mozilla-beta/rev/7aa981fdbc96

Comment 14

[George Wright (:gw280) (needinfo me!)]

Hrm, so it seems we're failing tests on beta/aurora because we fuzzed a bunch of tests on m-c that aren't fuzzed on beta/aurora. We should uplift the patches from bug 888446 as well

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Had to increase the fuzz in text-not-in-doc-test.html a bit.
https://hg.mozilla.org/releases/mozilla-aurora/rev/2769f814a0bb
https://hg.mozilla.org/releases/mozilla-beta/rev/e2916694bdf2