Skip to content

/ servo

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

Clip display list based on frame viewport #3809

Merged
merged 1 commit into from Nov 13, 2014
Merged

Clip display list based on frame viewport #3809

merged 1 commit into from Nov 13, 2014

Conversation

@mrobinson
Copy link
Member

mrobinson commented Oct 25, 2014

Instead of creating a display list for the entire page, only create one
for an area that expands around the viewport. On my machine this makes
incremental layout of http://timecube.com 50% faster.
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Oct 25, 2014

Critic review: https://critic.hoppipolla.co.uk/r/2974

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@mrobinson
Copy link
Member Author

mrobinson commented Oct 25, 2014

@pcwalton r? I'm curious how this improves performance for you, as I've had a little trouble getting good numbers locally.
@jdm
Copy link
Member

jdm commented Oct 25, 2014

This will definitely necessitate fixing #3790.
@mrobinson
Copy link
Member Author

mrobinson commented Oct 25, 2014

Oh yes, that's certainly correct. Do you want me to start working on #3790 or is someone already tackling it?
@pcwalton
Copy link
Contributor

pcwalton commented Oct 25, 2014

I'm not working on it—although it also will make the maze solver a lot faster because we can remove PseudoDisplayItem :)
@jdm
Copy link
Member

jdm commented Oct 25, 2014

@mrobinson: It would a big help if you could work on that.
@jdm
Copy link
Member

jdm commented Nov 4, 2014

Time for a rebase!
@mrobinson mrobinson force-pushed the mrobinson:display-list-optimization branch from 3f8c31f to 63a6079 Nov 5, 2014
@mrobinson
Copy link
Member Author

mrobinson commented Nov 5, 2014

Thanks! Rebased.
@mrobinson
Copy link
Member Author

mrobinson commented Nov 5, 2014

Er. I think I found a problem with the rebase, but I'll try to work things out and post a new version of the PR.
@mrobinson mrobinson force-pushed the mrobinson:display-list-optimization branch from 63a6079 to b131604 Nov 6, 2014
@mrobinson
Copy link
Member Author

mrobinson commented Nov 6, 2014

Okay. I had to make some adjustments for the smooth scrolling work, but now reference tests seem to be passing. Sorry for the churn.
components/script/page.rs Outdated

// We only need to move the clip rect if the viewport is getting near the edge of
// our preexisting clip rect. We use half of the size of the viewport as a heuristic
// for "close."

This comment has been minimized.

Sign in to view
@pcwalton

pcwalton Nov 8, 2014

Contributor

Can you factor 2.0 out into a constant so that we can easily tweak it later if we need?

This comment has been minimized.

Sign in to view
@mrobinson

mrobinson Nov 11, 2014

Author Member

Sure thing. I factored out both 2.0 constant used, including the one here and the one below.
@pcwalton
Copy link
Contributor

pcwalton commented Nov 8, 2014

r=me with a rebase and that one nit addressed. Sorry this took so long!
@mrobinson
Clip display list based on frame viewport
Loading status checks…
c732745 
Instead of creating a display list for the entire page, only create one
for an area that expands around the viewport. On my machine this makes
incremental layout of http://timecube.com 50% faster.
@mrobinson mrobinson force-pushed the mrobinson:display-list-optimization branch from b131604 to c732745 Nov 11, 2014
@mrobinson
Copy link
Member Author

mrobinson commented Nov 11, 2014

Thanks for the review!
@mrobinson

This comment has been minimized.

Sign in to view
Copy link
Owner Author

mrobinson commented on c732745 Nov 11, 2014

r=pcwalton

This comment has been minimized.

Sign in to view
Copy link
Owner Author

mrobinson replied Nov 13, 2014

@bors: retry
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented on c732745 Nov 11, 2014

saw approval from pcwalton
at mrobinson@c732745

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 11, 2014

merging mrobinson/servo/display-list-optimization = c732745 into auto

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 11, 2014

mrobinson/servo/display-list-optimization = c732745 merged ok, testing candidate = 49bf0d2

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 11, 2014

some tests failed:
failure: http://build.servo.org/builders/linux/builds/590

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 13, 2014

saw approval from pcwalton
at mrobinson@c732745

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 13, 2014

merging mrobinson/servo/display-list-optimization = c732745 into auto

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 13, 2014

mrobinson/servo/display-list-optimization = c732745 merged ok, testing candidate = 26045d7

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 13, 2014

all tests pass:
success: http://build.servo.org/builders/linux/builds/601
success: http://build.servo.org/builders/mac1/builds/143
success: http://build.servo.org/builders/mac2/builds/142

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 13, 2014

fast-forwarding master to auto = 26045d7
bors-servo pushed a commit that referenced this pull request Nov 11, 2014
bors-servo
auto merge of #3809 : mrobinson/servo/display-list-optimization, r=pc…
Loading status checks…
49bf0d2 
…walton

Instead of creating a display list for the entire page, only create one
for an area that expands around the viewport. On my machine this makes
incremental layout of http://timecube.com 50% faster.
@mrobinson

This comment has been minimized.

Sign in to view
Copy link
Owner Author

mrobinson commented on c732745 Nov 11, 2014

r=pcwalton

This comment has been minimized.

Sign in to view
Copy link
Owner Author

mrobinson replied Nov 13, 2014

@bors: retry
bors-servo pushed a commit that referenced this pull request Nov 13, 2014
bors-servo
auto merge of #3809 : mrobinson/servo/display-list-optimization, r=pc…
Loading status checks…
26045d7 
…walton

Instead of creating a display list for the entire page, only create one
for an area that expands around the viewport. On my machine this makes
incremental layout of http://timecube.com 50% faster.
@bors-servo bors-servo closed this Nov 13, 2014
@bors-servo bors-servo merged commit c732745 into servo:master Nov 13, 2014
1 check passed
1 check passed
default all tests passed
@mrobinson mrobinson deleted the mrobinson:display-list-optimization branch Nov 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@mrobinson @hoppipolla-critic-bot @jdm @pcwalton @bors-servo
You can’t perform that action at this time.