Closed
Bug 1024144
Opened 10 years ago
Closed 10 years ago
Deliver acquire fence to HwComposer
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
People
(Reporter: sotaro, Assigned: sotaro)
References
Details
Attachments
(1 file, 4 obsolete files)
4.40 KB,
patch
|
sushilchauhan
:
review+
nical
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1024025 +++ gecko does not deliver fence to HwComposer. Need to deliver it to HwComposer. The patch will be moved from Bug 1024025.
Assignee | ||
Comment 1•10 years ago
|
||
Does it mean, the acquire fence on Gecko layers has been enabled and tested on v1.4 as well ? Until now, we were setting hwcLayer.acquireFenceFd = -1, for each HWC layer as content was promised to be ready on layers when they come for Composition.
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Sushil from comment #2) > Does it mean, the acquire fence on Gecko layers has been enabled and tested > on v1.4 as well ? Suhil, what do you mean tested? Can't we set acuqire fence? After it is enabled, all WebGL content is going to use this path. It say that we are going to test on flame device.
Flags: needinfo?(sotaro.ikeda.g) → needinfo?(sushilchauhan)
Assignee | ||
Updated•10 years ago
|
Attachment #8438736 -
Flags: review?(sushilchauhan)
Attachment #8438736 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #1) > Created attachment 8438736 [details] [diff] [review] > patch - Deliver acquire fence to HwComposer attachment 8438736 [details] [diff] [review] changes a place of Wait fence of OpenGL. It improve also OpenGL Composition's performance.
(In reply to Sotaro Ikeda [:sotaro] from comment #3) > (In reply to Sushil from comment #2) > > Does it mean, the acquire fence on Gecko layers has been enabled and tested > > on v1.4 as well ? > > Suhil, what do you mean tested? Can't we set acuqire fence? After it is > enabled, all WebGL content is going to use this path. It say that we are > going to test on flame device. I mean, have you tested this patch for any potential side-effect in use cases like Video, Camera, etc. where HWC is used (on overlay & non H/W overlay devices) ? Can I pull this patch on v1.4 and test ?
Flags: needinfo?(sushilchauhan)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Sushil from comment #6) > (In reply to Sotaro Ikeda [:sotaro] from comment #3) > > (In reply to Sushil from comment #2) > > > Does it mean, the acquire fence on Gecko layers has been enabled and tested > > > on v1.4 as well ? > > > > Suhil, what do you mean tested? Can't we set acuqire fence? After it is > > enabled, all WebGL content is going to use this path. It say that we are > > going to test on flame device. > > I mean, have you tested this patch for any potential side-effect in use > cases like Video, Camera, etc. where HWC is used (on overlay & non H/W > overlay devices) ? Can I pull this patch on v1.4 and test ? Which effect? Camera and video does not provide Acquire fence. The patch is for master. But you can apply it to v1.4.
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #7) > (In reply to Sushil from comment #6) > > (In reply to Sotaro Ikeda [:sotaro] from comment #3) > > > (In reply to Sushil from comment #2) > > > > Does it mean, the acquire fence on Gecko layers has been enabled and tested > > > > on v1.4 as well ? > > > > > > Suhil, what do you mean tested? Can't we set acuqire fence? After it is > > > enabled, all WebGL content is going to use this path. It say that we are > > > going to test on flame device. > > > > I mean, have you tested this patch for any potential side-effect in use > > cases like Video, Camera, etc. where HWC is used (on overlay & non H/W > > overlay devices) ? Can I pull this patch on v1.4 and test ? > > Which effect? Camera and video does not provide Acquire fence. The patch is > for master. But you can apply it to v1.4. Sorry, video does not provide acquire fence. But camera could provide acquire fence. But until Bug 1001417, acquire fence is not handled at all. It seems more problematic.
Flags: needinfo?(sotaro.ikeda.g)
In Video playback use-case, when UI seek-bar is present, I believe we should be getting acquire fences now (with your patch). I will test it on v1.4.
Assignee | ||
Comment 10•10 years ago
|
||
Thanks for checking! But iirc, OMXCodec does not provide acquire fence. Who provides AcquireFence during playback?
Comment 11•10 years ago
|
||
In Comment 9, I was pointing to acquire fences of UI layers in frame, when video controls are present.
Comment 12•10 years ago
|
||
Comment on attachment 8438736 [details] [diff] [review] patch - Deliver acquire fence to HwComposer On a good v1.4 build, I pulled patch from Bug 1001417 and this patch, but now the device crashes. Please check. Reason: "Compositor: unhandled page fault". Please list if there are any dependencies which I should make sure, I must have.
Attachment #8438736 -
Flags: review?(sushilchauhan) → review-
Comment 13•10 years ago
|
||
Reason for crash. You need to skip or fix the loop in Commit(), in case of GPU Composition frame. Number of layers = 2. HWC layer[0] = Bogus app layer HWC layer[1] = FB layer
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Attachment #8438736 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 14•10 years ago
|
||
Add more checks.
Attachment #8438736 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8439317 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
Add HWC_FRAMEBUFFER check.
Attachment #8439357 -
Attachment is obsolete: true
Assignee | ||
Comment 17•10 years ago
|
||
Sushi, does the patch attachment 8439368 [details] [diff] [review] work on your device? On my side, I did not saw the crash on flame and on qrd device with old patch.
Flags: needinfo?(sushilchauhan)
Comment 18•10 years ago
|
||
(mList->hwLayers[j].flags & HWC_SKIP_LAYER) check is not needed, HWC_FRAMEBUFFER check takes care of it. I am little busy currently. Later today, I will test your patch on my device and let you know.
Flags: needinfo?(sushilchauhan)
Assignee | ||
Comment 19•10 years ago
|
||
Apply the comment.
Attachment #8439368 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8439421 -
Flags: review?(sushilchauhan)
Attachment #8439421 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 20•10 years ago
|
||
Nominate to "b2g-v1.4". This bug affect to WebGL game's performance and blocking 1.4+ bug.
blocking-b2g: --- → 1.4?
Updated•10 years ago
|
Attachment #8439421 -
Flags: review?(nical.bugzilla) → review+
Attachment #8439421 -
Flags: review?(sushilchauhan) → review+
Assignee | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/161cad49ab77
https://hg.mozilla.org/mozilla-central/rev/161cad49ab77
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment 24•10 years ago
|
||
NI :elan, if we still need given we passed partner certification?
Flags: needinfo?(elancaster)
Comment 25•10 years ago
|
||
Sotaro, Please provide risk analysis here. Also provide information on improvement in perf with taking this fix in 1.4.
Flags: needinfo?(sotaro.ikeda.g)
Comment 26•10 years ago
|
||
Moving to backlog please re-nom if necessary for partner cert.
blocking-b2g: 1.4? → backlog
Assignee | ||
Comment 27•10 years ago
|
||
(In reply to Preeti Raghunath(:Preeti) from comment #25) > Sotaro, > > Please provide risk analysis here. Also provide information on improvement > in perf with taking this fix in 1.4. This patch change as to deliver Fence to HwComposer. HwComposer's fence handling might cause the crash like Bug 1025824. By applying Bug 1025824 fix, the risk becomes low. I checked performance down on master flame. At the time performance was degraded from 46 fps(OpenGL) to 33fps(HwComposer). But on latest v1.4 flame, the fps was like the following. It is good. - b2g-v1.4, OpenGL composition: 41fps - b2g-v1.4, HwComposer composition: 42fps The performance difference might comes gfx layer's difference.
Flags: needinfo?(sotaro.ikeda.g)
Comment 28•10 years ago
|
||
Sotaro, Please land this patch (and Bug 1025824) on b2g v2.0 as well.
blocking-b2g: backlog → 2.0?
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 29•10 years ago
|
||
(In reply to Sushil from comment #28) > Sotaro, > > Please land this patch (and Bug 1025824) on b2g v2.0 as well. Bug 1024144 is not landed on v2.0. Therefore, Bug 1025824 is not necessary.
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 30•10 years ago
|
||
(In reply to Sushil from comment #28) > Sotaro, > > Please land this patch (and Bug 1025824) on b2g v2.0 as well. If this bug get "b2g-v2.0+", this bug's patch is automatically uplifted.
Comment 31•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #30) > (In reply to Sushil from comment #28) > > Sotaro, > > > > Please land this patch (and Bug 1025824) on b2g v2.0 as well. > > If this bug get "b2g-v2.0+", this bug's patch is automatically uplifted. I am missing to understand the impact on 2.0 here to make this a blocker. Sushil/Sotoro can you help with here? What do the fps numbers look like currently on 2.0 and how much will the landing here improve?
Comment 32•10 years ago
|
||
HwcComposer2D used to set acquire fence of layers as -1 which means that whenever TryRender() is called, the content on all Gecko layers is ready to be composed, i.e. the painting is complete. Since Bug 1001417 has landed on v2.0 so it makes sense to land this patch to make HwcComposer2D and below modules (HAL and driver) to be aware of acquire fences of each HWC layer. If we do not land this patch, we might hit issues like tearing or corruption, i.e. when driver is reading the content (since acquire fence is -1) but the acquire fence has not been signaled (layer is still being painted). Sotaro can give details on FPS improvement.
Blocks: CAF-v2.0-FC-metabug
Comment 33•10 years ago
|
||
(In reply to Sushil from comment #32) > HwcComposer2D used to set acquire fence of layers as -1 which means that > whenever TryRender() is called, the content on all Gecko layers is ready to > be composed, i.e. the painting is complete. > > Since Bug 1001417 has landed on v2.0 so it makes sense to land this patch to > make HwcComposer2D and below modules (HAL and driver) to be aware of acquire > fences of each HWC layer. If we do not land this patch, we might hit issues > like tearing or corruption, i.e. when driver is reading the content (since > acquire fence is -1) but the acquire fence has not been signaled (layer is > still being painted). > > Sotaro can give details on FPS improvement. Sotaro can you help here?
Flags: needinfo?(ikeda.sohtaroh)
Assignee | ||
Comment 34•10 years ago
|
||
> > Since Bug 1001417 has landed on v2.0 so it makes sense to land this patch to > make HwcComposer2D and below modules (HAL and driver) to be aware of acquire > fences of each HWC layer. If we do not land this patch, we might hit issues > like tearing or corruption, i.e. when driver is reading the content (since > acquire fence is -1) but the acquire fence has not been signaled (layer is > still being painted). The tearing/corruption problem should not happen. Acquire fence wait happens at GrallocTextureSourceOGL::BindEGLImage(). Bind EGL Image happen even when on full hw composition to correctly unbind previous gralloc buffer. https://hg.mozilla.org/releases/mozilla-aurora/file/234b0fa642ad/gfx/layers/opengl/GrallocTextureHost.cpp#l444 > Sotaro can give details on FPS improvement. I checked the improvement on b2g-v2.0 flame, with an application that I recently always use for the performance check. It was about 1 fps improvement. - without fix: 47fps - with fix: 48fps
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(ikeda.sohtaroh)
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
Flags: needinfo?(elancaster)
Comment 35•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/befee2ec3a82
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•