Closed Bug 732013 Opened 12 years ago Closed 12 years ago

MAPLE: Unnecessary overdraw is causing framerate issues on high-resolution devices

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: cwiiis, Assigned: cwiiis)

Details

Attachments

(1 file)

On the Galaxy Nexus, the average maximum frame-rate seems to hover around 25fps. If I remove background drawing in LayerRenderer.java, this shoots back up to 60fps (with down-spikes during large-ish updates).

We could probably fix this by adding a mask to SingleTileLayer so that we don't cause so much overdraw when rendering the background and checkerboard layers. Working on a patch.

This is less noticeable on devices with lower-resolution screens (or, presumably, higher memory bandwidth/fill-rate).
Nice find! I commented out drawBackground() and am also seeing a huge jump in fps while panning around.
I didn't expect this to hurt so much with powervr's Tile Based Deferred Rendering (TBDR) but the problem is that we draw with alpha blending which doesn't help us reduce overdraw.
I think another offender might be RenderColorLayer.
This fixes the overdraw by adding an optional rectangular mask that can be set on SingleTileLayer and then splitting the draw up into the rectangles that comprise the masked region.
Attachment #602037 - Flags: review?(bugmail.mozilla)
Comment on attachment 602037 [details] [diff] [review]
Fix overdraw when drawing background and checkerboard

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

r+, but the RectUtils functions don't need to be added IMO, so feel free to take them out if you agree.

::: mobile/android/base/gfx/RectUtils.java
@@ +120,5 @@
> +    /** Returns the smallest integer rect that encompasses the given rect. */
> +    public static Rect roundOut(RectF rect) {
> +        return new Rect((int)Math.floor(rect.left), (int)Math.floor(rect.top),
> +                        (int)Math.ceil(rect.right), (int)Math.ceil(rect.bottom));
> +    }

Can we just use RectF.roundOut()?

@@ +126,5 @@
> +    /** Returns the largest integer rect that is encompassed by the given rect. */
> +    public static Rect roundIn(RectF rect) {
> +        return new Rect((int)Math.ceil(rect.left), (int)Math.ceil(rect.top),
> +                        (int)Math.floor(rect.right), (int)Math.floor(rect.bottom));
> +    }

This function doesn't seem to be used.

::: mobile/android/base/gfx/SingleTileLayer.java
@@ +126,1 @@
>  

I'm wondering if it's cheaper to create this float[] outside the loop and then update the coordinates for the x and y values. Probably not worth it because it would be harder to understand, but just a thought.
Attachment #602037 - Flags: review?(bugmail.mozilla) → review+
Thanks, made suggested changes and pushed: http://hg.mozilla.org/projects/maple/rev/e9c8164fab45

Didn't make the float[] change - I hope the compiler is clever enough to optimise things like that, and if it isn't and it makes a difference we can change it later.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Much better on the GN; nice work.

--
Tested via
Galaxy Nexus (Android 4.0.2)
20120302041248
http://hg.mozilla.org/projects/maple/rev/e595d6c1d837
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: