Closed Bug 939392 Opened 9 years ago Closed 9 years ago

[Homescreen] Invisible border causes layers to be much bigger

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed, b2g-v1.3 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
Because of this invisible border we extend the size of the layer to the bottom of the page. Without this border I can get the size of the layer on hidpi device to much smaller.

crdlc can you elaborate why we have this border? It seems to be slowing things down.
Attachment #833315 - Flags: review?(crdlc)
Notice how the layer is now just the upper 20% portion of the screen.
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Comment on attachment 833315 [details] [diff] [review]
patch

Very simple, it was added 3 or more months ago because when we changed adjacent pages from display: none to display: block, they were invisible (not repainted)
Attachment #833315 - Flags: review?(crdlc) → review+
(In reply to Cristian Rodriguez (:crdlc) from comment #2)
> Comment on attachment 833315 [details] [diff] [review]
> patch
> 
> Very simple, it was added 3 or more months ago because when we changed
> adjacent pages from display: none to display: block, they were invisible
> (not repainted)

Are you talking about Bug 842218 ?

We've experimented on HD display device.
Removing "border-bottom" did significantly reduce painting time and stutter issue is improved a lot.

Even if not removing "border-bottom", what we found was the previous/next pages were actually not pre-rendered in memory(2nd approach of https://bugzilla.mozilla.org/show_bug.cgi?id=842218#c16). Repainting still happened when swiping homescreen pages. Does it still meet your expectations on the fix for Bug 842218.

We're also struggling on this kind of issue, especially for HD display device. Bug 920921 records our course during the past 1~2 months.
Flags: needinfo?(crdlc)
(In reply to Vincent Lin[:vincentlin] from comment #3)
> (In reply to Cristian Rodriguez (:crdlc) from comment #2)
> > Comment on attachment 833315 [details] [diff] [review]
> > patch
> > 
> > Very simple, it was added 3 or more months ago because when we changed
> > adjacent pages from display: none to display: block, they were invisible
> > (not repainted)
> 
> Are you talking about Bug 842218 ?

Yes, so 8-9 months ago

> 
> We've experimented on HD display device.
> Removing "border-bottom" did significantly reduce painting time and stutter
> issue is improved a lot.
> 

Good news

> Even if not removing "border-bottom", what we found was the previous/next
> pages were actually not pre-rendered in memory(2nd approach of
> https://bugzilla.mozilla.org/show_bug.cgi?id=842218#c16). Repainting still
> happened when swiping homescreen pages. Does it still meet your expectations
> on the fix for Bug 842218.

:( At that moment it happened, adjacent pages go to display block and when they are translated following the finger, sometimes were invisible. I added that border and the issue was fixed. AFAIR if you add rgba(0,0,0,0.0.1) as background color to pages, there were visible as well like with border happens. I am not pretty sure but I this so

> 
> We're also struggling on this kind of issue, especially for HD display
> device. Bug 920921 records our course during the past 1~2 months.
Flags: needinfo?(crdlc)
Attached file Pull request 13853
Attachment #833315 - Attachment is obsolete: true
Attachment #8334847 - Flags: review+
Not sure if I need to wait until Travis completes before checkin-needed. Let me know.
Keywords: checkin-needed
Requesting koi+ since this is a big performance win.
blocking-b2g: --- → koi?
(In reply to Benoit Girard (:BenWa) from comment #8)
> Requesting koi+ since this is a big performance win.

What perf metrics would this help improve? FPS?
Benoit, what is the performance win, and do we have numbers?
Flags: needinfo?(bgirard)
Vincent you collect data for this I saw on gdocs. Can you share this publicly here?

(In reply to Milan Sreckovic [:milan] from comment #10)
> Benoit, what is the performance win, and do we have numbers?

Vincent has numbers. We saw that the allocation time (latency for the first 2 frames) was significantly reduced.

In any case it will save a half screen size buffer x 2 buffer x 2 buffer per layer * 4 bytes per pixel = ~2MB of gralloc.
Flags: needinfo?(bgirard) → needinfo?(vlin)
Profiling for the cost of "nsLayoutUtils::PaintFrame()" with 100 times sampling statistics.

w/t border-bottom
avg:202.980 ms, max:931.126 ms, min:4.438 ms, std:209.756

w/o border-bottom
avg:75.376 ms, max:474.395 ms, min:4.555 ms, std:99.589
(Note: the min is the time for the painting of Clock(Top-Right corner) during my profiling process.)

It's a significant improvement for stutter issue(The lag at the beginning of swiping homescreen).

But for transforming between two pages, it's irrelevant to this fix because there's no painting here.
I just create Bug 941984 for it.
Flags: needinfo?(vlin)
We know we're running into OOM situations, so 2MB is not something we should ignore. I'd say it's worth being koi+.
blocking-b2g: koi? → koi+
Given the perf win and a simple fix, taking this,
Reuben - Shouldn't this be closed as resolved fixed if this already landed in the above comment?
Flags: needinfo?(reuben.bmo)
Attached file Uplift to v1.2
Do we need additional approval step before merging?
(In reply to Jason Smith [:jsmith] from comment #15)
> Reuben - Shouldn't this be closed as resolved fixed if this already landed
> in the above comment?

Yes, every time I land a Gaia commit I forget it doesn't work like inbound :P
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: needinfo?(reuben.bmo)
Resolution: --- → FIXED
Uplifted 019275e2e193ce7e9b5ac5f362af93ac0773722a to:
v1.3 already had this commit
v1.2: cd7b3e6889c427f3068063b97498b0b6c6dcc7f5
You need to log in before you can comment on or make changes to this bug.