Closed
Bug 887819
Opened 11 years ago
Closed 11 years ago
Investigate using the tiled layers backend
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: cwiiis, Assigned: blassey)
References
Details
(Whiteboard: [UCID: Browser1, FT:Browser, KOI:P1])
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
We get quite a performance boost using the tiled layers backend in fennec, as it:
- Allows us to avoid NPOT textures
- Allows us to avoid subimage upload
- Enables progressive updating
- Enables update cancelling
It makes sense to investigate using the same backend on b2g. Note that unfortunately, it's not as simple as just flipping a switch, this likely requires some supporting code that is currently implemented in Java in fennec.
Comment 1•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #0)
> - Allows us to avoid NPOT textures
The tile backend actually use 256^2 tiles
> - Allows us to avoid subimage upload
> - Enables progressive updating
> - Enables update cancelling
Other benefit are handling resizing the buffer more efficiently. Running with layer border you can notice that we resize our layers quite often.
>
> It makes sense to investigate using the same backend on b2g. Note that
> unfortunately, it's not as simple as just flipping a switch, this likely
> requires some supporting code that is currently implemented in Java in
> fennec.
We could just disable those 2 features for now and still investigate the tile layer backend.
The real problem is making tile sharing cross-process safe. I had some patch that were nearly there but they will be bit-rotted so will need some tedious manual rebasing. This is the bulk of the work.
Reporter | ||
Comment 2•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #1)
> (In reply to Chris Lord [:cwiiis] from comment #0)
> > - Allows us to avoid NPOT textures
> The tile backend actually use 256^2 tiles
> > - Allows us to avoid subimage upload
> > - Enables progressive updating
> > - Enables update cancelling
>
> Other benefit are handling resizing the buffer more efficiently. Running
> with layer border you can notice that we resize our layers quite often.
Missed this one :)
> >
> > It makes sense to investigate using the same backend on b2g. Note that
> > unfortunately, it's not as simple as just flipping a switch, this likely
> > requires some supporting code that is currently implemented in Java in
> > fennec.
>
> We could just disable those 2 features for now and still investigate the
> tile layer backend.
Right, we should definitely do this in stages. I would imagine that even without these features, we may see a performance boost, especially on devices with larger screens.
> The real problem is making tile sharing cross-process safe. I had some patch
> that were nearly there but they will be bit-rotted so will need some tedious
> manual rebasing. This is the bulk of the work.
Care to attach those to this bug? Maybe someone will have the time to do so before you do, unless you think it's not quite at that stage yet?
Comment 3•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #2)
> > The real problem is making tile sharing cross-process safe. I had some patch
> > that were nearly there but they will be bit-rotted so will need some tedious
> > manual rebasing. This is the bulk of the work.
>
> Care to attach those to this bug? Maybe someone will have the time to do so
> before you do, unless you think it's not quite at that stage yet?
Making this depend on bug 747811 since it still accurately describes the pre-req.
Depends on: 747811
Comment 4•11 years ago
|
||
I think bug 859929 might also come into play here if we turn this on. Linking it here just in case.
Depends on: 859929
Comment 5•11 years ago
|
||
You're right. I suggest we use this bug for enabling tiling without progressive drawing and without draw aborting. Those features have their own set of problems but they just happen to depend on tiling. Tiling should remain independent of features that use them (progressive, drawing aborting).
Reporter | ||
Comment 6•11 years ago
|
||
So I'm going to be pushing the patches on bug 747811 today, and with those, the tiles backend now works on b2g.
Discussing this with blassey, we came to the conclusion that even without better display-port sizing and the other features listed in comment #0 and comment #1, we should enable this to see how it does/get further testing.
This patch enables tiles on b2g.
Reporter | ||
Comment 7•11 years ago
|
||
I'm not really sure whether the patch I've attached should be here or on bug 894333 (and if it should be there, I don't really know what this bug is for).
I'm leaving it here for now, though I would say that we've done the investigation now and initial results look pretty promising.
Comment 8•11 years ago
|
||
Comment on attachment 792112 [details] [diff] [review]
Enable tiles on b2g
Landing this would disable gralloc for content. Do you have data to suggest that we're ready doing this without a Gralloc implementation of tiles?
Reporter | ||
Comment 9•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #8)
> Comment on attachment 792112 [details] [diff] [review]
> Enable tiles on b2g
>
> Landing this would disable gralloc for content. Do you have data to suggest
> that we're ready doing this without a Gralloc implementation of tiles?
I don't yet, but hopefully I'll have eideticker results soon.
Personally, I think if the drop just switching it on with no further enhancements isn't too large (who knows, it may not even be a drop), we should take it and not use gralloc anyway - I see obvious visible glitches using gralloc on both the Keon and Peak (though I pushed a patch to disable gralloc on the Peak because it got hopelessly broken at some point).
I'd be very surprised if after enabling progressive rendering and cancelling, if using tiles without gralloc was outperformed by not using it, with.
Comment 10•11 years ago
|
||
I agree with what you said. I'd just like to have some analysis to back up our decision in this bug. Lets wait for the eideticker results.
Comment 11•11 years ago
|
||
Comment on attachment 792112 [details] [diff] [review]
Enable tiles on b2g
Postponing review until we have an argument to support switching this on.
Attachment #792112 -
Flags: review?(bgirard)
Assignee | ||
Comment 12•11 years ago
|
||
Assignee: chrislord.net → blassey.bugs
Attachment #792112 -
Attachment is obsolete: true
Attachment #802493 -
Flags: review?(bgirard)
Updated•11 years ago
|
Attachment #802493 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 13•11 years ago
|
||
Attachment #802496 -
Flags: review?(gal)
Attachment #802496 -
Flags: review?(bgirard)
Comment 14•11 years ago
|
||
Comment on attachment 802496 [details] [diff] [review]
enable tiles pref for b2g
Review of attachment 802496 [details] [diff] [review]:
-----------------------------------------------------------------
Flipping this preference will have the following results:
- We wont use buffer rotation for thebes content, we will use tiling which will support changing dimensions better.
- Thebes layer will no longer use gralloc but instead use regular OGL textures. Some things will get better but some things may get worse. This means disabling HWC since we always have a non gralloc layer.
I think at this point we should get wider testing with the preference flipp before officially flipping it by default during feature freeze.
Assignee | ||
Comment 15•11 years ago
|
||
pushed the pref patch https://hg.mozilla.org/mozilla-central/rev/efc53394043e
Comment 16•11 years ago
|
||
We would need to restore gralloc for this to make 1.2. That seems doable though. BenWa, can you comment?
Comment 17•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #14)
> Comment on attachment 802496 [details] [diff] [review]
> enable tiles pref for b2g
>
> Review of attachment 802496 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Flipping this preference will have the following results:
> - We wont use buffer rotation for thebes content, we will use tiling which
> will support changing dimensions better.
> - Thebes layer will no longer use gralloc but instead use regular OGL
> textures. Some things will get better but some things may get worse. This
> means disabling HWC since we always have a non gralloc layer.
>
> I think at this point we should get wider testing with the preference flipp
> before officially flipping it by default during feature freeze.
FWIW - This sounds like a good case to spin a try build and see if we can run a specialized Gaia UI Test run against this build to see if there's critical gfx regressions that result from this patch.
Assignee | ||
Comment 18•11 years ago
|
||
sure thing, but try is closed right now
Assignee | ||
Comment 19•11 years ago
|
||
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to Andreas Gal :gal from comment #16)
> We would need to restore gralloc for this to make 1.2. That seems doable
> though. BenWa, can you comment?
why?
Comment 21•11 years ago
|
||
Hi Jason I can't run gaia-ui on this because the try build is not engineering/marionette enabled.
Comment 22•11 years ago
|
||
(In reply to Andreas Gal :gal from comment #16)
> We would need to restore gralloc for this to make 1.2. That seems doable
> though. BenWa, can you comment?
If we try to do gralloc tiles for 1.2 were going to have to fix the gralloc lock regression. Turning on tiling over a regression will give us a win so that's not useful data. I think we need to fix the regressions for 1.2 and take our time to carefully tweak tiling. Jeff has some idea that Gralloc should be a mix of gralloc+textures which would match the android browser and perform better. If we rush tiling were likely to end in a state where tiling isn't polished enough to be better then 1.1 and now we're stuck with buffer rotation and tiling that are both worse then 1.1. IMO tiles need to wait for 1.3
Comment 23•11 years ago
|
||
(In reply to Zac C (:zac) from comment #21)
> Hi Jason I can't run gaia-ui on this because the try build is not
> engineering/marionette enabled.
Ack okay - didn't realize that. We should get a bug file to generating try builds with marionette enabled though. i'll do that.
QA Wanted - Do some exploratory testing around the daily smoketests to see if there's any critical issues that result from this patch.
Build for testing is here - https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/try-builds/blassey@mozilla.com-1e0c76686a0a/try-unagi/.
Keywords: qawanted
Comment 24•11 years ago
|
||
Comment on attachment 802496 [details] [diff] [review]
enable tiles pref for b2g
a=me, assuming this had sufficient basic testing that this won't burn our tree and make trunk unusable
Attachment #802496 -
Flags: review?(gal)
Attachment #802496 -
Flags: review?(bgirard)
Attachment #802496 -
Flags: review+
Comment 25•11 years ago
|
||
Assignee | ||
Comment 26•11 years ago
|
||
R6 had two tests that unexpectedly started passing, R9 has gradients with off-by-one color values.
Attachment #802496 -
Attachment is obsolete: true
Attachment #803339 -
Flags: review?(bgirard)
Updated•11 years ago
|
Attachment #803339 -
Flags: review?(bgirard) → review+
Comment 27•11 years ago
|
||
Brad, if I understand the dependencies here correctly then bug 909887 will break the FFOS homescreen. In other words we can't flip the pref until that and any other major regression is addressed.
Comment 28•11 years ago
|
||
Looks like the tiled layers pref is turned on Moz-central. I'm seeing it in the build I synced to last night. HWC is disabled with this.
Also, does this apply to other use cases such as video, camera playback?
Comment 29•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 30•11 years ago
|
||
Looks like given that this landed, we no longer need to do testing pre-landing. If regressions come up, we'll see it tomorrow's smoketest run.
Keywords: qawanted
Assignee | ||
Comment 31•11 years ago
|
||
(In reply to Andreas Gal :gal from comment #27)
> Brad, if I understand the dependencies here correctly then bug 909887 will
> break the FFOS homescreen. In other words we can't flip the pref until that
> and any other major regression is addressed.
no bug 909887 is a regression from turning APZC on
Assignee | ||
Comment 32•11 years ago
|
||
(In reply to Mandyam Vikram from comment #28)
> Looks like the tiled layers pref is turned on Moz-central. I'm seeing it in
> the build I synced to last night. HWC is disabled with this.
> Also, does this apply to other use cases such as video, camera playback?
Yes, this change applies to all apps.
(In reply to Jason Smith [:jsmith] from comment #30)
> Looks like given that this landed, we no longer need to do testing
> pre-landing. If regressions come up, we'll see it tomorrow's smoketest run.
Thanks, please ping me if there are any regressions.
Updated•11 years ago
|
Whiteboard: [UCID: Browser1, FT:Browser, KOI:P1]
Comment 33•11 years ago
|
||
Some preliminary observations (eyeballing on a 7x27 QRD device without any detail testing):
- Homescreen looks ok
- Without detailed profiling, there is improvement in scrolling in Contacts (maybe not 60 FPS yet)over the earlier build at the beginning of the week
- Ditto on Gallery app scroll
Comment 34•11 years ago
|
||
When tiling is enabled on b2g device, it always renders by using OpenGL, HwComposer rendeirng is aborted with the following log.
-----------
D/HWComposer( 143): ThebesLayerComposite Layer doesn't have a gralloc buffer
D/HWComposer( 143): Render aborted. Nothing was drawn to the screen
D/HWComposer( 143): ThebesLayerComposite Layer doesn't have a gralloc buffer
D/HWComposer( 143): Render aborted. Nothing was drawn to the screen
Comment 35•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #34)
That's expected. See comment 14, 2nd bullet.
Comment 36•11 years ago
|
||
Does this patch affect Fennec? I see the patch in regression ranges for several branches as a possible cause for a checkerboarding regression. For example:
Regression: Mozilla-Inbound - Robocop Checkerboarding Real User Benchmark - Android 4.0.4 - 249% increase
---------------------------------------------------------------------------------------------------------
Previous: avg 8.829 stddev 1.380 of 12 runs up to revision feea0997db36
New : avg 30.777 stddev 1.775 of 12 runs since revision 9fd3e98cf2f8
Change : +21.948 (249% / z=15.909)
Graph : http://mzl.la/18Syabr
Regression: Fx-Team - Robocop Checkerboarding Real User Benchmark - Android 4.0.4 - 234% increase
-------------------------------------------------------------------------------------------------
Previous: avg 9.226 stddev 1.161 of 12 runs up to revision bdf495121827
New : avg 30.791 stddev 1.313 of 12 runs since revision a98569f21abe
Change : +21.565 (234% / z=18.573)
Graph : http://mzl.la/1eJxudg
Changeset range: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=bdf495121827&tochange=a98569f21abe
Comment 37•11 years ago
|
||
Can we revert this change while bug 915673 is investigated? It's affecting bring up of Hwc on JB (bug 911391) and will affect power measurements in v1.2 in general.
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 38•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #36)
> Does this patch affect Fennec?
It really shouldn't. I doubled check. I could be wrong so I recommend having someone break in TiledLayerBuffer.h.
(In reply to Diego Wilson [:diego] from comment #37)
> and will affect power measurements in
> v1.2 in general.
Can you quantify or share a way to measure power? The GFX team is hoping to switch to tiling everywhere (and ideally not just for scrolling) including b2g in some future release. Understanding the power impact of tiles is key.
Comment 39•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #38)
>
> Can you quantify or share a way to measure power? The GFX team is hoping to
> switch to tiling everywhere (and ideally not just for scrolling) including
> b2g in some future release. Understanding the power impact of tiles is key.
b2g preformance team made a small hw and tool to measure power consumption in realtime from actual b2g phone. Jon Hylands showed a demo in b2g work week. I think that video playback(youtube) in web browser is the important use case to measure power. Diego know better about it.
Comment 40•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #36)
> Does this patch affect Fennec? I see the patch in regression ranges for
> several branches as a possible cause for a checkerboarding regression. For
> example:
>
> Regression: Mozilla-Inbound - Robocop Checkerboarding Real User Benchmark -
> Android 4.0.4 - 249% increase
> -----------------------------------------------------------------------------
> ----------------------------
> Previous: avg 8.829 stddev 1.380 of 12 runs up to revision feea0997db36
> New : avg 30.777 stddev 1.775 of 12 runs since revision 9fd3e98cf2f8
> Change : +21.948 (249% / z=15.909)
> Graph : http://mzl.la/18Syabr
>
> Regression: Fx-Team - Robocop Checkerboarding Real User Benchmark - Android
> 4.0.4 - 234% increase
> -----------------------------------------------------------------------------
> --------------------
> Previous: avg 9.226 stddev 1.161 of 12 runs up to revision bdf495121827
> New : avg 30.791 stddev 1.313 of 12 runs since revision a98569f21abe
> Change : +21.565 (234% / z=18.573)
> Graph : http://mzl.la/1eJxudg
>
> Changeset range:
> http://hg.mozilla.org/integration/fx-team/
> pushloghtml?fromchange=bdf495121827&tochange=a98569f21abe
I believe bug 912806 is the guilty one here. See https://bugzilla.mozilla.org/show_bug.cgi?id=912806#c27
Comment 41•11 years ago
|
||
This is still in the tree, right? Please only reopen this bug if you actually back it out.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Comment 42•11 years ago
|
||
Can someone please clarify the status of tiled rendering for 1.3? As mentioned previously, if we don't use HWC (due to non usage of gralloc bufers), this will negatively impact power consumption.
Reporter | ||
Comment 43•11 years ago
|
||
(In reply to Mandyam Vikram from comment #42)
> Can someone please clarify the status of tiled rendering for 1.3? As
> mentioned previously, if we don't use HWC (due to non usage of gralloc
> bufers), this will negatively impact power consumption.
So a patch has gone in that limits tiled layer usage to scrollable layers (when tiles are enabled at all, that is) - So if we enable it now, it will only have an impact while a scrollable layer is visible on the screen (this is not the case when just the home-screen is visible, for example).
A lot of apps will always have tiled layers visible though, but would this affect power usage unless we're recompositing? And if we're recompositing, don't we just want the best performance?
I don't know what our decision is on tiles in 1.3, but just adding this information here.
Comment 44•11 years ago
|
||
APZC without tiles is not feasible since we lose buffer rotation and repainting is too slow.
Mandyam, my understanding from previous conversations is that power only matters for video playback and camera. Both don't scroll, so this bug won't affect them in any way. Is that correct?
Comment 45•11 years ago
|
||
(In reply to Andreas Gal from https://bugzilla.mozilla.org/show_bug.cgi?id=887819#c44)
> Mandyam, my understanding from previous conversations is that power only matters for video playback and
> camera. Both don't scroll, so this bug won't affect them in any way. Is that correct?
Will tiled rendering be effective for AZPC use cases only? Can you please describe which use cases it would affect? Is tiled rendering then planning to be turned on for 1.3?
We do need to use HWC for camera and video playback. We haven't gotten good power numbers for the other use cases for 1.3 yet.
Comment 46•11 years ago
|
||
Tiling would not be used for video or camera so those would not be impacted by using tiling.
Comment 47•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #46)
> Tiling would not be used for video or camera so those would not be impacted
> by using tiling.
What about WebGL? IIRC the performance of WegGL was much better with HWC.
Comment 48•11 years ago
|
||
As BenWa said, tiling would be used only scrolling layers, so no WebGL, no Camera, no Video playback. Mostly app content (scrolling contacts) and browser etc.
Comment 49•11 years ago
|
||
Is it ready to be turned on for 1.3 then? Has tiled rendering been profiled (v/s current implementation on 1.3)?
Reporter | ||
Comment 50•11 years ago
|
||
In case it isn't already assumed, turning tiles on really requires turning APZC on too (bug 909877).
You need to log in
before you can comment on or make changes to this bug.
Description
•