Closed Bug 771653 Opened 8 years ago Closed 8 years ago
Gonk EGL rendering should use hardware composer API's instead of egl
Some android vendors providing hw compositor module which include specific extra stuff which is need to be done for specific device. http://androidxref.com/source/xref/hardware/libhardware/include/hardware/hwcomposer.h http://androidxref.com/source/xref/hardware/libhardware/modules/hwcomposer/hwcomposer.cpp for example some time default eglSwapBuffers call is not enough, and additional actions need to be performed for proper display refresh, like OMAPFB_WINDOW_UPDATE. all that specific calls hidden by vendor behind hwcomposer.h apis for example here is hwcomposer for N9 http://gitorious.org/android-n900/vendor_nitdroid_n9/blobs/ics/hwcomposer/hwcomposer.cpp we don't use surface flinger, and replacing surfaceFlinger, but surface flinger use hwcomposer.h APIS, so vendors rely on that... probably we should apply patch below for gonk port
Comment on attachment 639784 [details] [diff] [review] hwc compoer APIs usage The idea is right. We'll need something like this to support the pandaboard. Two things - 1. As discussed on IRC, we need to also call prepare. I don't know if this needs to be called before every swap, or just once. 2. Gingerbread doesn't have hwc afaik, so we'd need to fallback to normal swaps in that case. The try server will tell you if your patch builds on GB.
Attachment #639784 - Flags: feedback?(mwu) → feedback+
Just a version of the hwc patch that adds a prepare call so it works on the pandaboard.
Comment on attachment 639784 [details] [diff] [review] hwc compoer APIs usage The bug here only affects the display framebuffer's GLContext, right? Using this for swapping all GLContexts seems wrong, and in fact likely wouldn't work in content processes. Along those lines, I don't like this living in gfx/gl ... is there a way to localize it to widget/gonk?
> Along those lines, I don't like this living in gfx/gl ... is there a way to > localize it to widget/gonk? then we need to expose eglSurface and eglDisplay to widget gonk... also we don't need it for content process... it is only needed for process which does actually composition and holding top level eglSurface.
This bug blocks bug 769167 which is blocking basecamp.
blocking-basecamp: --- → ?
Michael, does this still need to block Basecamp? And would you be a good assignee? Thanks.
Assignee: nobody → mwu
(In reply to Andrew Overholt [:overholt] from comment #7) > Michael, does this still need to block Basecamp? And would you be a good > assignee? Thanks. Depends on whether we still consider bug 769167 a blocker, which it appears to be. I'm planning on cleaning this up and landing it next week.
Looks like bug 769167 isn't a blocker any more.
blocking-basecamp: + → ?
(In reply to Michael Wu [:mwu] from comment #8) > (In reply to Andrew Overholt [:overholt] from comment #7) > > Michael, does this still need to block Basecamp? And would you be a good > > assignee? Thanks. > > Depends on whether we still consider bug 769167 a blocker, which it appears > to be. While we've removed the blocker flag, that's only since it's not a development blocker. If that makes it unclear, feel free to re-nom it :) > I'm planning on cleaning this up and landing it next week. Great, thanks!
blocking-basecamp: ? → +
I wonder if Andrew meant to blocking+ this. I don't think it needs to block, though I will be happy to take this patch of course.
blocking-basecamp: + → ?
Correct: this doesn't technically block release, but is highest-not-blocking-priority because we need this to run tests on real HW.
blocking-basecamp: ? → -
Sorry, you're right.
The ifdef for the swap buffers call is a little weird. I'm open to suggestions there. Another issue is that hwcomposer is a ICS specific thing. This patch just assumes ICS, but I'll try to get it building on GB before landing.
You can ignore GB at this point.
it is hard to ignore something that has public tinderbox builds... can we just check for some ICS/GB specific define in headers and use that for compile time detection?
We will only ship ICS-kernel based builds, but we can stick in a define if you like for testing purposes.
Comment on attachment 653518 [details] [diff] [review] Use HWComposer instead of swapBuffer where appropriate Need to fall back on SwapBuffers() when we can't initialize the HAL. Is HWC available on all our ICS builds? I assume so, but want to confirm.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #18) > Comment on attachment 653518 [details] [diff] [review] > Use HWComposer instead of swapBuffer where appropriate > > Need to fall back on SwapBuffers() when we can't initialize the HAL. > > Is HWC available on all our ICS builds? I assume so, but want to confirm. Yup. See http://androidxref.com/4.0.4/xref/hardware/libhardware/modules/hwcomposer/hwcomposer.cpp which is the default implementation for most devices that aren't omap.
Comment on attachment 653518 [details] [diff] [review] Use HWComposer instead of swapBuffer where appropriate >commit 981d5b41a9a0d1c3cba3544b2bd279ad7c108c89 >Author: Michael Wu <email@example.com> >Date: Mon Aug 20 17:02:07 2012 -0400 > > Use HWComposer to swap buffers > >diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp >index f214192..e6cb1b8 100644 >--- a/gfx/gl/GLContextProviderEGL.cpp >+++ b/gfx/gl/GLContextProviderEGL.cpp >+ if (!aIsOffscreen) >+ mHwc = new HWComposer(); Sorry, I think my first comment wasn't clear enough. If initializing hwc fails, we'll just crash trying to use it below.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #20) > Sorry, I think my first comment wasn't clear enough. If initializing hwc > fails, we'll just crash trying to use it below. Ah. Why not crash, though? If hwc init fails, something has gone horribly wrong anyway and a crash report would help us see it instead of letting things silently fail.
Attachment #655103 - Flags: review?(jones.chris.g) → review+
We'll need to land this before we get pandaboard automation going.
blocking-basecamp: - → ?
Target Milestone: --- → mozilla18
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
This broke B2G GB builds: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=B2G%20gb_armv7a_gecko%20mozilla-inbound%20build&rev=6075cc5dbe31 The push before was also busted, but that was caused by the now backed out bug 749518. B2G on GB was hidden on try, inbound and m-c - when I came to this bug I had no idea why, but I'm now guessing because of: (In reply to Andreas Gal :gal from comment #15) > You can ignore GB at this point. I've just spent a while tracking this down, after fx-team experienced B2G bustage (due to them not having GB B2G builds hidden). Please can we communicate the deliberate hiding of builds like this to sheriffs a bit more, just so we know what's going on & can make sure they get hidden across all trunk trees, and not just the main three :-) Also, presuming we can stop running the GB builds now? Want me to file the bug for that?
(In reply to Ed Morley [:edmorley] from comment #26) > more, just so we know what's going on & can make sure they get hidden across > all trunk trees, and not just the main three :-) Have also hidden on: Fx-team, Ionmonkey, Services-Central, Build-System, Alder, Maple, Pine,
Bug 780915 was filed to remove the builds
You need to log in before you can comment on or make changes to this bug.