Center boot animation on a black background

RESOLVED FIXED

Status

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: mwu, Assigned: m1)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

7 years ago
Right now boot animation frames are stretched to the edges of the screen. We should center them on a black background like Android does.
Thanks to :mwu, who provided the main basis of this patch, here is a patch that fixes this and makes BootAnimation working correctly on devices like Nexus S. Please test it on Otoro/Unagi devices, of course :)
Posted video Result on Nexus S
Please find attached a video showing the result on Nexus S
Assignee: nobody → mvines
Posted patch apatch (obsolete) — Splinter Review
The new libdisplay stuff makes this patch a little smaller, which is nice.
Attachment #786084 - Flags: review?(mwu)
Attachment #786084 - Flags: feedback?(lissyx+mozillians)
Reporter

Comment 4

6 years ago
Comment on attachment 786084 [details] [diff] [review]
apatch

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

::: widget/gonk/libdisplay/BootAnimation.cpp
@@ +535,5 @@
> +                    for (int i = 0; i < frame.height; i++) {
> +                        memcpy(vaddr + i * buf->stride * frame.bytepp +
> +                               (buf->width - frame.width) / 2 * frame.bytepp,
> +                               frame.buf + i * frame.width * frame.bytepp,
> +                               frame.width * frame.bytepp);

Hmm this is a bit hard to read..

Can we add the left padding to the initial vaddr offset? That should let us avoid adding "(buf->width - frame.width) / 2 * frame.bytepp" every time, though maybe gcc would optimize that out. Seems easier to understand though.

We could also replace the multiplication with addition for accessing lines, which might be more clear if not faster. The original approach did that for the source buffer but not the destination buffer.
Posted patch bpatchSplinter Review
Should be a little nicer now.
Attachment #786084 - Attachment is obsolete: true
Attachment #786084 - Flags: review?(mwu)
Attachment #786084 - Flags: feedback?(lissyx+mozillians)
Attachment #786361 - Flags: review?(mwu)
Reporter

Comment 6

6 years ago
Comment on attachment 786361 [details] [diff] [review]
bpatch

Much nicer. Thanks
Attachment #786361 - Flags: review?(mwu) → review+
https://hg.mozilla.org/mozilla-central/rev/7b572e8c5a04
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
This is working nice, I had the chance to check on HTC Desire Z today :)
You need to log in before you can comment on or make changes to this bug.