Closed Bug 843888 Opened 9 years ago Closed 9 years ago

LayerView has 1 overdraw

Categories

(Firefox for Android Graveyard :: Toolbar, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 22

People

(Reporter: sriram, Assigned: kats)

Details

Attachments

(1 file)

Our LayerView has one overdraw all the time. This will reduce the performance of the app.

http://cl.ly/image/1E3l1r2C2z0S <-- The page are shouldn't have any overdraw. The overscroll color we show should be same as the url-bar without overdraw (they both are same color).

Steps:
1. Close Fennec
2. In Settings > Developer Options, turn on "Show GPU Overdraw"
3. Open Fennec
4. Navigate to a page.

This will show the LayerView in light blue, which means 1 overdraw.

To Note: Chrome renders the same page with no overdraw. The Google homepage's white is clearly seen.

How will it enhance performance? At every frame, we reduce one draw operation of the LayerView area (that occupies most of the screen).
Is this overdraw coming from the application background, or do we have a background set on the LayerView itself? Are you already looking at this, Sriram?
The background is not coming from the application. I am removing it once the app starts.
I am confused with the background of the LayerView. We set a #FFF in XML, because we want some color to show up until we start loading the page. But after that we should be changing it to transparent (may be). And there is a fix from bnicholson, where we change the background based on HTML page background. It also takes care of resetting it. So, I am really not sure if we have background or not -- at any instant.

Also, there is a overscroll color (light blue) that's drawn we drag the page more (or shrink it). That made me feel that could be the reason for 2 draws. But here is the catch! Even the overscroll part has 2 draws! It is showing a light blue overlay -- 1 overdraw.

And no, I am not working on this. :( I don't know any part of LayerView, LayerRender code :(
(In reply to Sriram Ramasubramanian [:sriram] from comment #2)
> Also, there is a overscroll color (light blue) that's drawn we drag the page
> more (or shrink it). That made me feel that could be the reason for 2 draws.
> But here is the catch! Even the overscroll part has 2 draws! It is showing a
> light blue overlay -- 1 overdraw.

That makes me think of bug 836818 -- intermittent failures in the robocop overscroll test.
I can reproduce the light blue on the page content area, but I'm not convinced that the overdraw detection is working properly on that area. I suspect that the overdraw detection will not work on SurfaceView just because we own the GL context of that area and android doesn't know what we're doing with it.

The reason I think the overdraw detection isn't working on that area is because it is solid blue everywhere. There are definitely parts where there would be more overdraw (e.g. the scrollbars, or the shadow around the page) and less overdraw (the overscroll area) because of how we composite the content. So it might be the case that there is one layer of overdraw just before the SurfaceView becomes visible, and once it's visible the overdraw indicator doesn't update.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> I can reproduce the light blue on the page content area, but I'm not
> convinced that the overdraw detection is working properly on that area. I
> suspect that the overdraw detection will not work on SurfaceView just
> because we own the GL context of that area and android doesn't know what
> we're doing with it.
> 
> The reason I think the overdraw detection isn't working on that area is
> because it is solid blue everywhere. There are definitely parts where there
> would be more overdraw (e.g. the scrollbars, or the shadow around the page)
> and less overdraw (the overscroll area) because of how we composite the
> content. So it might be the case that there is one layer of overdraw just
> before the SurfaceView becomes visible, and once it's visible the overdraw
> indicator doesn't update.

The overdraw indicator shows overdraw from the Android UI perspective, so it could be that it's taking the trouble to draw a background behind our SurfaceView - it won't indicate overdraw in our GL drawing, of which I imagine we have quite a bit :)

The blue background that you see during overscroll is rendered in LayerRenderer, this is not the background of the SurfaceView, so if there was a background you'd not see it there anyway.

I also am not totally convinced that it would work correctly in this area, but it's worth investigating I think.
Attached patch PatchSplinter Review
This removes the overdraw and as far as i can tell has no other adverse effects.
Attachment #718043 - Flags: review?(sriram)
Comment on attachment 718043 [details] [diff] [review]
Patch

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

That's easy win. Awesome :)
Attachment #718043 - Flags: review?(sriram) → review+
https://hg.mozilla.org/mozilla-central/rev/dba451863c29
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Assignee: nobody → bugmail.mozilla
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.