Jesse Schwartzentruber (:truber) Reporter
	Description • 6 years ago

The attached testcase causes a panic in m-c rev 20180910-75cc42919ab2.

thread 'WRRenderBackend#1' panicked at 'explicit panic', gfx/webrender/src/render_task.rs:43:9
stack backtrace:
   0:     0x7f46cc2fef02 - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::h845003dfd4057b58
                               at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1:     0x7f46cc2eff4c - std::panicking::default_hook::{{closure}}::h67eac88e3f053d87
                               at libstd/sys_common/backtrace.rs:71
   2:     0x7f46cc2ef3a9 - std::panicking::default_hook::hb9cbad724f559203
                               at libstd/panicking.rs:227
   3:     0x7f46cc2eef09 - std::panicking::rust_panic_with_hook::he4c3a67f6258a8f9
                               at libstd/panicking.rs:511
   4:     0x7f46cbe0aa46 - std::panicking::begin_panic::h11925d0f85f0dce2
                               at /checkout/src/libstd/panicking.rs:445
   5:     0x7f46cbee0e7e - webrender::render_task::RenderTask::new_mask::hae746ee8072a8348
                               at gfx/webrender/src/render_task.rs:43
                               at gfx/webrender/src/render_task.rs:331
                               at gfx/webrender/src/render_task.rs:495
   6:     0x7f46cbea6adc - webrender::prim_store::PrimitiveStore::prepare_prim_runs::h1585c64b58717007
                               at gfx/webrender/src/prim_store.rs:2273
                               at gfx/webrender/src/prim_store.rs:2782
                               at gfx/webrender/src/prim_store.rs:1780
                               at gfx/webrender/src/prim_store.rs:1895
   7:     0x7f46cbea9e7d - webrender::prim_store::PrimitiveStore::prepare_prim_runs::h1585c64b58717007
                               at gfx/webrender/src/prim_store.rs:1638
                               at gfx/webrender/src/prim_store.rs:1895
   8:     0x7f46cbea9e7d - webrender::prim_store::PrimitiveStore::prepare_prim_runs::h1585c64b58717007
                               at gfx/webrender/src/prim_store.rs:1638
                               at gfx/webrender/src/prim_store.rs:1895
   9:     0x7f46cbea9e7d - webrender::prim_store::PrimitiveStore::prepare_prim_runs::h1585c64b58717007
                               at gfx/webrender/src/prim_store.rs:1638
                               at gfx/webrender/src/prim_store.rs:1895
  10:     0x7f46cbea9e7d - webrender::prim_store::PrimitiveStore::prepare_prim_runs::h1585c64b58717007
                               at gfx/webrender/src/prim_store.rs:1638
                               at gfx/webrender/src/prim_store.rs:1895
  11:     0x7f46cbe9ef38 - webrender::frame_builder::FrameBuilder::build::h1c98fe49f6d82821
                               at gfx/webrender/src/frame_builder.rs:274
                               at gfx/webrender/src/frame_builder.rs:380
  12:     0x7f46cbe82fc7 - webrender::render_backend::Document::build_frame::hf67c4b1e58feb162
                               at gfx/webrender/src/render_backend.rs:256
  13:     0x7f46cbe5cade - webrender::render_backend::RenderBackend::update_document::h2cfb6446f18cba97
                               at gfx/webrender/src/render_backend.rs:986
  14:     0x7f46cbe53a3b - webrender::render_backend::RenderBackend::run::hf15e30940dd77ab7
                               at gfx/webrender/src/render_backend.rs:601
  15:     0x7f46cbe5008a - std::sys_common::backtrace::__rust_begin_short_backtrace::h406417e982951de3
                               at gfx/webrender/src/renderer.rs:1797
                               at /checkout/src/libstd/sys_common/backtrace.rs:136
  16:     0x7f46cbe4f56b - <F as alloc::boxed::FnBox<A>>::call_box::h2a5028f34bc1a8c3
                               at /checkout/src/libstd/thread/mod.rs:409
                               at /checkout/src/libstd/panic.rs:308
                               at /checkout/src/libstd/panicking.rs:310
                               at /checkout/src/libpanic_abort/lib.rs:39
                               at /checkout/src/libstd/panic.rs:392
                               at /checkout/src/libstd/thread/mod.rs:408
                               at /checkout/src/liballoc/boxed.rs:640
  17:     0x7f46cc2f1e11 - std::sys::unix::thread::Thread::new::thread_start::h7d7a420a78cfa84d
                               at /checkout/src/liballoc/boxed.rs:650

	Bobby Holley (:bholley)
	Comment 1 • 6 years ago

