Closed Bug 786267 Opened 7 years ago Closed 7 years ago

B2G: Lower resolution while doing accelerated panning

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: drs, Assigned: drs)

References

Details

Attachments

(1 file, 3 obsolete files)

To make repainting during acceleration faster, we should lower the resolution we repaint at while doing this.
Depends on: 745136
Attached patch WIP (obsolete) — Splinter Review
WIP, will come back to this later. Unscientific tests on timecube.com show to me a massive increase in performance. Combined with better prediction I think we can make checkerboarding happen far less.
Blocks: 745136
No longer depends on: 745136
Please please please don't use timecube.com as any sort of benchmark. It does things (scaled repeated background image) that aren't representative of the rest of the web.

CNN, NYTimes, just about *anything* else is better.
(In reply to Joe Drew (:JOEDREW!) from comment #2)
> Please please please don't use timecube.com as any sort of benchmark. It
> does things (scaled repeated background image) that aren't representative of
> the rest of the web.
> 
> CNN, NYTimes, just about *anything* else is better.

but... but... it's so long!
Proposed patch. This actually works pretty well. It includes some additional code that gets the amount of time it took for a few previous paints and then tries to guess where the viewport will be panned to when the paint has finished. This works well if we get higher than 3-4 layer txns/second (otherwise it's just too slow and there's not really anything we can do).

I also snuck in a fix for pinch-to-zoom. It works slightly better now.
Assignee: nobody → bugzilla
Attachment #655988 - Attachment is obsolete: true
Attachment #657308 - Flags: review?(jones.chris.g)
Duplicate of this bug: 786282
Comment on attachment 657308 [details] [diff] [review]
Lower resolution while doing accelerated panning


>diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp

>+  // No velocity, no acceleration, no idea how long painting will take.

Move this comment to above the row of 0's.  Say why we don't have any
idea.

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp

>-bool AsyncPanZoomController::EnlargeDisplayPortAlongAxis(float aCompositionBounds,
>+bool AsyncPanZoomController::EnlargeDisplayPortAlongAxis(float aSkateSizeMultiplier,
>+                                                         double aEstimatedPaintDuration,
>+                                                         float aCompositionBounds,
>                                                          float aVelocity,
>+                                                         float aAcceleration,
>                                                          float* aDisplayPortOffset,
>                                                          float* aDisplayPortLength)

>+    // Position the area we paint such that all of the excess that extends past
>+    // the screen is on the side towards the velocity.
>+    *aDisplayPortOffset = aVelocity > 0 ? 0 : aCompositionBounds - *aDisplayPortLength;

Can we not fling/pan diagonally?  If we can, this would cause some serious
problems.  If not (as I recall), note that here.

We don't cap the computed sizes here, huh?  We should be doing that
for safety (with assertions). Can't upload anything with a dimension
>= 2000 pixels anyway.

> const gfx::Rect AsyncPanZoomController::CalculatePendingDisplayPort(
>   const FrameMetrics& aFrameMetrics,
>-  const gfx::Point& aVelocity)
>+  const gfx::Point& aVelocity,
>+  const gfx::Point& aAcceleration,
>+  double aEstimatedPaintDuration)
> {
>+  const float X_SKATE_SIZE_MULTIPLIER = 3.0f;
>+  const float Y_SKATE_SIZE_MULTIPLIER = 3.5f;
>+
>+  const float X_STATIONARY_SIZE_MULTIPLIER = 1.5f;
>+  const float Y_STATIONARY_SIZE_MULTIPLIER = 2.5f;
>+

Document these.

>+  // If we don't get an estimated paint duration, we probably don't have any
>+  // data. In this case, we're dealing with either a stationary frame or a first
>+  // paint. In either of these cases, don't really care and can just assume
>+  // it'll take 1 second to paint.

It's not that we don't /care/, it's that it doesn't really /matter/
since the user can't really initiate high-velocity flings before we
seed this data.  Plz update comment.

>   if (!enlargedX && !enlargedY) {
>-    displayPort.x = -displayPort.width / 4;
>-    displayPort.y = -displayPort.height / 4;
>+    // Position the x and y such that the screen falls in the middle of the displayport.

We want the screen to fall at the *top* of the displayport, yeah?
(When panning downwards.)  Then we have all those juicy prerendered
pixels to move around in while we wait on content to give us another
frame.

>+  double estimatedPaintDuration = 0.0;
>+  if (estimatedPaintSum > EPSILON) {
>+    estimatedPaintDuration = estimatedPaintSum / mPreviousPaintDurations.Length();
>+  }

We could do some smarter things for this but it's always best to take
the dumb solution as far as it can go.

Looks pretty good.  Would like to see one more version.
Attachment #657308 - Flags: review?(jones.chris.g)
Depends on: 784908
(In reply to Chris Jones [:cjones] [:warhammer] from comment #6)
> Can we not fling/pan diagonally?  If we can, this would cause some serious
> problems.  If not (as I recall), note that here.

We can fling diagonally, but I don't think it's realistically possible for a human to diagonally accelerate.

> We don't cap the computed sizes here, huh?  We should be doing that
> for safety (with assertions). Can't upload anything with a dimension
> >= 2000 pixels anyway.

I'm not sure of the specifics here. Is this in layers pixels? I would assume so since that's what the buffer is handled in. If so, I don't think it can ever go over the limit since the calculation is |compositeArea * constant| so as long as the constant is tuned correctly we should be fine. Can you clarify this and then I'll post a new version to deal with that?

For now, I'm putting up this new version that addresses everything else.
Attachment #657308 - Attachment is obsolete: true
Attachment #663841 - Flags: review?(jones.chris.g)
Attachment #663841 - Flags: review?(jones.chris.g) → review+
Rebased, r+ carried.
Attachment #663841 - Attachment is obsolete: true
Attachment #665262 - Flags: review+
Should we block on this, Doug/Chris?
This is an optimization that should make the browser appear to rerender faster while panning.  It's a small and safe change.

I wouldn't block but I would approve it.
https://hg.mozilla.org/mozilla-central/rev/33031c64f7d1
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.