[meta] Turn on by default hardware vsync and the Vsync Aligned Compositor on b2g

RESOLVED FIXED in Firefox 38, Firefox OS v2.2

Status

()

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mchang, Assigned: mchang)

Tracking

37 Branch
mozilla38
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 wontfix, firefox37 wontfix, firefox38 fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

4 years ago
Enable the prefs:

gfx.vsync.hw-vsync.enabled
gfx.vsync.compositor

by default on b2g. Attach any blocking bugs here.
(Assignee)

Updated

4 years ago
Summary: [meta] Turn on by default hardware vsync and the Vsync Aligned Compositor by default on b2g → [meta] Turn on by default hardware vsync and the Vsync Aligned Compositor on b2g
(Assignee)

Updated

4 years ago
Depends on: 1118531
(Assignee)

Updated

4 years ago
Depends on: 1119981
(Assignee)

Updated

4 years ago
Depends on: 1120753
(Assignee)

Updated

4 years ago
Depends on: 1121065
(Assignee)

Updated

4 years ago
Component: Widget: Gonk → Graphics
Hardware: x86 → ARM
Version: 26 Branch → 37 Branch
(Assignee)

Comment 1

4 years ago
Adding QA wanted before enabling default everywhere. 

QA - Please enable the two preferences listed in comment 0. This fundamentally changes how we composite, so if there are any graphics bugs with this enabled, please report them. This is part 1 of 3 to enable Silk by default on b2g.

Expected behavior: Nothing should actually change from a functional standpoint. Should be able to use the phone exactly the same as with this feature enabled. There should also be a slight improvement in smoothness throughout the phone. 

Potential bugs: Graphics not showing or extra jankiness. Crashes.

Tested environments: Flame device, v188-1 firmware.
Gecko (git) - a2f9315326e5918a5d8f5e6e7c9052ed07a08664
Gaia - f0a247f015227a18050e03e75a7dd8c877fb1ca8
Keywords: qawanted
(Assignee)

Comment 2

4 years ago
Created attachment 8552552 [details]
Startup Results With Silk Compositor

30 runs on a flame device with silk off/on with make-test perf. All tests results are from the moz-app-visually-complete start up event. From the results, it looks like negligible difference in start up times. 

The worst case is the camera with a 20 ms increase in start up time with the best case being contacts, with a 20ms decrease in start up time. Both results seem to be within noise measurements.
(Assignee)

Comment 3

4 years ago
There is a slight dip in FPS while scrolling the homescreen, ~1-2 fps. This is probably occurring from missing layer transactions due to the refresh driver not being enabled by default yet. The same problem from (https://bugzilla.mozilla.org/show_bug.cgi?id=1077651#c7). However, both are still pretty good.

Master
Discarded 56 frames over 919.358594 ms in histogram for Compositor
FPS for Compositor. Total Frames: 597 Time Interval: 10.009997 seconds
FPS: 57 = 1. FPS: 60 = 4. FPS: 61 = 4. 
Mean: 60.111111 , std dev 1.196703

Silk Compositor Only
Discarded 56 frames over 967.293334 ms in histogram for Compositor
FPS for Compositor. Total Frames: 576 Time Interval: 10.005537 seconds
FPS: 54 = 1. FPS: 56 = 1. FPS: 57 = 2. FPS: 58 = 2. FPS: 60 = 3. 
Mean: 57.777778 , std dev 1.930905

Discarded 60 frames over 984.192708 ms in histogram for Compositor
FPS for Compositor. Total Frames: 598 Time Interval: 10.008900 seconds
FPS: 58 = 1. FPS: 60 = 8. 
Mean: 59.777778 , std dev 0.628539
(Assignee)

Comment 4

4 years ago
Created attachment 8554591 [details] [diff] [review]
Enable Vsync Compositor by default on b2g
(Assignee)

Updated

4 years ago
Attachment #8554591 - Flags: review?(bugmail.mozilla)
Comment on attachment 8554591 [details] [diff] [review]
Enable Vsync Compositor by default on b2g

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

Change commit message to r=kats
Attachment #8554591 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Updated

4 years ago
Blocks: 1125885
(Assignee)

Updated

4 years ago
Keywords: leave-open
(Assignee)

Updated

4 years ago
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/d914c177e2ba
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Updated

4 years ago
Depends on: 1126373
(Assignee)

Comment 8

4 years ago
Created attachment 8555975 [details] [diff] [review]
Enable vsync compositor by default on b2g, rebased for mozillab2g37_v2_2

Carrying r+, waiting until next week to uplift.
Flags: needinfo?(mchang)
Attachment #8555975 - Flags: review+
(Assignee)

Comment 9

4 years ago
Manually tested this on v2_2 branch, git rev 173569b3aa1efb97db3fc0443a9bb053acf28b42. Everything looking good for uplift next week.
Flags: needinfo?(mchang)
(Assignee)

Updated

4 years ago
Flags: needinfo?(mchang)
(Assignee)

Comment 10

4 years ago
Comment on attachment 8555975 [details] [diff] [review]
Enable vsync compositor by default on b2g, rebased for mozillab2g37_v2_2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Project Silk, 987532
User impact if declined: Composite performance will remain the same. This is mostly a performance improvement patch.
Testing completed: Mochitests, has been tested on master for 1 week, manual testing with today's 2.2 branch.
Risk to taking this patch (and alternatives if risky): High, this fundamentally changes how the compositor is scheduled and uses hardware features on kit-kat to create smoother experiences.
String or UUID changes made by this patch: None
Flags: needinfo?(mchang)
Attachment #8555975 - Flags: approval-mozilla-b2g37?
(Assignee)

Comment 11

4 years ago
BTW, if test_deferred_start.html becomes orange, on 2.2, we have to disable the test from bug 1119981, which was not uplifted to 2.2.
no-jun, can you confirm the verification of this on master/central and that we have no blocking regressions QA found before we do the branch uplift?
Flags: needinfo?(npark)
Bug 1124398 is a touch-related bug that cropped up recently, and I filed bug 1129126, but both of them came out before silk was enabled.  So I think this patch is good, and I do notice less jankiness under repeated UI input.
Flags: needinfo?(npark)

Updated

4 years ago
Attachment #8555975 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0adbd6b0fbd5
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
status-firefox36: --- → wontfix
status-firefox37: --- → wontfix
status-firefox38: --- → fixed
Removing qawanted since this was tested and implemented.
QA Whiteboard: [QAnalyst-Triage+]
Keywords: qawanted

Comment 16

3 years ago
gfx.vsync.compositor=true slows down the mouse move events. Mouse move events are not synched with the vsync and lags the rendering process. Mouse pointer moves faster than the things it carries (such as in games). It would be better to leave it disabled for the sake of end user experience (isnt it the whole purpose?) while hardware one does not affect the performance.
You need to log in before you can comment on or make changes to this bug.