-
Notifications
You must be signed in to change notification settings - Fork 290
Commit 6b29455

bors-servo
Auto merge of #1772 - christolliday:fix_gpu_location_panic, r=glennw
Fix 'RenderBackend' panicked at 'handle not requested or allocated!'
I started writing up an issue for this, but since it's not easily reproduced I did some debugging and found the issue, so thought I'd just make a PR.
After upgrading webrender I started seeing this crash in an example app using a custom webrender based UI library. I could reproduce it somewhat regularly but unfortunately not through a reliable set of steps, so I don't have a simple test case.
```
thread 'RenderBackend' panicked at 'handle not requested or allocated!', /checkout/src/libcore/option.rs:839:4
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
1: std::sys_common::backtrace::_print
at /checkout/src/libstd/sys_common/backtrace.rs:71
2: std::panicking::default_hook::{{closure}}
at /checkout/src/libstd/sys_common/backtrace.rs:60
at /checkout/src/libstd/panicking.rs:381
3: std::panicking::default_hook
at /checkout/src/libstd/panicking.rs:397
4: std::panicking::rust_panic_with_hook
at /checkout/src/libstd/panicking.rs:577
5: std::panicking::begin_panic
at /checkout/src/libstd/panicking.rs:538
6: std::panicking::begin_panic_fmt
at /checkout/src/libstd/panicking.rs:522
7: rust_begin_unwind
at /checkout/src/libstd/panicking.rs:498
8: core::panicking::panic_fmt
at /checkout/src/libcore/panicking.rs:71
9: core::option::expect_failed
at /checkout/src/libcore/option.rs:839
10: <core::option::Option<T>>::expect
at /checkout/src/libcore/option.rs:302
11: webrender::gpu_cache::GpuCache::get_address
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/gpu_cache.rs:592
12: webrender::tiling::<impl webrender::render_task::AlphaRenderItem>::add_to_batch
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/tiling.rs:364
13: webrender::tiling::AlphaBatcher::build
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/tiling.rs:745
14: <webrender::tiling::ColorRenderTarget as webrender::tiling::RenderTarget>::build
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/tiling.rs:1091
15: <webrender::tiling::RenderTargetList<T>>::build
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/tiling.rs:983
16: webrender::tiling::RenderPass::build
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/tiling.rs:1450
17: webrender::frame_builder::FrameBuilder::build
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/frame_builder.rs:2358
18: webrender::frame::Frame::build_frame::{{closure}}
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/frame.rs:1253
19: <core::option::Option<T>>::map
at /checkout/src/libcore/option.rs:398
20: webrender::frame::Frame::build_frame
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/frame.rs:1252
21: webrender::frame::Frame::build
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/frame.rs:1229
22: webrender::render_backend::Document::render
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/render_backend.rs:106
23: webrender::render_backend::RenderBackend::process_document
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/render_backend.rs:400
24: webrender::render_backend::RenderBackend::run
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/render_backend.rs:468
25: webrender::renderer::Renderer::new::{{closure}}
at /media/chris/linux_data/dev/rust/gui/webrender/webrender/src/renderer.rs:1713
```
The problem is that there's no gpu cache location set for an `AlphaRenderItem::Primitive` inside of `AlphaRenderItem::add_to_batch`, because during the previous call to `PrimitiveStore::prepare_prim_for_render` which is supposed fill the gpu cache, the early return in this block can prevent the call to `gpu_cache.request`:
let clip_task = if prim_clips.is_masking() {
// Take into account the actual clip info of the primitive, and
// mutate the current bounds accordingly.
let mask_rect = match prim_clips.bounds.outer {
Some(ref outer) => match prim_screen_rect.intersection(&outer.device_rect) {
Some(rect) => rect,
None => return None,
},
_ => prim_screen_rect,
};
The change just makes sure the gpu location is set before adding the item. The `is_some` method lets the location be checked without exposing the `CacheLocation` type.
<!-- 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/1772)
<!-- Reviewable:end -->1 file changed
+4
-1
lines changedwebrender/src/prim_store.rs
Copy file name to clipboardExpand all lines: webrender/src/prim_store.rs+4-1Lines changed: 4 additions & 1 deletion
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
1148 | 1148 |
| |
1149 | 1149 |
| |
1150 | 1150 |
| |
1151 |
| - | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
1152 | 1155 |
| |
1153 | 1156 |
| |
1154 | 1157 |
| |
|
0 commit comments