[Webkit-unassigned] [Bug 25431] Simplify how SVG containers paint (and likely fix bugs by doing so)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25431


simon.fraser at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29827|review?                     |review+
               Flag|                            |




------- Comment #4 from simon.fraser at apple.com  2009-04-28 11:16 PDT -------
(From update of attachment 29827)
>  bool RenderSVGContainer::selfWillPaint() const
>  {
>  #if ENABLE(SVG_FILTERS)
> @@ -121,29 +110,29 @@ void RenderSVGContainer::paint(PaintInfo& paintInfo, int, int)
>       // Spec: groups w/o children still may render filter content.
>      if (!firstChild() && !selfWillPaint())
>          return;
> -    
> -    paintInfo.context->save();
> -    applyContentTransforms(paintInfo);
>  
> -    SVGResourceFilter* filter = 0;
> -    PaintInfo savedInfo(paintInfo);
> +    PaintInfo childPaintInfo(paintInfo);
>  
> -    FloatRect boundingBox = repaintRectInLocalCoordinates();
> -    if (paintInfo.phase == PaintPhaseForeground)
> -        prepareToRenderSVGContent(this, paintInfo, boundingBox, filter); 
> +    childPaintInfo.context->save();
> +
> +    // Let the RenderSVGViewportContainer subclass clip if necessary
> +    applyViewportClip(childPaintInfo);
>  
> -    applyAdditionalTransforms(paintInfo);
> +    applyTransformToPaintInfo(childPaintInfo, localToParentTransform());
> +
> +    SVGResourceFilter* filter = 0;
> +    FloatRect boundingBox = repaintRectInLocalCoordinates();
> +    if (childPaintInfo.phase == PaintPhaseForeground)
> +        prepareToRenderSVGContent(this, childPaintInfo, boundingBox, filter);
>  
> -    // default implementation. Just pass paint through to the children
> -    PaintInfo childInfo(paintInfo);
> -    childInfo.paintingRoot = paintingRootForChildren(paintInfo);
> +    childPaintInfo.paintingRoot = paintingRootForChildren(childPaintInfo);
>      for (RenderObject* child = firstChild(); child; child = child->nextSibling())
> -        child->paint(childInfo, 0, 0);
> +        child->paint(childPaintInfo, 0, 0);
>  
>      if (paintInfo.phase == PaintPhaseForeground)
> -        finishRenderSVGContent(this, paintInfo, boundingBox, filter, savedInfo.context);
> +        finishRenderSVGContent(this, childPaintInfo, boundingBox, filter, paintInfo.context);

Can we start to optimize painting by skipping children that are entirely
clipped out?

> +void RenderSVGImage::computeRectForRepaint(RenderBoxModelObject* repaintContainer, IntRect& repaintRect, bool fixed)
>  {
> -    // FIXME: handle non-root repaintContainer
> -    return m_absoluteBounds;
> +    // Translate to coords in our parent renderer, and then call computeRectForRepaint on our parent
> +    repaintRect = localToParentTransform().mapRect(repaintRect);
> +    parent()->computeRectForRepaint(repaintContainer, repaintRect, fixed);
>  }

It's a shame we can't just fall back on RenderBox::computeRectForRepaint(). If
we fixed that to be more
HTML-agnostic, could we?

>  void RenderSVGRoot::calcViewport()
> @@ -204,35 +182,59 @@ void RenderSVGRoot::calcViewport()
>      if (!selfNeedsLayout() && !svg->hasRelativeValues())
>          return;
>  
> -    SVGLength width = svg->width();
> -    SVGLength height = svg->height();
>      if (!svg->hasSetContainerSize()) {
> -        m_viewport = FloatRect(0, 0, width.value(svg), height.value(svg));
> -        return;
> +        // In the normal case of <svg> being stand-alone or in a CSSBoxModel object we use
> +        // RenderBox::width()/height() (which pulls data from RenderStyle)
> +        m_viewportSize = FloatSize(width(), height());
> +    } else {
> +        // In the SVGImage case grab the SVGLength values off of SVGSVGElement and use
> +        // the special relativeWidthValue accessors which respect the specified containerSize
> +        SVGLength width = svg->width();
> +        SVGLength height = svg->height();
> +        float viewportWidth = (width.unitType() == LengthTypePercentage) ? svg->relativeWidthValue() : width.value(svg);
> +        float viewportHeight = (height.unitType() == LengthTypePercentage) ? svg->relativeHeightValue() : height.value(svg);
> +        m_viewportSize = FloatSize(viewportWidth, viewportHeight);

Does this logic deserve to be wrapped in its own method?

>  TransformationMatrix RenderSVGRoot::localToParentTransform() const
>  {
> -    TransformationMatrix offsetTranslation;
> -    offsetTranslation.translate(x(), y());
> -    return localToParentTransformWithoutCSSParentOffset() * offsetTranslation;
> +    IntSize parentToBorderBoxOffset = parentOriginToBorderBox();
> +
> +    TransformationMatrix borderBoxOriginToParentOrigin;
> +    borderBoxOriginToParentOrigin.translate(parentToBorderBoxOffset.width(), parentToBorderBoxOffset.height());
> +
> +    return localToBorderBoxTransform() * borderBoxOriginToParentOrigin;
>  }

Some tests with <svg> inside transformed HTML would be good to see if this all
works.

> +void RenderSVGText::computeRectForRepaint(RenderBoxModelObject* repaintContainer, IntRect& repaintRect, bool fixed)
> +{
> +    // Translate to coords in our parent renderer, and then call computeRectForRepaint on our parent
> +    repaintRect = localToParentTransform().mapRect(repaintRect);
> +    parent()->computeRectForRepaint(repaintContainer, repaintRect, fixed);
>  }

Again, it's a shame we can't just drop into RenderBox::computeRectForRepaint()

> +    bool calculateLocalTransform();

What does the return value mean?

r=me


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.