Closed Bug 840871 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 22

People

(Reporter: sriram, Assigned: sriram)

References

Details

Attachments

(3 files, 2 obsolete files)

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?
Blocks: 819468
Attached patch Patch (obsolete) — Splinter Review
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)
Attached patch Part 1: TabsPanel (obsolete) — Splinter Review
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)
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: nobody → sriram
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-
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)
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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
You need to log in before you can comment on or make changes to this bug.