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)
Core
Graphics: WebRender
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.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
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
Assignee | ||
Comment 3•7 years ago
|
||
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
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(bugmail)
Comment 4•7 years ago
|
||
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)
Assignee | ||
Comment 5•7 years ago
|
||
(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!
Comment 6•7 years ago
|
||
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
Comment 7•7 years ago
|
||
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
See Also: → https://github.com/servo/webrender/pull/1623
Assignee | ||
Comment 8•7 years ago
|
||
(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.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Priority: -- → P3
Updated•7 years ago
|
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-mvp]
Target Milestone: --- → mozilla57
Updated•7 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → unaffected
Comment 10•7 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
Pushed by ethlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a99fc334476f Enable layers-free mode for some reftests. r=kats
Comment 13•7 years ago
|
||
Pushed by jmuizelaar@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0337d3437615 Remove duplicated gfxContext* parameter. r=mattwoodrow
Comment 14•7 years ago
|
||
Pushed by jmuizelaar@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/de03f659d5e6 Revert "Bug 1392523. Remove duplicated gfxContext* parameter. r=mattwoodrow"
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a99fc334476f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 16•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0337d3437615
You need to log in
before you can comment on or make changes to this bug.
Description
•