Closed Bug 831438 Opened 8 years ago Closed 8 years ago

Add pref for forcing 16bit on B2G

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
mozilla21
blocking-b2g -
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- fixed

People

(Reporter: jrmuizel, Unassigned)

Details

Attachments

(1 file)

No description provided.
Attached patch Do itSplinter Review
Attachment #702981 - Flags: review?(jones.chris.g)
Comment on attachment 702981 [details] [diff] [review]
Do it

Nit: "gfx.force-rgb16".  (These settings are really useful on desktop too when honored.)

r=me with that
Attachment #702981 - Flags: review?(jones.chris.g) → review+
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Comment on attachment 702981 [details] [diff] [review]
> Do it
> 
> Nit: "gfx.force-rgb16".  (These settings are really useful on desktop too
> when honored.)
> 
> r=me with that

The patch only applies to android though. Doing more is more complicated.
blocking-b2g: --- → tef?
Please re-nominate with justification, which is sorely missing here.
blocking-b2g: tef? → -
This is needed so that we can do testing with the 16 bit code paths in the dogfooding program. Currently, the plan is that we will ship on devices using 16bit, but the dogfooding program (unagi) is using the 32bit path. This means we're not testing the graphics path that we plan on shipping.
blocking-b2g: - → tef?
I think this should be approved and landed, but not a blocker as such.
Assignee: nobody → jmuizelaar
(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> > Comment on attachment 702981 [details] [diff] [review]
> > Do it
> > 
> > Nit: "gfx.force-rgb16".  (These settings are really useful on desktop too
> > when honored.)
> > 
> > r=me with that
> 
> The patch only applies to android though. Doing more is more complicated.

Just to be explicit, "Android" includes B2G?
(In reply to Milan Sreckovic [:milan] from comment #8)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> > (In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> > > Comment on attachment 702981 [details] [diff] [review]
> > > Do it
> > > 
> > > Nit: "gfx.force-rgb16".  (These settings are really useful on desktop too
> > > when honored.)
> > > 
> > > r=me with that
> > 
> > The patch only applies to android though. Doing more is more complicated.
> 
> Just to be explicit, "Android" includes B2G?

Correct. B2G and Android share gfxAndroidPlatform.
blocking-b2g: tef? → tef+
Should we also flip this pref as soon as possible so that we start testing the 16bit code paths?
blocking-b2g: tef+ → tef?
(In reply to Jonas Sicking (:sicking) from comment #10)
> Should we also flip this pref as soon as possible so that we start testing
> the 16bit code paths?

Yes. However, I'm not sure about the mechanism we use for doing this.
blocking-b2g: tef? → tef+
Chris, we need you to make a call - we can force this pref on to 16-bit as the default, and keep it that way for a few days in order to get as much testing or possible.  Or, we can leave it the way it is, and you can arrange for the dogfooding devices pref to change to enable the 16-bit - however that is done.  Thoughts?
Flags: needinfo?(jones.chris.g)
We should distribute a dogfooder update that enables this pref using the magic dogfood override file.
Flags: needinfo?(jones.chris.g)
OK - can you take care of that?  We're not sure what to do and how to make this happen.
Unfortunately no, I don't know how all this works since it moved to releng infra.
Couldn't we simply set the pref in app/b2g.js in the same update as we implement the pref?
That would force it on everybody?  I'm good either way, Chris?
https://hg.mozilla.org/mozilla-central/rev/e7c8f4228da4
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
We don't want this on for everyone, but if it would require rocket science to update the pref overrides, we could consider that.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #20)
> We don't want this on for everyone, but if it would require rocket science
> to update the pref overrides, we could consider that.

How do we find out whether it requires rocket science or not? Perhaps in the mean time we should just switch everywhere and switch back once we have that figured out.
Let's leave this opened until we're actually using 16 bit
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Marking fixed on v1.0.0 since this landed on mozilla-b2g18 prior to 1/25 branching.
Who's going to take this to completion?
What's left to do here? Do we need an OTA update that sets this pref for our dogfooders?
(In reply to Chris AtLee [:catlee] from comment #25)
> What's left to do here? Do we need an OTA update that sets this pref for our
> dogfooders?

Yes.
Over to catlee to ensure the OTA update happens with this pref set to on.
Assignee: jmuizelaar → catlee
catlee, do you know how to do this pref-setting in an OTA update?
Flags: needinfo?(catlee)
Not off the top of my head. I'm not aware of any existing support for this in the build system. I was thinking we could inject a prefs file into the generated mar file, but that would mean freshly flashed devices wouldn't get it.
Flags: needinfo?(catlee)
Dave, do you know how we can do this?
Assignee: catlee → dhylands
Flags: needinfo?(dhylands)
There are a couple of ways we can flip the pref.

1 - Write a script, similar to this one: https://gist.github.com/dhylands/2656232 and run it. This requires adb access to the phone, but no UI changes.

2 - Add an option to the Developer Settings in the settings App. This requires changes to the following files:
- gaia/build.settings.py (add a default value for the setting)
- gaia/apps/settings/index.html (add some HTML to make the setting show up. search for "console-enabled" to see an example
- gecko/b2g/chrome/content/settings.js - add a forwarder which causes changes in the setting to trigger changes in the pref. Search for app.update.interval
Flags: needinfo?(dhylands)
Doh - I misread something somewhere. I'm pretty sure you can do the pref setting in an OTA update, but you'd need to talk to Marshall about the details.
Flags: needinfo?(marshall)
This may be moot given that dogfooding program status has changed?
I don't understand how OTA factors into this.

Isn't what we want is just some init code that sets the pref if it isn't set?

But for some subset of phones (perhaps just the unagi)?
Flags: needinfo?(jmuizelaar)
(In reply to Dave Hylands [:dhylands] from comment #34)
> I don't understand how OTA factors into this.
> 
> Isn't what we want is just some init code that sets the pref if it isn't set?
> 
> But for some subset of phones (perhaps just the unagi)?

I was told that we didn't want to hard-code for particular phones, however it does seem like it is rocket science to do this for the already over dog fooding program. Right now, I don't know what devices are being tested and which are important. So I'm fine with what ever cjones decides.
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(jones.chris.g)
Manually injecting the pref in /system/b2g/defaults/pref/<whatever.js> into a one-off OTA update is possible, but it's not clear to me how we would coordinate that now with the releng based updates..
Flags: needinfo?(marshall) → needinfo?(catlee)
Doesn't seem like this will make it, we won't necessarily benefit from it, now that dogfooders are elsewhere, let's revisit this.
blocking-b2g: tef+ → tef?
(In reply to Milan Sreckovic [:milan] from comment #37)
> Doesn't seem like this will make it, we won't necessarily benefit from it,
> now that dogfooders are elsewhere, let's revisit this.

Yep, not a blocker.
blocking-b2g: tef? → -
(In reply to Marshall Culpepper [:marshall_law] from comment #36)
> Manually injecting the pref in /system/b2g/defaults/pref/<whatever.js> into
> a one-off OTA update is possible, but it's not clear to me how we would
> coordinate that now with the releng based updates..

I think we could inject it into our updates as well. It's not clear how we'd inject it into a fresh build though.
Flags: needinfo?(catlee)
Assignee: dhylands → nobody
(In reply to Chris AtLee [:catlee] from comment #39)
> (In reply to Marshall Culpepper [:marshall_law] from comment #36)
> > Manually injecting the pref in /system/b2g/defaults/pref/<whatever.js> into
> > a one-off OTA update is possible, but it's not clear to me how we would
> > coordinate that now with the releng based updates..
> 
> I think we could inject it into our updates as well. It's not clear how we'd
> inject it into a fresh build though.

It's especially hard when the list of devices this needs to apply to is unknown...
Not worth it at this point.
Flags: needinfo?(jones.chris.g)
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.