Closed
Bug 1100727
Opened 10 years ago
Closed 10 years ago
[lollipop] Porting LL FrameBufferSurface to gonk
Categories
(Firefox OS Graveyard :: GonkIntegration, defect)
Firefox OS Graveyard
GonkIntegration
Tracking
(Not tracked)
RESOLVED
FIXED
2.1 S9 (21Nov)
People
(Reporter: pchang, Assigned: boris)
References
Details
Attachments
(1 file, 9 obsolete files)
6.37 KB,
patch
|
boris
:
review+
|
Details | Diff | Splinter Review |
Porting new FrameBufferSurface from LL to gonk.
Assignee | ||
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
Comment on attachment 8524278 [details] [diff] [review] Widget/Gonk patch from all-hammerhead Review of attachment 8524278 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/libdisplay/FramebufferSurface.cpp @@ +49,5 @@ > * This implements the (main) framebuffer management. This class > * was adapted from the version in SurfaceFlinger > */ > +#if ANDROID_VERSION >= 21 > +FramebufferSurface::FramebufferSurface(int disp, uint32_t width, uint32_t height, uint32_t format, const sp<IGraphicBufferConsumer>& consumer) : It looks like you're switching from BufferQueue to IGraphicBufferConsumer. IGraphicBufferConsumer should be available in KK. Can you convert KK to use this too?
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → boris.chiou
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #2) > Comment on attachment 8524278 [details] [diff] [review] > Widget/Gonk patch from all-hammerhead > > Review of attachment 8524278 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/gonk/libdisplay/FramebufferSurface.cpp > @@ +49,5 @@ > > * This implements the (main) framebuffer management. This class > > * was adapted from the version in SurfaceFlinger > > */ > > +#if ANDROID_VERSION >= 21 > > +FramebufferSurface::FramebufferSurface(int disp, uint32_t width, uint32_t height, uint32_t format, const sp<IGraphicBufferConsumer>& consumer) : > > It looks like you're switching from BufferQueue to IGraphicBufferConsumer. > IGraphicBufferConsumer should be available in KK. Can you convert KK to use > this too? Thanks, I can check it in flame-kk now. BTW, this patch is just copied from b2g-l branch (patches/all-hammerhead/), so I will also revise the format of this patch.
Comment 4•10 years ago
|
||
Hm on second thought, I think it should be possible to adapt the current FramebufferSurface constructor to work here - we'd just have to make sure the function signature changes on KK and newer, but it'll let us avoid duplicating all the code in the constructor.
Comment hidden (obsolete) |
Assignee | ||
Comment 6•10 years ago
|
||
Use the new API, BufferQueue::createBufferQueue() to new the consumer and the producer on LL.
Attachment #8524408 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8524416 -
Flags: feedback?(pchang)
Reporter | ||
Comment 7•10 years ago
|
||
Comment on attachment 8524416 [details] [diff] [review] [WIP] Port LL FrameBufferSurface to gonk (v1.1) Review of attachment 8524416 [details] [diff] [review]: ----------------------------------------------------------------- Please consider to rename the GonkDispalyJB to align KK and LL. ::: widget/gonk/libdisplay/GonkDisplayJB.cpp @@ +137,5 @@ > +#else > + sp<BufferQueue> bq = new BufferQueue(true, mAlloc); > +#endif > + mFBSurface = new FramebufferSurface(0, mWidth, mHeight, surfaceformat, bq); > + I think we could also declare consumer and producer for KK since BufferQuere is their derived class. sp<BufferQueue>bq = new BufferQueue(mAlloc); producer = bq; consumer = bq; Then we could assign consumer for 'new FramebufferSurface' and produce for 'new Surface'
Attachment #8524416 -
Flags: feedback?(pchang)
Comment 8•10 years ago
|
||
I don't think there's much value in renaming GonkDisplayJB - it supports everything from JB to L.
Reporter | ||
Comment 9•10 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #8) > I don't think there's much value in renaming GonkDisplayJB - it supports > everything from JB to L. How about rename to GonkDisplay.cpp for latest android versions support? If there is no big difference between KK and LL, we could seperate JB into GonkDisplayJB.cpp and leave KK/LL in GonkDisplay.cpp.
Comment 10•10 years ago
|
||
(In reply to Boris Chiou [:boris] from comment #6) > Created attachment 8524416 [details] [diff] [review] > [WIP] Port LL FrameBufferSurface to gonk (v1.1) > > Use the new API, BufferQueue::createBufferQueue() to new the consumer > and the producer on LL. Hi Boris Patch (v1.1) will cause build break due to the underlying code change. Please adjust it accordingly. Thanks diff --git a/widget/gonk/libdisplay/FramebufferSurface.cpp b/widget/gonk/libdisplay/Frame index 073c9de..351285d 100644 --- a/widget/gonk/libdisplay/FramebufferSurface.cpp +++ b/widget/gonk/libdisplay/FramebufferSurface.cpp @@ -116,7 +116,7 @@ status_t FramebufferSurface::nextBuffer(sp<GraphicBuffer>& outBuffer, err = releaseBufferLocked(mCurrentBufferSlot, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR); #endif - if (err != NO_ERROR && err != BufferQueue::STALE_BUFFER_SLOT) { + if (err != NO_ERROR && err != IGraphicBufferConsumer::STALE_BUFFER_SLOT) { ALOGE("error releasing buffer: %s (%d)", strerror(-err), err); return err; }
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to wchi from comment #10) > (In reply to Boris Chiou [:boris] from comment #6) > > Created attachment 8524416 [details] [diff] [review] > > [WIP] Port LL FrameBufferSurface to gonk (v1.1) > > > > Use the new API, BufferQueue::createBufferQueue() to new the consumer > > and the producer on LL. > > Hi Boris > > Patch (v1.1) will cause build break due to the underlying code change. > Please adjust it accordingly. > Thanks > > diff --git a/widget/gonk/libdisplay/FramebufferSurface.cpp > b/widget/gonk/libdisplay/Frame > index 073c9de..351285d 100644 > --- a/widget/gonk/libdisplay/FramebufferSurface.cpp > +++ b/widget/gonk/libdisplay/FramebufferSurface.cpp > @@ -116,7 +116,7 @@ status_t > FramebufferSurface::nextBuffer(sp<GraphicBuffer>& outBuffer, > err = releaseBufferLocked(mCurrentBufferSlot, EGL_NO_DISPLAY, > EGL_NO_SYNC_KHR); > #endif > - if (err != NO_ERROR && err != BufferQueue::STALE_BUFFER_SLOT) { > + if (err != NO_ERROR && err != > IGraphicBufferConsumer::STALE_BUFFER_SLOT) { > ALOGE("error releasing buffer: %s (%d)", strerror(-err), err); > return err; > } Thanks for your help. I will fix this soon.
Comment hidden (obsolete) |
Assignee | ||
Comment 13•10 years ago
|
||
Use the new API, BufferQueue::createBufferQueue() to new the consumer and the producer on LL.
Attachment #8525189 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8525201 -
Flags: feedback?(pchang)
Assignee | ||
Updated•10 years ago
|
Attachment #8525201 -
Flags: feedback?(sotaro.ikeda.g)
Comment 14•10 years ago
|
||
Comment on attachment 8525201 [details] [diff] [review] Port LL FrameBufferSurface to gonk (v1.3) Review of attachment 8525201 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine. Inline CreateSurface and request review. ::: widget/gonk/libdisplay/GonkDisplayJB.cpp @@ +124,5 @@ > ALOGW("Couldn't show bootanimation (%s)", strerror(-error)); > } > > +sp<ANativeWindow> > +GonkDisplayJB::CreateSurface() Inline this. No point in making a new function.
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #14) > Comment on attachment 8525201 [details] [diff] [review] > Port LL FrameBufferSurface to gonk (v1.3) > > Review of attachment 8525201 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks fine. Inline CreateSurface and request review. > > ::: widget/gonk/libdisplay/GonkDisplayJB.cpp > @@ +124,5 @@ > > ALOGW("Couldn't show bootanimation (%s)", strerror(-error)); > > } > > > > +sp<ANativeWindow> > > +GonkDisplayJB::CreateSurface() > > Inline this. No point in making a new function. Thanks, Michael. I will put this code back.
Comment hidden (obsolete) |
Reporter | ||
Comment 17•10 years ago
|
||
Comment on attachment 8525201 [details] [diff] [review] Port LL FrameBufferSurface to gonk (v1.3) Review of attachment 8525201 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Regarding to separate the JB related code in this file, I'll leave it up to you ::: widget/gonk/libdisplay/GonkDisplayJB.cpp @@ +132,5 @@ > + sp<IGraphicBufferConsumer> consumer; > + BufferQueue::createBufferQueue(&producer, &consumer, mAlloc); > +#elif ANDROID_VERSION >= 19 > + sp<BufferQueue> consumer = new BufferQueue(mAlloc); > + sp<IGraphicBufferProducer> producer = consumer; I would prefer to declare sp<BufferQueue>bq and convert it to consumer and producer.
Attachment #8525201 -
Attachment is obsolete: false
Comment hidden (obsolete) |
Assignee | ||
Updated•10 years ago
|
Attachment #8525748 -
Flags: review?(sotaro.ikeda.g)
Updated•10 years ago
|
Attachment #8525748 -
Flags: review?(sotaro.ikeda.g) → review+
Comment hidden (obsolete) |
Assignee | ||
Updated•10 years ago
|
Attachment #8525766 -
Flags: review?(mwu) → review+
Comment hidden (obsolete) |
Assignee | ||
Comment 21•10 years ago
|
||
Use the new API, BufferQueue::createBufferQueue() to new the consumer and the producer on LL. Fix JB build errors (Android version 18). Try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e721dead57f
Attachment #8525766 -
Attachment is obsolete: true
Attachment #8525815 -
Attachment is obsolete: true
Attachment #8525839 -
Flags: review+
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Boris Chiou [:boris] from comment #21) > Created attachment 8525839 [details] [diff] [review] > Port LL FrameBufferSurface to gonk. (v2.3, carry mwu's r+) > > Use the new API, BufferQueue::createBufferQueue() to new the consumer > and the producer on LL. > > Fix JB build errors (Android version 18). > > Try server: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e721dead57f By the way, I tested this patch on my nexus-5-l device and it looks OK.
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2513d21bbff2
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S9 (21Nov)
You need to log in
before you can comment on or make changes to this bug.
Description
•