Closed Bug 1429508 Opened 6 years ago Closed 6 years ago

thread 'WRRenderBackend#1' panicked at 'Vector image error Unknown error', /builds/worker/workspace/build/src/gfx/webrender/src/


(Core :: Graphics: WebRender, defect, P1)

59 Branch



Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- disabled
firefox60 --- fixed


(Reporter: jkratzer, Assigned: jnicol)


(Blocks 2 open bugs)


(Keywords: assertion, testcase)


(4 files)

Attached file trigger.html
Testcase found while fuzzing mozilla-central rev d5f42a23909e.

thread 'WRRenderBackend#1' panicked at 'Vector image error Unknown error', /builds/worker/workspace/build/src/gfx/webrender/src/
stack backtrace:
[Parent 10110, Main Thread] WARNING: attempt to modify an immutable nsStandardURL: file /builds/worker/workspace/build/src/netwerk/base/nsStandardURL.cpp, line 1698
[Parent 10110, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /builds/worker/workspace/build/src/parser/html/nsHtml5StreamParser.cpp, line 1009
   0:     0x7f3adaa470f3 - std::sys::imp::backtrace::tracing::imp::unwind_backtrace::h8ed7485deb8ab958
   1:     0x7f3adaa41840 - std::sys_common::backtrace::_print::h3d4f9ea58578e60f
   2:     0x7f3adaa545f3 - std::panicking::default_hook::{{closure}}::h0088fe51b67c687c
   3:     0x7f3adaa54362 - std::panicking::default_hook::hf425c768c5ffbbad
   4:     0x7f3adaa54af6 - std::panicking::rust_panic_with_hook::h25b934bb4484e9e0
   5:     0x7f3adaa54984 - std::panicking::begin_panic::h59483e27e93d7bc6
   6:     0x7f3adaa54889 - std::panicking::begin_panic_fmt::h5f221297e8a3dbdb
   7:     0x7f3ada7f954f - webrender::resource_cache::ResourceCache::block_until_all_resources_added::h6251023497fa883e
   8:     0x7f3ada8b50d7 - webrender::frame_builder::FrameBuilder::build::h14092b62004c54e0
   9:     0x7f3ada849d3b - webrender::render_backend::Document::render::h7823269006843dee
  10:     0x7f3ada84d53a - webrender::render_backend::RenderBackend::run::h72b63eaaa65524e3
  11:     0x7f3ada7b7b51 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5e11173f8d4d2ef9
  12:     0x7f3ada7b887d - std::panicking::try::do_call::h5b4eb2dbf08e40a1
  13:     0x7f3adaa593cb - __rust_maybe_catch_panic
Redirecting call to abort() to mozalloc_abort

OS|Linux|0.0.0 Linux 4.4.0-104-generic #127-Ubuntu SMP Mon Dec 11 12:16:42 UTC 2017 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
Flags: in-testsuite?
(pretty sure this belongs in the webrender component, at first glance)
Component: CSS Parsing and Computation → Graphics: WebRender
Assignee: nobody → jnicol
Priority: -- → P1
The call to CreateSimilarDrawTaraget in RecordedCreateSimilarDrawTarget::PlayEvent is failing, which leads to this panic.
CreateSimilarDrawTarget fails because the requested size is 181x182621, which is greater than the max surface size of 32767. This will always fail.

The page has a very tall nsTextControlFrame. In WebRenderCommandBuilder::GenerateFallbackData, there is a comment explaining why this is not clipped, saying we can rely on webrender to do so.

Indeed on the webrender side the blob image is a reasonable size (181x1024), and the base draw target when replaying is set to that size rather than the recorded size. However the error comes from creating the draw target for the CreateSimilarDrawTarget event, not the base draw target. This uses the recorded size rather than webrender's clipped size. This event originates from GenerateAndPushTextMask(), called from nsDisplayBackgroundColor::Paint().
Jeff, I've been thinking about this and I don't think there's any way we can clip the size of the CreateSimilarDrawTarget draw target when replaying. Certainly not in a general way. Do we therefore need to do some clipping in WebRenderCommandBuilder::GenerateFallbackData?
Flags: needinfo?(jmuizelaar)
There are some WIP patches here, following advice from Jeff on irc to record at a higher level. i.e. that the draw target will be used for a mask, so we don't need to record the size.

Recording without a size seems like a good approach. It was still panicking when replaying if you scrolled down the page - it would create the draw target from 0,0 rather than the same offset as the blob image's DT. I used a trick with DrawTargetTiled to get that to work. Then it was panicking because in DrawTargetSkia::PopLayer it needs to copy the mask DrawTargetTiled/SnapshotTiled to a non-tiled DataSourceSurface. There is an ugly and specific hack to make that work, but it needs generalised - possibly either by iterating the tiles or copying out a subregion.

The size of the mask draw target might not always be intended to be exactly the size of the base draw target, and we don't want to increase the size for the non-recording case. It might be possible to take the desired size and transform and be a bit cleverer.

I probably also need to figure out how to write a good test case to make sure I'm still rendering correctly.
We should be able to create the actual mask surface based on the clipped size when we're playing back.

I think it might also be useful to also lift up the application of the mask if we need to make assurances about it's size and transform.
Flags: needinfo?(jmuizelaar)
Comment on attachment 8948648 [details]
Bug 1429508 - Mark DrawTarget::GetSize as const.
Attachment #8948648 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8948649 [details]
Bug 1429508 - Make SnapshotTiled::GetDataSurface return a surface the size of backed tiles only.

These seem reasonable. I'm going to punt the review to Bas though.
Attachment #8948649 - Flags: review?(jmuizelaar) → review?(bas)
Attachment #8948650 - Flags: review?(jmuizelaar) → review?(bas)
Comment on attachment 8948649 [details]
Bug 1429508 - Make SnapshotTiled::GetDataSurface return a surface the size of backed tiles only.

This makes me cringe a little, but at the same time, I have no better ideas.
Attachment #8948649 - Flags: review?(bas) → review+
Comment on attachment 8948650 [details]
Bug 1429508 - Allow created mask surfaces to be clipped to the necessary size when replaying a recording.
Attachment #8948650 - Flags: review?(bas) → review+
Pushed by
Mark DrawTarget::GetSize as const. r=jrmuizel
Make SnapshotTiled::GetDataSurface return a surface the size of backed tiles only. r=bas
Allow created mask surfaces to be clipped to the necessary size when replaying a recording. r=bas
You need to log in before you can comment on or make changes to this bug.