Closed Bug 1392523 Opened 7 years ago Closed 7 years ago

Turn on "layers-free" for some reftests on try server

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox56 --- unaffected
firefox57 --- unaffected

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

(Whiteboard: [wr-mvp])

Attachments

(1 file)

I think we need to turn on some reftests to prevent regressions, especially for tests about masking, clipping and transform.
It seems to cause crash on try server[1]. Not sure why I can't switch to the layers-free.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=acc3ea7c70cbf0ea09f9831cf00538a878a47815&selectedJob=124813551
The crash happens when the reftest switch to normal WR from layers-free mode. I can reproduce the problem with './mach reftest ...'. The stack backtrace is:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   7: core::panicking::panic_fmt
   8: core::option::expect_failed
   9: webrender::frame_builder::FrameBuilder::build
  10: webrender::render_backend::Document::render
  11: webrender::render_backend::RenderBackend::process_document
  12: webrender::render_backend::RenderBackend::run

I also found that the crash line is [1]. If I mark the wr_dp_push_clip() and wr_dp_pop_clip() in WebRenderAPI.cpp, the crash won't happen. I'm not familiar with the clip_scroll_tree. kats, do you have any idea about this? 


[1] https://github.com/servo/webrender/blob/d2c911f6ecfe08b55e1033802c2e42037315c303/webrender/src/frame_builder.rs#L1784
Flags: needinfo?(bugmail)
Hm, weird. Not sure off the top of my head why this would happen but I can look into it.
Assignee: ethlin → bugmail
Flags: needinfo?(bugmail)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Hm, weird. Not sure off the top of my head why this would happen but I can
> look into it.

Thanks!
I think I figured out what's going on. The call at [1] is turning a newly-generated clip id into a nested clip id even though the pipeline id doesn't match. The other two paths that call convert_id_to_nested both do pipeline id checks [2]. This results in a mismatch, because the newly-generated clip id is turned into a nested clip id, but when that same clip id is used as the scroll_node_id on a ClipAndScrollInfo, it is *not* turned into a nested clip id (because of the pipeline id check). This results in problems later because the item can't find its parent clip.

Here's a try push with a fix that seems to work locally: https://treeherder.mozilla.org/#/jobs?repo=try&revision=53dc0ceb1302c07f7f75bd1e1a11be62bb8824bd. If it's good I'll upstream it.

[1] http://searchfox.org/mozilla-central/rev/18c16ebf818abb86805ce08a6e537e4cd826f044/gfx/webrender/src/frame.rs#126
[2] http://searchfox.org/mozilla-central/rev/18c16ebf818abb86805ce08a6e537e4cd826f044/gfx/webrender/src/frame.rs#63,75
I filed https://github.com/servo/webrender/pull/1623 with the upstream fix. If it gets merged soonish we can pick it up in the next WR update. However the try push above has a couple of failures so the patch on this bug might need some tweaking to pick up only tests that are passing. I'll turn this back over to you now :)
Assignee: bugmail → ethlin
Depends on: 1394394
Depends on: 1396952
(In reply to Kartikaya Gupta (Away Sep02-Sep10) (email:kats@mozilla.com) from comment #7)
> I filed https://github.com/servo/webrender/pull/1623 with the upstream fix.
> If it gets merged soonish we can pick it up in the next WR update. However
> the try push above has a couple of failures so the patch on this bug might
> need some tweaking to pick up only tests that are passing. I'll turn this
> back over to you now :)

Right, "corner-joins-1.xhtml" seems to depend on apz. "animate-backface-hidden.html" depends on bug 1392200. I want to commit other changes first. We can keep adding more tests with layers-free on.
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-mvp]
Target Milestone: --- → mozilla57
Comment on attachment 8899770 [details]
Bug 1392523 - Enable layers-free mode for some reftests.

https://reviewboard.mozilla.org/r/171110/#review183272
Attachment #8899770 - Flags: review?(bugmail) → review+
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a99fc334476f
Enable layers-free mode for some reftests. r=kats
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0337d3437615
Remove duplicated gfxContext* parameter. r=mattwoodrow
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/de03f659d5e6
Revert "Bug 1392523. Remove duplicated gfxContext* parameter. r=mattwoodrow"
https://hg.mozilla.org/mozilla-central/rev/a99fc334476f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.