New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retain tiles just outside the viewport a bit longer. #3467

Merged
merged 1 commit into from Jan 4, 2019

Conversation

Projects
None yet
3 participants
@gw3583
Copy link
Collaborator

gw3583 commented Jan 4, 2019

Expand the size of the needed region that active tiles exist
over. This means that as tiles get scrolled just in / out of
the screen we retain them for a bit longer.

Since we only support a single dirty rect (for now), this is
helpful for avoiding the case of creating a large dirty rect
when something in the middle of the screen is animating while
scrolling. In future, once multiple dirty rects are supported,
it will be far less important to retain tiles outside the
viewport.

Also fix a subtle bug with the descriptor dependencies where
clipped out primitives would invalidate the tile due to changing
the length of the descriptor needed_rects array.


This change is Reviewable

@gw3583

This comment has been minimized.

Copy link
Collaborator

gw3583 commented Jan 4, 2019

This includes #3465, which is being reviewed separately. We can either wait until that's reviewed or close that and land both commits in this PR.

r? @bholley or @kvark

Pending picture caching try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2153754475503d6cf80886877f5f9a133d228e85

Retain tiles just outside the viewport a bit longer.
Expand the size of the needed region that active tiles exist
over. This means that as tiles get scrolled just in / out of
the screen we retain them for a bit longer.

Since we only support a single dirty rect (for now), this is
helpful for avoiding the case of creating a large dirty rect
when something in the middle of the screen is animating while
scrolling. In future, once multiple dirty rects are supported,
it will be far less important to retain tiles outside the
viewport.

Also fix a subtle bug with the descriptor dependencies where
clipped out primitives would invalidate the tile due to changing
the length of the descriptor needed_rects array.

@gw3583 gw3583 force-pushed the gw3583:less-evicts-3 branch from 43b0630 to 63a331d Jan 4, 2019

@gw3583

This comment has been minimized.

Copy link
Collaborator

gw3583 commented Jan 4, 2019

@bors-servo r=bholley (via email)

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 4, 2019

📌 Commit 63a331d has been approved by bholley

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 4, 2019

⌛️ Testing commit 63a331d with merge a40a5ff...

bors-servo added a commit that referenced this pull request Jan 4, 2019

Auto merge of #3467 - gw3583:less-evicts-3, r=bholley
Retain tiles just outside the viewport a bit longer.

Expand the size of the needed region that active tiles exist
over. This means that as tiles get scrolled just in / out of
the screen we retain them for a bit longer.

Since we only support a single dirty rect (for now), this is
helpful for avoiding the case of creating a large dirty rect
when something in the middle of the screen is animating while
scrolling. In future, once multiple dirty rects are supported,
it will be far less important to retain tiles outside the
viewport.

Also fix a subtle bug with the descriptor dependencies where
clipped out primitives would invalidate the tile due to changing
the length of the descriptor needed_rects array.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3467)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 4, 2019

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: bholley
Pushing a40a5ff to master...

@bors-servo bors-servo merged commit 63a331d into servo:master Jan 4, 2019

2 of 3 checks passed

Taskcluster (pull_request) TaskGroup: Pending (for pull_request.synchronize)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jan 4, 2019

Bug 1517723 - Update webrender to commit a40a5ffd1649d6bf6f55b76c7d63…
…3e4d157d4478 (WR PR #3467). r=kats

servo/webrender#3467

Differential Revision: https://phabricator.services.mozilla.com/D15722

--HG--
extra : moz-landing-system : lando

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Jan 5, 2019

@kvark
Copy link
Member

kvark left a comment

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gw3583)


webrender/src/picture.rs, line 506 at r1 (raw file):

        let needed_device_rect = needed_device_rect.inflate(
            0.0,
            3.0 * TILE_SIZE_HEIGHT as f32,

would be good to move it out as a const, unless this is strictly temporary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment