1126373 - Disable silk on Jelly bean and lower android versions

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mason Chang [Inactive] [:mchang]]

There are hardware vsync issues in jelly bean and below devices. Disable silk on non kit-kat and above devices.

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Specify versions to enable silk

This patch modifies b2gpref.js to only enable silk for specific versions of android. We also enable it for ICS (version 15) since that's where we run the emulators and we don't support ICS devices anymore, so I think it's ok.

I didn't want to pref the call sites since once we start enabling silk on desktop, we'd have a super long chain of ifdefs. There's also no nice way that I've found to check if we're in an emulator, which I still want to keep enabled to test silk. For now, we can keep b2g.js updated with new versions until JB becomes end of life.

In follow up patches, I'll consolidate checking gfxPrefs with member variables.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8555384 [details] [diff] [review]
Specify versions to enable silk

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

Fair enough. I'd feel a little better if you had an #else clause where you explicitly set them to false, so that if/when we turn it on globally we don't accidentally turn it on in JellyBean as well.

Comment 3

[Mason Chang [Inactive] [:mchang]]

Attachment: Specify versions to enable silk

Carrying r+, updated with feedback from comment 2.

Comment 4

[:gerard-majax]

Mason, I tried the current status, i.e., vsync enabled on ICS for Nexus S, and it looks like it's doing more harm than good.

Comment 5

[Chris Lord [:cwiiis]]

The emulator test machines have separate pref files that can be set, right? Might be worth disabling this on everything below kk and enabling it specifically on the test machines?

Comment 6

[Chris Lord [:cwiiis]]

Comment on attachment 8555451 [details] [diff] [review]
Specify versions to enable silk

Applied:

17:55 <Cwiiis> I have the patch applied, but I didn't know how to test the pref immediately, so kinda forgot about it... Don't suppose you know off-hand?
17:56 <mchang> hmm, if you have adb logcat
17:56 <mchang> just scroll once
17:56 <mchang> wait a couple of seconds
17:56 <mchang> and scroll again
17:56 <mchang> if you see a error message that says non-uniform vsync interval
17:56 <mchang> its on
17:59 <Cwiiis> mchang: I don't see any error messages :)

Comment 7

[Mason Chang [Inactive] [:mchang]]

(In reply to Chris Lord [:cwiiis] from comment #5)
> The emulator test machines have separate pref files that can be set, right?
> Might be worth disabling this on everything below kk and enabling it
> specifically on the test machines?

I've been asking in #ateam and #b2g and spoke with a few people, we don't have any real way to test if we're on an emulator test machine. The emulators don't have their own specific config files. Mochitests and reftests have their own profiles, but that wouldn't cover xpcom / CPP unit tests. If we really push, we could get global test only profiles, but I'd still rather not have more config files.

I think since ICS is EOL, we will just explicitly say disable silk on those still on ICS.

Comment 8

[Mason Chang [Inactive] [:mchang]]

(In reply to Alexandre LISSY :gerard-majax from comment #4)
> Mason, I tried the current status, i.e., vsync enabled on ICS for Nexus S,
> and it looks like it's doing more harm than good.

Yeah, unfortunately there are hardware vsync bugs on kit-kat and below devices which make it worse :(.

Comment 9

[Mason Chang [Inactive] [:mchang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/54f9114eef86

Comment 10

[Mason Chang [Inactive] [:mchang]]

(In reply to Mason Chang [:mchang] from comment #8)
> (In reply to Alexandre LISSY :gerard-majax from comment #4)
> > Mason, I tried the current status, i.e., vsync enabled on ICS for Nexus S,
> > and it looks like it's doing more harm than good.
> 
> Yeah, unfortunately there are hardware vsync bugs on kit-kat and below
> devices which make it worse :(.

Should say on Jelly-Bean and below devices.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/54f9114eef86