-
Notifications
You must be signed in to change notification settings - Fork 306
Move all layer tree operations to a ScrollTree struct #754
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
Conversation
This will make it easier to introduce the idea of reference frames into WebRender. This should not change behavior.
|
This looks good, and I've tested scrolling on a number of different sites. |
|
@bors-servo r+ |
|
📌 Commit 5687f7d has been approved by |
Move all layer tree operations to a ScrollTree struct This will make it easier to introduce the idea of reference frames into WebRender. This should not change behavior. <!-- 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/754) <!-- Reviewable:end -->
|
Thanks! |
|
☀️ Test successful - status-travis |
kvark
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff!
| } | ||
|
|
||
| scrolled_a_layer | ||
| self.scroll_tree.scroll(scroll_location, cursor, phase,) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra comma?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I will fix this in a followup.
| &DEFAULT_SCROLLBAR_COLOR, | ||
| PrimitiveFlags::Scrollbar(self.root_scroll_layer_id.unwrap(), | ||
| 4.0)); | ||
| context.builder.add_solid_rectangle( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't the new formatting (see below) against the style guidelines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, but the PrimitiveFlags::Scrollbar(self.root_scroll_layer_id.unwrap(), line wouldn't fit within the line length limit any other way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I don't know what the guidelines are for this case then
| pub fn collect_layers_bouncing_back(&self) | ||
| -> HashSet<ScrollLayerId, BuildHasherDefault<FnvHasher>> { | ||
| let mut layers_bouncing_back = HashSet::with_hasher(Default::default()); | ||
| for (scroll_layer_id, layer) in self.layers.iter() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit (unrelated to the PR): we could do something fancy like
self.layers.iter()
.filter(|(_, layer)| layer.scrolling.bouncing_back)
.map(|(id, _)| *id)
.collect()
This will make it easier to introduce the idea of reference frames
into WebRender. This should not change behavior.
This change is