Add API for scrolling individual layers #590
Conversation
|
@glennw r? |
| } | ||
|
|
||
| found_layer = true; | ||
| scrolled_a_layer = layer.set_scroll_origin(&origin); |
There was a problem hiding this comment.
probably you want this to be scrolled_a_layer |= layer.set_scroll_origin(&origin), right?
Otherwise (if we can just scroll one layer) we might as well break from the loop when that happens.
There was a problem hiding this comment.
Nice catch! I will fix this.
In practice this doesn't cause a problem because all layers with the same ScrollRootId should be be in sync. On the other hand, it is good to write resilient code, which is what I tried to do here, but failed.
|
|
||
| if !found_layer { | ||
| let scroll_offsets = | ||
| self.pending_scroll_offsets.entry(pipeline_id).or_insert(HashMap::new()); |
There was a problem hiding this comment.
nit: or_insert_with(HashMap::new) doesn't call HashMap::new() unconditionally, though I don't know if that allocates so it could be meaningless.
There was a problem hiding this comment.
or_insert_with is much better here. Thanks!
mrobinson
force-pushed
the
scroll-to-fragment
branch
from
979f65d to
bf9ad79
Compare
| @@ -52,6 +52,7 @@ pub struct Frame { | |||
| AuxiliaryLists, | |||
| BuildHasherDefault<FnvHasher>>, | |||
| pub root_scroll_layer_id: Option<ScrollLayerId>, | |||
| pending_scroll_offsets: HashMap<PipelineId, HashMap<ServoScrollRootId, LayerPoint>>, | |||
There was a problem hiding this comment.
wouldn't it be more efficient to have a single HashMap<(PipelineId, ServoScrollRootId), LayerPoint>?
There was a problem hiding this comment.
Yeah, that is much nicer!
| @@ -86,6 +86,26 @@ impl Layer { | |||
| LayerSize::new(overscroll_x, overscroll_y) | |||
| } | |||
|
|
|||
| pub fn set_scroll_origin(&mut self, origin: &LayerPoint) -> bool{ | |||
|
Looks good once @kvark's comments are addressed. |
This will allow Servo to reimplement scroll_to_fragment.
mrobinson
force-pushed
the
scroll-to-fragment
branch
from
bf9ad79 to
c33d8e4
Compare
|
@bors-servo r=glennw Thanks everyone for the reviews! |
|
📌 Commit c33d8e4 has been approved by |
|
⚡ Test exempted - status |
Add API for scrolling individual layers This will allow Servo to reimplement scroll_to_fragment. <!-- 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/590) <!-- Reviewable:end -->
This will allow Servo to reimplement scroll_to_fragment.
This change is