Here's a slightly reduced testcase.

	Bobby Holley (:bholley)
	Comment 2 • 6 years ago

(In reply to Bobby Holley (:bholley) from comment #1)
> Here's a slightly reduced testcase.

Note that the reduced testcase doesn't work on another low-resolution linux device, so it may be dependent on screen size.

	Bobby Holley (:bholley)
	Comment 3 • 6 years ago

So we're trying to render a clip mask RenderTask (ClipRegionTask) whose dimensions are too big.

We have some half-hearted clamping for this at render time here at [1]. But it doesn't help very much, because we abort shortly thereafter at [2]. Note that the particular value here ~10000 is between RENDER_TASK_SIZE_SANITY_CHECK (16k), which would cause panic at RenderTask creation-time, and the hardware max texture dimension (4K).

So presumably the blame lies earlier, either in the RenderBackend or in display list construction. The task gets created at [3], which in turn gets the size from the ClipItem::BoxShadow without validation. I can trace it back a bit more next time, but if somebody knows off-hand which part of the code should be responsible for validating the size for clip masks, I'm all ears.

[1] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/gfx/webrender/src/device/gl.rs#1116
[2] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/gfx/webrender/src/tiling.rs#240
[3] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/gfx/webrender/src/render_task.rs#473

	Glenn Watson [:gw]
	Comment 4 • 6 years ago

Yep, you're right that those checks in [1] and [2] are too late - we want to ensure we don't create tasks that are too big early on, since they are expensive to render and use.

With box shadows, we draw the box shadow via a clip mask to the texture cache, where it's persisted and used on subsequent frames (since they are quite expensive to render with the blur etc).

I haven't looked at the code in a long time, but what we generally try and do is find a minimal size box shadow corner and edge we can draw, and then we stretch that across the primitive when we are drawing it.

So we need to find out what in this test case results in creating a large box shadow task. It may be a simple bug, or it might be that the input data is resulting in a very large box shadow (e.g. a crazy overlapping border radius?).

What we should do in that case is downscale the size of the render task that we create (taking into account aspect ratio etc).

When implemented correctly, the worst case is that in these edge cases, the box shadow is drawn at a slightly lower resolution than ideal, but is still stretched across the primitive correctly. This wouldn't really matter since box shadows are blurred anyway.

	Bobby Holley (:bholley)
	Comment 5 • 6 years ago

I'd like to get back to this at some point but won't likely in the next few days, so someone else is welcome to steal it in the interim.

	u480271 Assignee
	Comment 6 • 6 years ago

I'll have a look at this.

	Josh Matthews [:jdm]
	Comment 7 • 6 years ago

To reproduce the RenderTask::new_mask crash, all I have to do is scroll down https://www.magicleap.com/creator.

	u480271 Assignee
	Comment 8 • 6 years ago

https://github.com/servo/webrender/pull/3230

	u480271 Assignee
	Comment 9 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=570acf1b68318b04a12500825da22b71f0ae103c&selectedJob=209075983

	Jeff Muizelaar [:jrmuizel]
	Comment 10 • 6 years ago

The magicleap crash is bug 1505934. This test case is fixed.

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 6 years ago

Can we land a crashtest for this?