Tidy up FEImage

 * Const qualify referenced objects (SVGPreserveAspectRatio, TreeScope).
 * Have ReferencedLayoutObject() return a const LayoutObject.
 * Rearrange the code in FEImage::MapInputs() and CreateImageFilter() to
   get the same general structure (element reference, external image,
   failure).
 * Reorder PaintRecord recording in CreateImageFilterForLayoutObject().
 * Mark local helper MakeMapBetweenRects() static. Split out "magic
   viewport"-mapping into a helper function.
 * Use DynamicTo<>() where applicable.

Change-Id: If9fe1cf076257cd680c05bb1c97e14ab2fe7c1c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2110013
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751974}

third_party/blink/renderer/core/svg/graphics/filters/svg_fe_image.cc

@@ -23,16 +23,18 @@
 
 #include "third_party/blink/renderer/core/svg/graphics/filters/svg_fe_image.h"
 
+#include "third_party/blink/renderer/core/dom/tree_scope.h"
 #include "third_party/blink/renderer/core/layout/layout_object.h"
 #include "third_party/blink/renderer/core/paint/svg_object_painter.h"
 #include "third_party/blink/renderer/core/svg/svg_element.h"
 #include "third_party/blink/renderer/core/svg/svg_length_context.h"
+#include "third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h"
 #include "third_party/blink/renderer/core/svg/svg_uri_reference.h"
 #include "third_party/blink/renderer/platform/graphics/filters/filter.h"
-#include "third_party/blink/renderer/platform/graphics/filters/paint_filter_builder.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_context.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_record.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_record_builder.h"
+#include "third_party/blink/renderer/platform/graphics/paint/paint_recorder.h"
+#include "third_party/blink/renderer/platform/graphics/skia/skia_utils.h"
 #include "third_party/blink/renderer/platform/transforms/affine_transform.h"
 #include "third_party/blink/renderer/platform/wtf/text/text_stream.h"
 
@@ -40,7 +42,7 @@ namespace blink {
 
 FEImage::FEImage(Filter* filter,
                  scoped_refptr<Image> image,
-                 SVGPreserveAspectRatio* preserve_aspect_ratio)
+                 const SVGPreserveAspectRatio* preserve_aspect_ratio)
     : FilterEffect(filter),
       image_(std::move(image)),
       tree_scope_(nullptr),
@@ -49,9 +51,9 @@ FEImage::FEImage(Filter* filter,
 }
 
 FEImage::FEImage(Filter* filter,
-                 TreeScope& tree_scope,
+                 const TreeScope& tree_scope,
                  const String& href,
-                 SVGPreserveAspectRatio* preserve_aspect_ratio)
+                 const SVGPreserveAspectRatio* preserve_aspect_ratio)
     : FilterEffect(filter),
       tree_scope_(&tree_scope),
       href_(href),
@@ -65,13 +67,13 @@ void FEImage::Trace(Visitor* visitor) {
   FilterEffect::Trace(visitor);
 }
 
