Introduce SVGPaintServer

Source/core/rendering/svg/RenderSVGResourcePattern.cpp

Index: Source/core/rendering/svg/RenderSVGResourcePattern.cpp
diff --git a/Source/core/rendering/svg/RenderSVGResourcePattern.cpp b/Source/core/rendering/svg/RenderSVGResourcePattern.cpp
index 19a1a1b5493cdd1cb7610193540cf9d432094001..a2dd73416b9ecfb913fe9032b0bcc9d0602911a9 100644
--- a/Source/core/rendering/svg/RenderSVGResourcePattern.cpp
+++ b/Source/core/rendering/svg/RenderSVGResourcePattern.cpp
@@ -19,11 +19,10 @@
  */
 
 #include "config.h"
-
 #include "core/rendering/svg/RenderSVGResourcePattern.h"
 
 #include "core/dom/ElementTraversal.h"
-#include "core/rendering/svg/SVGRenderSupport.h"
+#include "core/paint/SVGPaintServer.h"
 #include "core/rendering/svg/SVGRenderingContext.h"
 #include "core/svg/SVGFitToViewBox.h"
 #include "platform/graphics/GraphicsContext.h"
@@ -112,17 +111,16 @@ PatternData* RenderSVGResourcePattern::buildPattern(RenderObject* object, const
     return m_patternMap.set(object, patternData.release()).storedValue->value.get();
 }
 
-bool RenderSVGResourcePattern::applyResource(RenderObject* object, RenderStyle* style, GraphicsContext* context, RenderSVGResourceModeFlags resourceMode)
+SVGPaintServer RenderSVGResourcePattern::preparePaintServer(RenderObject* object, RenderStyle* style, RenderSVGResourceModeFlags resourceMode)
 {
     ASSERT(object);
     ASSERT(style);
-    ASSERT(context);
 
     clearInvalidationMask();
 
     SVGPatternElement* patternElement = toSVGPatternElement(element());
     if (!patternElement)
-        return false;
+        return SVGPaintServer::invalid();
 
     if (m_shouldCollectPatternAttributes) {
         patternElement->synchronizeAnimatedSVGAttribute(anyQName());
@@ -136,33 +134,18 @@ bool RenderSVGResourcePattern::applyResource(RenderObject* object, RenderStyle*
     // then the given effect (e.g. a gradient or a filter) will be ignored.
     FloatRect objectBoundingBox = object->objectBoundingBox();
     if (m_attributes.patternUnits() == SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX && objectBoundingBox.isEmpty())
-        return false;
+        return SVGPaintServer::invalid();
 
     PatternData* patternData = buildPattern(object, patternElement);
     if (!patternData)
-        return false;
+        return SVGPaintServer::invalid();
 
     const SVGRenderStyle& svgStyle = style->svgStyle();
 
     AffineTransform computedPatternSpaceTransform = computeResourceSpaceTransform(object, patternData->transform, svgStyle, resourceMode);
     patternData->pattern->setPatternSpaceTransform(computedPatternSpaceTransform);
 
-    // Draw pattern
-    context->save();
-
-    if (resourceMode & ApplyToFillMode)
-        context->setFillPattern(patternData->pattern);
-    else if (resourceMode & ApplyToStrokeMode)
-        context->setStrokePattern(patternData->pattern);
-
-    updateGraphicsContext(context, style, *object, resourceMode);
-    return true;
-}
-
-void RenderSVGResourcePattern::postApplyResource(GraphicsContext* context)
-{
-    ASSERT(context);
-    context->restore();
+    return SVGPaintServer(patternData->pattern);

[chrishtr, 2014/10/10 16:59:20]

Why not just have a method that exposes the data needed to construct the
SVGPaintServer, and have the constructor call entirely in the paint/ directory?
That would seem to be a cleaner separation of style and details of painting
algorithms, and avoid the need to link against something in the paint/ directory
here.

[fs, 2014/10/11 12:05:50]

Then I'd have to think, that it'd be better to just move SVGPaintServer to
rendering/ and move the updateGraphicsContext helper to SVGRenderSupport (for
now - it should be possible to move to paint/ in due time). Otherwise, since
there exists no one type this method can return, we'd need to build another
parameter blob - either exactly the same (structurally) as SVGPaintServer or a
conglomerate of *GradientAttributes and similar for Pattern [1] ( essentially
PatternData/GradientData but w/ a common base-class and different contents).
With the per-renderer caching moved out the latter seems more tractable [2], but
I'm 1-2 CLs away from being in a decent position for doing that.

So does the suggestion in the first sentence sound good enough for now? I'd be
happy to try and work towards the "larger param-blob" thingy and the caching
separation (which I was planning to do anyway, since there some stuff that would
benefit from that already). What side should do the caching could probably be
discussed further.

I must say though, that it's pretty darn hard to adapt to a design that you're
fed piece-meal on a need-to-know basis - feels a bit like being back in the army
;-). (Requested access to the doc, but no luck accessing it yet.)

Anyway, thanks for looking. I'll try to get something uploaded monday
(discussions are unfortunately not very practical w/ a ~1h window and 20h
latency).

[1] transform + display list/imagebuffer (or maybe root/list of content
elements/renderers or something)
[2] I.e. then you'd have a context blob in, and a parameter blob out.

 }
 
 static inline FloatRect calculatePatternBoundaries(const PatternAttributes& attributes,