843888 - LayerView has 1 overdraw

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Sriram Ramasubramanian [:sriram]]

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).

Comment 1

[Chris Lord [:cwiiis]]

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?

Comment 2

[Sriram Ramasubramanian [: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 :(

Comment 3

[Geoff Brown [:gbrown]]

(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.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 5

[Chris Lord [:cwiiis]]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This removes the overdraw and as far as i can tell has no other adverse effects.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 718043 [details] [diff] [review]
Patch

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

That's easy win. Awesome :)

Comment 8

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dba451863c29

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dba451863c29