[Overdraw] Don't make all content visible on startup

RESOLVED FIXED in Firefox 22

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sriram, Assigned: sriram)

Tracking

unspecified
Firefox 22
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
The about:home (and other layers) are on top of a TabsPanel. Logically on startup, we don't show it right away. However, that's inflated as "visible", and a layer of about:home is drawn over it. Overdraw?
(Assignee)

Updated

5 years ago
Blocks: 819468
(Assignee)

Comment 1

5 years ago
Created attachment 713286 [details] [diff] [review]
Patch

This doesn't show TabsPanel when it starts up. This shows TabsPanel only when needed, and hides it when not shown.
We are going towards GREEEEEEN. http://cl.ly/image/3m3V383G2i0a
Attachment #713286 - Flags: review?(mark.finkle)
(Assignee)

Comment 2

5 years ago
Created attachment 713292 [details] [diff] [review]
Part 1: TabsPanel

Had a wrong value for an attribute. Rectified here.
Attachment #713286 - Attachment is obsolete: true
Attachment #713286 - Flags: review?(mark.finkle)
Attachment #713292 - Flags: review?(mark.finkle)
(Assignee)

Comment 3

5 years ago
Created attachment 713294 [details] [diff] [review]
Part 2: [WIP] LayerView

LayerView needn't be "visible" when Fennec starts up. If we are restoring a session, it can be shown. Or, we can leave about:home to load, and make it visible when user navigates to a new page.

I made it gone, and had the overdraw reduce further. Just BLUEUEUEUE: http://cl.ly/image/0x1c2h1Y3P08

However, I'm not sure where all we need to make it visible. Could someone from graphics take care of that part?
(Assignee)

Updated

5 years ago
Assignee: nobody → sriram
(Assignee)

Comment 4

5 years ago
Created attachment 713300 [details] [diff] [review]
Part 3: Thumbnails

The thumbnails are interesting. They can have a firefox logo with a translucent background when there is no bitmap available. They can have a "+" image with a translucent background when the space is empty. Or, they can have a thumbnail. In first two cases, we need a background. In the last case we don't need. That's a win! :D
Attachment #713300 - Flags: review?(mark.finkle)
Attachment #713292 - Flags: review?(mark.finkle) → review+
Comment on attachment 713294 [details] [diff] [review]
Part 2: [WIP] LayerView

Let's ask Kats
Attachment #713294 - Flags: feedback?(bugmail.mozilla)
Attachment #713300 - Flags: review?(mark.finkle) → review+
Comment on attachment 713294 [details] [diff] [review]
Part 2: [WIP] LayerView

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

In theory I don't have a problem with this being hidden on startup, as long as we show/hide it at the right times. AIUI this means we should:
1) show it in BrowserApp.hideAboutHome() (or wherever that is called)
2) hide it in BrowserApp.showAboutHome()
3) show it in WebApp.java.in's startup code path somewhere

I'm doing a build with these patches and can try to implement test what I just said above but it should be fairly straightforward so it might be faster if you do it as I may not get around to it until tomorrow or the day after.
Attachment #713294 - Flags: feedback?(bugmail.mozilla) → feedback+
Comment on attachment 713294 [details] [diff] [review]
Part 2: [WIP] LayerView

Actually after trying out this patch and looking at the behaviour I'm not sure this is a great idea. If the LayerView isn't visible it doesn't create the GL surface and so we cannot properly set up the compositor. This causes the Gecko and compositor threads to block until the surface is available (which is behaviour that is already causing us some grief and that we should fix). With our current startup code though this will be painful. If there's a significant benefit from this patch we should look harder at cleaning up the startup code path so that the compositor doesn't require a GL surface on start, or so we don't try to create a compositor until we have a GL surface ready for it.
Attachment #713294 - Flags: feedback+ → feedback-
(Assignee)

Comment 8

5 years ago
Created attachment 715641 [details] [diff] [review]
Part 1: TabsPanel

Turn out that with "gone", onLayout() will not be called. And so, there were regressions on tablet sidebars as the actual width is not known. Making it "invisible" forces the layout, and there is a valid width for animation, and also no overdraw.
Attachment #715641 - Flags: review?(mark.finkle)
(Assignee)

Updated

5 years ago
Attachment #713292 - Attachment is obsolete: true
Attachment #715641 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/6e7a420f099a
https://hg.mozilla.org/mozilla-central/rev/8ca0f27d8758
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
You need to log in before you can comment on or make changes to this bug.