Bug 909877 (gaia-apzc)

[meta] Turn on APZC for all of gaia

RESOLVED FIXED in 1.3 C2/1.4 S2(17jan)

Status

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

({perf})

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.2 wontfix, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

(Whiteboard: [c=handeye p= s=2014.01.17 u=1.3])

Attachments

(1 obsolete attachment)

Currently subframe scrolling in Gaia apps happens using BrowserElementPanning.js and occurs synchronously. This bug tracks all the work needed to switch that over to AsyncPanZoomController's async panning.
Created attachment 796178 [details] [diff] [review]
WIP - turn on APZC on everything in gaia

For the record this is what I'm using as my current hack to "turn on APZC everywhere". It may not be correct but it's good enough to find out what bugs need fixing.
Alias: gaia-apzc
We should keep an eye on the FPS (e.g., bug 912134) as we convert the apps, as a sanity check that the performance is not regressing with these changes.
To turn this on by default, check 929728 which introduced the preference.  Not ready for it yet.
blocking-b2g: --- → 1.3+
Spoke with nhirata in person about this - he's going to drive the QA testing on this.
Flags: in-moztrap?(nhirata.bugzilla)
Is there any UX design required for this? Or is APZC mainly related to performance?
Flags: needinfo?(bugmail.mozilla)
Flags: needinfo?(21)
(In reply to Rob MacDonald [:robmac] from comment #6)
> Is there any UX design required for this? Or is APZC mainly related to
> performance?

Afaict APZC in this bug is mostly related to performance. 

I believe that there are some UX questions that can be discussed into relevant bugs. For example about overscroll indicators in bug 775469. Or about the panning heuristic, or event about when to display scrollbars and when not, etc...

IMHO the main consideration you should have here is that APZC will unify the scrolling behavior between Gaia Web Browser and Apps running in Firefox OS. The behavior will be derived from the current behavior of the browser app, so if there are things that UX does not like in it, feel free to open bugs to that blocks this one and let's discuss the details there.
Flags: needinfo?(21)
Keywords: perf
Whiteboard: [FFOS_perf] → [c= p= s= u=]
Depends on: 943112
No longer depends on: 943112
Blocks: 845690
(In reply to Rob MacDonald [:robmac] from comment #6)
> Is there any UX design required for this? Or is APZC mainly related to
> performance?

What Vivien said.
Flags: needinfo?(bugmail.mozilla)
Is this gonna be in 1.3 after all?
Depends on: 945789
(In reply to Jan Jongboom [:janjongboom] from comment #10)
> Is this gonna be in 1.3 after all?

The target is 1.3.
Depends on: 946545
Depends on: 947354
(From FxOS Perf triage) I thought we didn't 1.3+ meta bug?
Flags: needinfo?(praghunath)
(In reply to Hubert Figuiere [:hub] from comment #12)
> (From FxOS Perf triage) I thought we didn't 1.3+ meta bug?

It doesn't hurt, and it makes it easier to make sure dependent bugs are triaged.  Sometimes we forget to put a 1.3? or 1.3+ on the blockers.
The patch doesn't apply to the current 1.3 build.  Need to talk to kats to figure out where to apply the patch.
Comment on attachment 796178 [details] [diff] [review]
WIP - turn on APZC on everything in gaia

The patch is obsolete now. There is an option in the gaia settings to enable APZ
Attachment #796178 - Attachment is obsolete: true
Depends on: 951290
Depends on: 951342
Depends on: 951357
Depends on: 950993
Depends on: 951433
Depends on: 951308
Depends on: 951321
Depends on: 951458

Updated

5 years ago
Blocks: 942267
Depends on: 957188
Vivien,

What are the next steps here?
Flags: needinfo?(praghunath) → needinfo?(21)
Pull request pending merge at https://github.com/mozilla-b2g/gaia/pull/15122 to enable by default.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #17)
> Pull request pending merge at https://github.com/mozilla-b2g/gaia/pull/15122
> to enable by default.

Are you looking for someone to review that patch?
There's not much to review. Milan asked me to make the change and fabrice said he would merge it once the travis run is green.
Depends on: 957790
(In reply to Fabrice Desré [:fabrice] from comment #20)
> https://github.com/mozilla-b2g/gaia/commit/
> 236e4f96cbfdaf2c90c12ebf61107dd274e7fd73

Should we mark this fixed with this landing then?
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 957848

Updated

5 years ago
Whiteboard: [c= p= s= u=] → [c=handeye p= s=2014.01.17 u=1.3]
I'm unsure of the gaia branching model but I believe this will need to be merged to 1.3 as well.
status-b2g-v1.2: --- → wontfix
status-b2g-v1.3: --- → affected
No longer depends on: 957925
Depends on: 942750
cannot scroll in FTU, see bug 958208
Depends on: 958208
This does not uplift cleanly:

$ git diff
* Unmerged path build/config/common-settings.json


$ git status
<snip>
Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

	deleted by us:      build/config/common-settings.json
I made a rebased version on the latest mozillaorg/v1.3 code:

https://github.com/staktrace/gaia/commit/969448b53a9148e58880f64c7a12425ddef99dda

Please let me know if you require it in some other format.
Flags: needinfo?(jhford)
Depends on: 951259
Depends on: 958245
[v1.3 226aa05] Bug 909877 - Enable APZ for all of gaia. rs=milan
status-b2g-v1.3: affected → fixed
Flags: needinfo?(jhford)
Next step has already been done. Clearing.
Flags: needinfo?(21)
No longer depends on: 951259
No longer depends on: 958036

Updated

5 years ago
Blocks: 958650
No longer blocks: 958650
Depends on: 958650
No longer depends on: 950301
Depends on: 959199
Depends on: 959198
No longer depends on: 942750
Depends on: 959866
No longer depends on: 959866

Updated

5 years ago
No longer depends on: 947337
Depends on: 959414
Depends on: 960163

Updated

5 years ago
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
No longer depends on: 958230
No longer depends on: 959425
Depends on: 962791
generic apz test created to cover a lot of things to check.
Flags: in-moztrap?(nhirata.bugzilla) → in-moztrap+
No longer blocks: 823206
Depends on: 965004
No longer depends on: 965004
Depends on: 964997
Depends on: 964981
Depends on: 965353
No longer depends on: 965353
Depends on: 964421
Depends on: 966476
Depends on: 966507
Depends on: 966510
Duplicate of this bug: 883433

Comment 31

5 years ago
Depends on: 968145
Depends on: 969483
No longer depends on: 966510
No longer depends on: 959414
Depends on: 972675
No longer depends on: 972675
Depends on: 975033
No longer depends on: 977295
Depends on: 980041
Depends on: 980679
Depends on: 982888
status-b2g-v1.3T: --- → fixed
status-b2g-v1.4: --- → fixed
Depends on: 987771
Depends on: 989403
No longer depends on: 989403
Depends on: 996991
Duplicate of this bug: 875538
You need to log in before you can comment on or make changes to this bug.