Paint code is called during PrePaint for SVG Filter |
|
Issue descriptionThis is a follow-up bug for bug 813446 . It crashed in paint code called from PrePaint because the required paint property doesn't exist: #7 0x000005b91965 blink::SVGForeignObjectPainter::Paint() #8 0x000005b90ae5 blink::LayoutSVGForeignObject::Paint() #9 0x0000059f30b2 blink::BoxPainter::PaintChildren() #10 0x000005a8f4e3 blink::SVGRootPainter::PaintReplaced() #11 0x0000058f28f5 blink::LayoutSVGRoot::PaintReplaced() #12 0x000005a87229 blink::ReplacedPainter::Paint() #13 0x0000057d9895 blink::LayoutReplaced::Paint() #14 0x000005a8cccd blink::SVGPaintContext::PaintResourceSubtree() #15 0x000005bfc27b blink::FEImage::CreateImageFilterForLayoutObject() #16 0x000005bfc51a blink::FEImage::CreateImageFilter() #17 0x0000046aed4f aura::WindowTreeHost::GetRootTransformForLocalEventCoordinates() #18 0x000005a06d15 blink::PaintFilterBuilder::Build() #19 0x000005a04932 blink::FilterEffectBuilder::BuildFilterOperations() #20 0x000005a3b627 blink::PaintLayer::UpdateCompositorFilterOperationsForFilter() #21 0x000005a70458 blink::ObjectPaintPropertyTreeBuilder::UpdateForSelf() #22 0x000005a64f2b blink::PrePaintTreeWalk::WalkInternal() #23 0x000005a64405 blink::PrePaintTreeWalk::Walk() #24 0x000005a6446b blink::PrePaintTreeWalk::Walk() #25 0x000005a6446b blink::PrePaintTreeWalk::Walk() #26 0x000005a6446b blink::PrePaintTreeWalk::Walk() #27 0x000005a63e84 blink::PrePaintTreeWalk::Walk() #28 0x000005a63326 blink::PrePaintTreeWalk::WalkTree() #29 0x0000052f146b blink::LocalFrameView::PrePaint() For bug 813446 I can workaround the crash by skipping access to paint properties that are not ready, but that will cause wrong filter created. We need to investigate the root cause. Perhaps we should defer updating SVG filters to the end of PrePaint.
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f6a82fc1c8b43938da813697c723c1db6834482b commit f6a82fc1c8b43938da813697c723c1db6834482b Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Thu Feb 22 19:36:38 2018 [SPv175] Don't crash on circular filter reference containing foreignObject or svg-in-svg This is a workaround for the crash bugs. The root cause is tracked in crbug.com/814815. Bug: 813446 , 813411 , 814815 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ied370eb2b1f0ffd4424c9c397ea1c899914bdbb0 Reviewed-on: https://chromium-review.googlesource.com/931922 Reviewed-by: Fredrik Söderquist <fs@opera.com> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#538527} [add] https://crrev.com/f6a82fc1c8b43938da813697c723c1db6834482b/third_party/WebKit/LayoutTests/external/wpt/svg/foreignobject/foreign-object-circular-filter-reference-crash.html [add] https://crrev.com/f6a82fc1c8b43938da813697c723c1db6834482b/third_party/WebKit/LayoutTests/external/wpt/svg/svg-in-svg/svg-in-svg-circular-filter-reference-crash.html [modify] https://crrev.com/f6a82fc1c8b43938da813697c723c1db6834482b/third_party/WebKit/Source/core/paint/SVGContainerPainter.cpp [modify] https://crrev.com/f6a82fc1c8b43938da813697c723c1db6834482b/third_party/WebKit/Source/core/paint/SVGForeignObjectPainter.cpp
,
Feb 22 2018
I'd say that the underlying issue is that there are too many filter representations with overlapping semantics. In this case we're attempting to generate a CompositorFilterOperations (glorified cc::FilterOperations wrapper) which can't take a chain of FilterEffects, but rather needs a PaintFilter (SkImageFilter wrapper.) To generate a PaintFilter, <feImage> (FEImage) may need to paint. If the code was dealing FilterEffect chains instead it wouldn't need to generate a PaintFilter, and this wouldn't happen. So currently it's: blink::FilterOperations -> CompositorFilterOperations, or blink::FilterOperations -> Filter/FilterEffect -> PaintFilter while it probably be better if it was: blink::FilterOperations -> Filter/FilterEffect -> CompositorFilterOperations, or blink::FilterOperations -> Filter/FilterEffect -> PaintFilter but IIRC, semantics is lost when converting shorthand filters into Filter/FilterEffect, so they won't be able to produce CompositorFilterOperations.
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70ed485eaa84d518a7ca6854e3573eb6142ac91d commit 70ed485eaa84d518a7ca6854e3573eb6142ac91d Author: Fredrik Söderquist <fs@opera.com> Date: Tue Apr 03 20:06:31 2018 [SPv175] Don't crash on <feImage> which reference an element with a mask This works around a crash where a layout box references a filter that contains a feImage filter primitive, which in turn reference content that has a mask applied. Bug: 814815, 825538 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ib2433b3ba48b3ea56462a1eb64209d36aa37cfa4 Reviewed-on: https://chromium-review.googlesource.com/982114 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: Fredrik Söderquist <fs@opera.com> Cr-Commit-Position: refs/heads/master@{#547802} [add] https://crrev.com/70ed485eaa84d518a7ca6854e3573eb6142ac91d/third_party/WebKit/LayoutTests/svg/masking/mask-within-feimage-filter-on-root-crash.html [modify] https://crrev.com/70ed485eaa84d518a7ca6854e3573eb6142ac91d/third_party/WebKit/Source/core/paint/SVGMaskPainter.cpp
,
Oct 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5 commit 9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Mon Oct 08 21:58:51 2018 [PE] Check and fix under-invalidation of GeometryMapper cache We invalidate GeometryMapper cache at the beginning of PrePaint, and expect that once the cache of a node is created, the node and its ancestors will never change. However, because of crbug.com/814815, when an SVG element is forward referenced, the cache of nodes from the SVG element's transform/clip node to the root will be updated when painting SVG filter or image for the element that forward references the SVG element. When PrePaint walks to the SVG element in the DOM tree order, the transform/clip ancestor chain may have changed and the cache previously updated may be out-dated. This CL detects the situation, print a warning and invalidate the cache when a transform or clip node changes when its cache has been updated. After crbug.com/814815 is fixed, the condition can be changed to a DCHECK. The CL also checks for visual viewport needing paint property update to invalidate the cache at the beginning of PrePaint. Bug: 814815, 892616 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ia51b03891a9a0448107d0ce7d33f7540e0ef2e32 Reviewed-on: https://chromium-review.googlesource.com/c/1266597 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#597695} [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/core/paint/paint_property_tree_update_tests.cc [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.cc [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.h [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/geometry_mapper_clip_cache.cc [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/geometry_mapper_clip_cache.h [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/geometry_mapper_transform_cache.cc [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/geometry_mapper_transform_cache.h [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/paint_property_node.h [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/scroll_paint_property_node.h [modify] https://crrev.com/9cdbb8acce4b90a37cf9f022bda2be2e6d9215a5/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h |
|
►
Sign in to add a comment |
|
Comment 1 by wangxianzhu@chromium.org
, Feb 22 2018