Closed Bug 1156140 Opened 9 years ago Closed 9 years ago

Choose different code path for boot animation based on device capability

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
2.2 S11 (1may)
Tracking Status
firefox40 --- fixed

People

(Reporter: chiajung, Assigned: chiajung)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch v1 (obsolete) — Splinter Review
In bug 1032180, I proposed a method to show boot animation on devices without hwc, however, that makes boot animation stops earlier than needed on devices with hwc(e.g., Flame).

In this bug, I want to make different devices run different pathes to get boot animation on both device and stop the animation as late as possible for hwc-compatible device.
Attachment #8594574 - Flags: feedback?(vliu)
Blocks: 1156141
LGTM. Thanks for your great help.
Attachment #8594574 - Flags: feedback?(vliu) → feedback+
Comment on attachment 8594574 [details] [diff] [review]
v1

@Michael Wu,
Can you help review this? We will search for better animation stop point in bug 1156141 base on this. Per talk last time, hwc-compatible device should need a better stop point only w/ this patch. fb only device should need more trick, which should be a follow up for bug 1156141 or something like bug 1055457.
Attachment #8594574 - Flags: review?(mwu)
Comment on attachment 8594574 [details] [diff] [review]
v1

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

The approach here looks good, and we can pass the animation buffer to gecko later if we want to extend the length of the animation. Good stuff - just have to polish the patch. Sotaro also probably bitrotted you a bit in bug 1152370.

The indentation in this patch is inconsistent - please use 4 spaces.

::: widget/gonk/libdisplay/GonkDisplayJB.cpp
@@ +132,5 @@
> +
> +    uint32_t usage = GRALLOC_USAGE_HW_FB | GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_COMPOSER;
> +    if (mFBDevice) {
> +        // If device uses fb, they can not use single buffer for boot animation
> +        mBootAnimBuffer = nullptr;

No need to set it to null - it defaults to null.

@@ +167,4 @@
>  ANativeWindow*
>  GonkDisplayJB::GetNativeWindow()
>  {
> +    if (mBootAnimBuffer.get() == nullptr) {

if (!mBootAnimBuffer.get()) {

@@ +235,4 @@
>  bool
>  GonkDisplayJB::SwapBuffers(EGLDisplay dpy, EGLSurface sur)
>  {
> +    if (mBootAnimBuffer.get() != nullptr) {

if (mBootAnimBuffer.get()) {

@@ +333,5 @@
>  GonkDisplayJB::QueueBuffer(ANativeWindowBuffer* buf)
>  {
>      bool success = Post(buf->handle, -1);
> +    int error = 0;
> +    if (mFBDevice) {

Checking mBootAnimBuffer would be more consistent.

@@ +342,5 @@
>  
>  void
>  GonkDisplayJB::UpdateFBSurface(EGLDisplay dpy, EGLSurface sur)
>  {
> +    if (mBootAnimBuffer.get() != nullptr) {

if (mBootAnimBuffer.get()) {
Attachment #8594574 - Flags: review?(mwu)
Attached patch v2Splinter Review
Rebase, fix indent, fix coding style.
Attachment #8594574 - Attachment is obsolete: true
Attachment #8597787 - Flags: review?(mwu)
Attachment #8597787 - Flags: review?(mwu) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/235ce90dd2d8
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S11 (1may)
Depends on: 1160877
You need to log in before you can comment on or make changes to this bug.