-static FloatRect GetLayoutObjectRepaintRect(LayoutObject* layout_object) {
+static FloatRect GetLayoutObjectRepaintRect(const LayoutObject* layout_object) {
   return layout_object->LocalToSVGParentTransform().MapRect(
       layout_object->VisualRectInLocalSVGCoordinates());
 }
 
-AffineTransform MakeMapBetweenRects(const FloatRect& source,
-                                    const FloatRect& dest) {
+static AffineTransform MakeMapBetweenRects(const FloatRect& source,
+                                           const FloatRect& dest) {
   AffineTransform transform;
   transform.Translate(dest.X() - source.X(), dest.Y() - source.Y());
   transform.Scale(dest.Width() / source.Width(),
@@ -79,48 +81,58 @@ AffineTransform MakeMapBetweenRects(const FloatRect& source,
   return transform;
 }
 
-FloatRect FEImage::MapInputs(const FloatRect&) const {
-  LayoutObject* layout_object = ReferencedLayoutObject();
-  if (!image_ && !layout_object)
-    return FloatRect();
+static base::Optional<AffineTransform> ComputeViewportAdjustmentTransform(
+    const SVGElement* element,
+    const FloatRect& target_rect) {
+  // If we're referencing an element with percentage units, eg. <rect
+  // with="30%"> those values were resolved against the viewport.  Build up a
+  // transformation that maps from the viewport space to the filter primitive
+  // subregion.
+  // TODO(crbug/260709): This fixes relative lengths but breaks non-relative
+  // ones.
+  SVGLengthContext length_context(element);
+  FloatSize viewport_size;
+  if (!length_context.DetermineViewport(viewport_size))
+    return base::nullopt;
+  return MakeMapBetweenRects(FloatRect(FloatPoint(), viewport_size),
+                             target_rect);
+}
 
+FloatRect FEImage::MapInputs(const FloatRect&) const {
   FloatRect dest_rect =
       GetFilter()->MapLocalRectToAbsoluteRect(FilterPrimitiveSubregion());
-  FloatRect src_rect;
-  if (layout_object) {
-    src_rect = GetLayoutObjectRepaintRect(layout_object);
+  if (const LayoutObject* layout_object = ReferencedLayoutObject()) {
+    FloatRect src_rect = GetLayoutObjectRepaintRect(layout_object);
     auto* context_node = To<SVGElement>(layout_object->GetNode());
 
     if (context_node->HasRelativeLengths()) {
-      // FIXME: This fixes relative lengths but breaks non-relative ones (see
-      // crbug/260709).
-      SVGLengthContext length_context(context_node);
-      FloatSize viewport_size;
-      if (length_context.DetermineViewport(viewport_size)) {
-        src_rect = MakeMapBetweenRects(FloatRect(FloatPoint(), viewport_size),
-                                       dest_rect)
-                       .MapRect(src_rect);
-      }
+      auto viewport_transform =
+          ComputeViewportAdjustmentTransform(context_node, dest_rect);
+      if (viewport_transform)
+        src_rect = viewport_transform->MapRect(src_rect);
     } else {
       src_rect = GetFilter()->MapLocalRectToAbsoluteRect(src_rect);
       src_rect.Move(dest_rect.X(), dest_rect.Y());
     }
     dest_rect.Intersect(src_rect);
-  } else {
-    src_rect = FloatRect(FloatPoint(), FloatSize(image_->Size()));
+    return dest_rect;
+  }
+  if (image_) {
+    FloatRect src_rect = FloatRect(FloatPoint(), FloatSize(image_->Size()));
     preserve_aspect_ratio_->TransformRect(dest_rect, src_rect);
+    return dest_rect;
   }
-  return dest_rect;
+  return FloatRect();
 }
 
-LayoutObject* FEImage::ReferencedLayoutObject() const {
+const LayoutObject* FEImage::ReferencedLayoutObject() const {
   if (!tree_scope_)
     return nullptr;
-  Element* href_element =
-      SVGURIReference::TargetElementFromIRIString(href_, *tree_scope_);
-  if (!href_element || !href_element->IsSVGElement())
+  const SVGElement* target = DynamicTo<SVGElement>(
+      SVGURIReference::TargetElementFromIRIString(href_, *tree_scope_));
+  if (!target)
     return nullptr;
-  return href_element->GetLayoutObject();
+  return target->GetLayoutObject();
 }
 
 WTF::TextStream& FEImage::ExternalRepresentation(WTF::TextStream& ts,
@@ -128,7 +140,7 @@ WTF::TextStream& FEImage::ExternalRepresentation(WTF::TextStream& ts,
   IntSize image_size;
   if (image_) {
     image_size = image_->Size();
-  } else if (LayoutObject* layout_object = ReferencedLayoutObject()) {
+  } else if (const LayoutObject* layout_object = ReferencedLayoutObject()) {
     image_size =
         EnclosingIntRect(GetLayoutObjectRepaintRect(layout_object)).Size();
   }
@@ -150,54 +162,44 @@ sk_sp<PaintFilter> FEImage::CreateImageFilterForLayoutObject(
   auto* context_node = To<SVGElement>(layout_object.GetNode());
 
   if (context_node->HasRelativeLengths()) {
-    SVGLengthContext length_context(context_node);
-    FloatSize viewport_size;
-
-    // If we're referencing an element with percentage units, eg. <rect
-    // with="30%"> those values were resolved against the viewport.  Build up a
-    // transformation that maps from the viewport space to the filter primitive
-    // subregion.
-    if (length_context.DetermineViewport(viewport_size))
-      transform =
-          MakeMapBetweenRects(FloatRect(FloatPoint(), viewport_size), dst_rect);
+    auto viewport_transform =
+        ComputeViewportAdjustmentTransform(context_node, dst_rect);
+    if (viewport_transform)
+      transform = *viewport_transform;
   } else {
     transform.Translate(dst_rect.X(), dst_rect.Y());
   }
 
-  PaintRecordBuilder builder;
-  SVGObjectPainter(layout_object).PaintResourceSubtree(builder.Context());
-
   PaintRecorder paint_recorder;
   cc::PaintCanvas* canvas = paint_recorder.beginRecording(dst_rect);
   canvas->concat(AffineTransformToSkMatrix(transform));
-  builder.EndRecording(*canvas);
-
+  {
+    PaintRecordBuilder builder;
+    SVGObjectPainter(layout_object).PaintResourceSubtree(builder.Context());
+    builder.EndRecording(*canvas);
+  }
   return sk_make_sp<RecordPaintFilter>(
       paint_recorder.finishRecordingAsPicture(), dst_rect);
 }
 
 sk_sp<PaintFilter> FEImage::CreateImageFilter() {
-  if (auto* layout_object = ReferencedLayoutObject())
+  if (const auto* layout_object = ReferencedLayoutObject())
     return CreateImageFilterForLayoutObject(*layout_object);
 
-  PaintImage image =
-      image_ ? image_->PaintImageForCurrentFrame() : PaintImage();
-  if (!image) {
-    // "A href reference that is an empty image (zero width or zero height),
-    //  that fails to download, is non-existent, or that cannot be displayed
-    //  (e.g. because it is not in a supported image format) fills the filter
-    //  primitive subregion with transparent black."
-    return CreateTransparentBlack();
+  if (PaintImage image =
+          image_ ? image_->PaintImageForCurrentFrame() : PaintImage()) {
+    FloatRect src_rect = FloatRect(FloatPoint(), FloatSize(image_->Size()));
+    FloatRect dst_rect =
+        GetFilter()->MapLocalRectToAbsoluteRect(FilterPrimitiveSubregion());
+    preserve_aspect_ratio_->TransformRect(dst_rect, src_rect);
+    return sk_make_sp<ImagePaintFilter>(std::move(image), src_rect, dst_rect,
+                                        kHigh_SkFilterQuality);
   }
-
-  FloatRect src_rect = FloatRect(FloatPoint(), FloatSize(image_->Size()));
-  FloatRect dst_rect =
-      GetFilter()->MapLocalRectToAbsoluteRect(FilterPrimitiveSubregion());
-
-  preserve_aspect_ratio_->TransformRect(dst_rect, src_rect);
-
-  return sk_make_sp<ImagePaintFilter>(std::move(image), src_rect, dst_rect,
-                                      kHigh_SkFilterQuality);
+  // "A href reference that is an empty image (zero width or zero height),
+  //  that fails to download, is non-existent, or that cannot be displayed
+  //  (e.g. because it is not in a supported image format) fills the filter
+  //  primitive subregion with transparent black."
+  return CreateTransparentBlack();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/svg/graphics/filters/svg_fe_image.h

@@ -24,19 +24,23 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_SVG_GRAPHICS_FILTERS_SVG_FE_IMAGE_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_SVG_GRAPHICS_FILTERS_SVG_FE_IMAGE_H_
 
-#include "third_party/blink/renderer/core/dom/tree_scope.h"
-#include "third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h"
 #include "third_party/blink/renderer/platform/graphics/filters/filter_effect.h"
+#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
 class Image;
 class LayoutObject;
+class SVGPreserveAspectRatio;
+class TreeScope;
 
 class FEImage final : public FilterEffect {
  public:
-  FEImage(Filter*, scoped_refptr<Image>, SVGPreserveAspectRatio*);
-  FEImage(Filter*, TreeScope&, const String&, SVGPreserveAspectRatio*);
+  FEImage(Filter*, scoped_refptr<Image>, const SVGPreserveAspectRatio*);
+  FEImage(Filter*,
+          const TreeScope&,
+          const String&,
+          const SVGPreserveAspectRatio*);
 
   // feImage does not perform color interpolation of any kind, so doesn't
   // depend on the value of color-interpolation-filters.
@@ -49,7 +53,7 @@ class FEImage final : public FilterEffect {
 
  private:
   ~FEImage() override = default;
-  LayoutObject* ReferencedLayoutObject() const;
+  const LayoutObject* ReferencedLayoutObject() const;
 
   FilterEffectType GetFilterEffectType() const override {
     return kFilterEffectTypeImage;
@@ -62,11 +66,9 @@ class FEImage final : public FilterEffect {
 
   scoped_refptr<Image> image_;
 
-  // m_treeScope will never be a dangling reference. See
-  // https://bugs.webkit.org/show_bug.cgi?id=99243
-  Member<TreeScope> tree_scope_;
+  Member<const TreeScope> tree_scope_;
   String href_;
-  Member<SVGPreserveAspectRatio> preserve_aspect_ratio_;
+  Member<const SVGPreserveAspectRatio> preserve_aspect_ratio_;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.cc

@@ -186,7 +186,7 @@ bool SVGPreserveAspectRatio::Parse(const UChar*& ptr,
 }
 
 void SVGPreserveAspectRatio::TransformRect(FloatRect& dest_rect,
-                                           FloatRect& src_rect) {
+                                           FloatRect& src_rect) const {
   if (align_ == kSvgPreserveaspectratioNone)
     return;
 

third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h

@@ -72,7 +72,7 @@ class SVGPreserveAspectRatio final
   }
   SVGMeetOrSliceType MeetOrSlice() const { return meet_or_slice_; }
 
-  void TransformRect(FloatRect& dest_rect, FloatRect& src_rect);
+  void TransformRect(FloatRect& dest_rect, FloatRect& src_rect) const;
 
   AffineTransform ComputeTransform(float logical_x,
                                    float logical_y,