Skip to content
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

Introducing a regular clip inside a sticky clip results in bad behaviour and stack overflow #1751

Closed
staktrace opened this issue Sep 25, 2017 · 2 comments
Assignees

Comments

@staktrace
Copy link
Contributor

Run the scrolling.rs example, and scroll the nested frame. Observe how the dark blue rect (which is initially not visible, but becomes visible as you scroll the nested frame down) behaves. In particular, it is sticky at two different ranges, once when it is near the bottom of the nested scroll frame, and once more when it is near the top of the nested scroll frame.

Now apply this patch, which just introduces a single clip inside the sticky frame. Note that the clip rect is the same as the bounds of the rect inside the sticky frame, so this clip should have no visual effect.

diff --git a/webrender/examples/scrolling.rs b/webrender/examples/scrolling.rs
index d335206b..f9747c03 100644
--- a/webrender/examples/scrolling.rs
+++ b/webrender/examples/scrolling.rs
@@ -116,18 +116,21 @@ impl Example for App {
                     Some(StickySideConstraint {
                         margin: 10.0,
                         max_offset: -40.0,
                     }),
                     None,
                 ),
             );
             builder.push_clip_id(sticky_id);
+            let dummy_clip = builder.define_clip(None, (50, 350).by(50, 50), vec![], None);
+            builder.push_clip_id(dummy_clip);
             let info = LayoutPrimitiveInfo::new((50, 350).by(50, 50));
             builder.push_rect(&info, ColorF::new(0.5, 0.5, 1.0, 1.0));
+            builder.pop_clip_id(); // dummy_clip
             builder.pop_clip_id(); // sticky_id

             // just for good measure add another teal square further down and to
             // the right, which can be scrolled into view by the user
             let info = LayoutPrimitiveInfo::new((250, 350).to(300, 400));
             builder.push_rect(&info, ColorF::new(0.0, 1.0, 1.0, 1.0));

             builder.pop_clip_id(); // nested_clip_id

Build with this and scroll. Observe how (a) the rect doesn't stick any more and (b) if you scroll far enough the app crashes with this:

thread 'RenderBackend' has overflowed its stack
fatal runtime error: stack overflow

Process finished with exit code 134 (interrupted by signal 6: SIGABRT)

/cc @mrobinson

@staktrace
Copy link
Contributor Author

Looks like the overflow is due to infinite recursion in RenderTaskTree::max_depth

@kvark
Copy link
Member

kvark commented Sep 28, 2017

I'm getting a different result:

thread 'RenderBackend' panicked at 'assertion failed: (left == right)
left: FrameId(21),
right: FrameId(22)', src/gpu_cache.rs:594:8
note: Run with RUST_BACKTRACE=1 for a backtrace.
thread 'main' panicked at 'called Result::unwrap() on an Err value: Error { repr: Custom(Custom { kind: Other, error: StringError("cannot send on closed channel") }) }', /checkout/src/libcore/result.rs:860:4

Which corresponds to

debug_assert_eq!(block.last_access_time, self.frame_id);

@kvark kvark self-assigned this Sep 29, 2017
bors-servo pushed a commit that referenced this issue Sep 29, 2017
ClipScrollGroup reference & visibility

This PR makes us properly ignore the primitive runs culled out by the clip-scroll tree hierarchy.
Fixes #1751

r? @glennw

<!-- 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/1775)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants