Changeset 180685 in webkit


Ignore:
Timestamp:
Feb 26, 2015 10:37:14 AM (9 years ago)
Author:
commit-queue@webkit.org
Message:

Cleanup RenderSVGResourceClipper class.
​https://bugs.webkit.org/show_bug.cgi?id=142032.

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2015-02-26
Reviewed by Darin Adler.
Source/WebCore:

This is a follow up for r180643: <​http://trac.webkit.org/changeset/180643>.
It includes cleanup for RenderSVGResourceClipper class.

  • rendering/svg/RenderSVGResourceClipper.cpp:

(WebCore::RenderSVGResourceClipper::applyClippingToContext):
(WebCore::RenderSVGResourceClipper::drawContentIntoMaskImage):

  • rendering/svg/RenderSVGResourceClipper.h: Change ClipperData to be a

typedef instead of a class and rename it to ClipperMaskImage. The purpose
of having it a class even though it includes only one member was because
we wanted it to be WTF_MAKE_FAST_ALLOCATED. We do not need to allocate it
as a separate object on the heap anymore.

(WebCore::RenderSVGResourceClipper::addRendererToClipper): Instead of doing
double hash table lookups by calling HashMap::contains() and then HashMap::get(),
we can use HashMap::add() instead.

LayoutTests:

  • svg/clip-path/clip-path-line-use-before-defined-expected.svg:
  • svg/clip-path/clip-path-line-use-before-defined.svg: Simplify the test

and make separate drawings for different cases.

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r180683 r180685  
     12015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Cleanup RenderSVGResourceClipper class.
     4        https://bugs.webkit.org/show_bug.cgi?id=142032.
     5
     6        Reviewed by Darin Adler.
     7
     8        * svg/clip-path/clip-path-line-use-before-defined-expected.svg:
     9        * svg/clip-path/clip-path-line-use-before-defined.svg: Simplify the test
     10        and make separate drawings for different cases.
     11
    1122015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
    213

  • trunk/LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg

    r180643 r180685  
    1 <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
     1<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    22  <defs>
    33    <style>
    44      line, path {
    55        stroke: black;
     6        stroke-width: 3;
    67      }
    78    </style>
    89    <clipPath id="clip-circle">
    9         <circle cx="50%" cy="50%" r="25%"/>
     10      <circle cx="75" cy="75" r="50"/>
    1011    </clipPath>
     12    <g id="circle-in-rect">
     13      <rect x="0" y="0" width="150" height="150" fill="lime"/>
     14      <rect x="0" y="0" width="150" height="150" fill="white" clip-path="url(#clip-circle)"/>
     15    </g>
    1116  </defs>
     17
     18  <use xlink:href="#circle-in-rect" transform="translate(10, 10)">
     19    <g clip-path="url(#clip-circle)">
     20      <!-- center lines -->
     21      <path d="M0 75 L150 75"/>
     22      <line x1="75" y1="0" x2="75" y2="150"/>
     23    </g>
     24  </use>
    1225 
    13   <rect x="0" y="0" width="100%" height="100%" fill="lime"/>
    14   <rect x="0" y="0" width="100%" height="100%" fill="white" clip-path="url(#clip-circle)"/>
     26  <use xlink:href="#circle-in-rect" transform="translate(170, 10)">
     27    <g clip-path="url(#clip-circle)">
     28      <!-- diagonal lines -->
     29      <path d="M0 0 L150 150"/>
     30      <line x1="0" y1="150" x2="150" y2="0"/>
     31    </g>
     32  </use>
    1533
    16   <!-- diagonal -->
    17   <line x1="170" y1="30" x2="30" y2="170" clip-path="url(#clip-circle)"/>
    18  
    19   <!-- top and left lines -->
    20   <path d="M30 75 L170 75" clip-path="url(#clip-circle)"/>
    21   <line x1="75" y1="30" x2="75" y2="170" clip-path="url(#clip-circle)"/>
    22 
    23   <!-- right and bottom lines -->
    24   <g clip-path="url(#clip-circle)">
    25     <path d="M30 125 L170 125"/>
    26     <line x1="125" y1="30" x2="125" y2="170"/>
    27   </g>
    28  
    29   <!-- center lines -->
    30   <path d="M30 100 L170 100" clip-path="url(#clip-circle)"/>
    31   <line x1="100" y1="30" x2="100" y2="170" clip-path="url(#clip-circle)"/>
    32  
     34  <use xlink:href="#circle-in-rect" transform="translate(330, 10)">
     35    <g clip-path="url(#clip-circle)">
     36      <!-- top and left lines -->
     37      <path d="M0 50 L150 50"/>
     38      <line x1="50" y1="0" x2="50" y2="150"/>
     39      <!-- bottom and right lines -->
     40      <path d="M0 100 L150 100"/>
     41      <line x1="100" y1="0" x2="100" y2="150"/>
     42    </g>
     43  </use>
    3344</svg>

  • trunk/LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg

    r180643 r180685  
    1 <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
     1<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    22  <defs>
    33    <style>
    44      line, path {
    55        stroke: black;
     6        stroke-width: 3;
    67      }
    78    </style>
     9    <g id="circle-in-rect">
     10      <rect x="0" y="0" width="150" height="150" fill="lime"/>
     11      <rect x="0" y="0" width="150" height="150" fill="white" clip-path="url(#clip-circle)"/>
     12    </g>
    813  </defs>
     14
     15  <use xlink:href="#circle-in-rect" transform="translate(10, 10)">
     16    <!-- center lines -->
     17    <path d="M0 75 L150 75" clip-path="url(#clip-circle)"/>
     18    <line x1="75" y1="0" x2="75" y2="150" clip-path="url(#clip-circle)"/>
     19  </use>
    920 
    10   <rect x="0" y="0" width="100%" height="100%" fill="lime"/>
    11   <rect x="0" y="0" width="100%" height="100%" fill="white" clip-path="url(#clip-circle)"/>
     21  <use xlink:href="#circle-in-rect" transform="translate(170, 10)">
     22    <!-- diagonal lines -->
     23    <path d="M0 0 L150 150" clip-path="url(#clip-circle)"/>
     24    <line x1="0" y1="150" x2="150" y2="0" clip-path="url(#clip-circle)"/>
     25  </use>
    1226
    13   <!-- diagonal -->
    14   <line x1="170" y1="30" x2="30" y2="170" clip-path="url(#clip-circle)"/>
    15  
    16   <!-- top and left lines -->
    17   <path d="M30 75 L170 75" clip-path="url(#clip-circle)"/>
    18   <line x1="75" y1="30" x2="75" y2="170" clip-path="url(#clip-circle)"/>
    19 
    20   <!-- bottom and right lines -->
    21   <g clip-path="url(#clip-circle)">
    22     <path d="M30 125 L170 125"/>
    23     <line x1="125" y1="30" x2="125" y2="170"/>
    24   </g>
     27  <use xlink:href="#circle-in-rect" transform="translate(330, 10)">
     28    <!-- top and left lines -->
     29    <path d="M0 50 L150 50" clip-path="url(#clip-circle)"/>
     30    <line x1="50" y1="0" x2="50" y2="150" clip-path="url(#clip-circle)"/>
     31    <g clip-path="url(#clip-circle)">
     32      <!-- bottom and right lines -->
     33      <path d="M0 100 L150 100"/>
     34      <line x1="100" y1="0" x2="100" y2="150"/>
     35    </g>
     36  </use>
    2537 
    2638  <defs>
    2739    <clipPath id="clip-circle">
    28       <circle cx="50%" cy="50%" r="50%" clip-path="url(#clip-small-circle)"/>
     40      <circle cx="75" cy="75" r="75" clip-path="url(#clip-small-circle)"/>
    2941    </clipPath>
    3042    <clipPath id="clip-small-circle">
    31       <circle cx="50%" cy="50%" r="25%"/>
     43      <circle cx="75" cy="75" r="50"/>
    3244    </clipPath>
    3345  </defs>
    34 
    35   <!-- center lines -->
    36   <path d="M30 100 L170 100" clip-path="url(#clip-circle)"/>
    37   <line x1="100" y1="30" x2="100" y2="170" clip-path="url(#clip-circle)"/>
    3846</svg>

  • trunk/Source/WebCore/ChangeLog

    r180683 r180685  
     12015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Cleanup RenderSVGResourceClipper class.
     4        https://bugs.webkit.org/show_bug.cgi?id=142032.
     5
     6        Reviewed by Darin Adler.
     7       
     8        This is a follow up for r180643: <http://trac.webkit.org/changeset/180643>.
     9        It includes cleanup for RenderSVGResourceClipper class.
     10
     11        * rendering/svg/RenderSVGResourceClipper.cpp:
     12        (WebCore::RenderSVGResourceClipper::applyClippingToContext):
     13        (WebCore::RenderSVGResourceClipper::drawContentIntoMaskImage):
     14        * rendering/svg/RenderSVGResourceClipper.h: Change ClipperData to be a
     15        typedef instead of a class and rename it to ClipperMaskImage. The purpose
     16        of having it a class even though it includes only one member was because
     17        we wanted it to be WTF_MAKE_FAST_ALLOCATED. We do not need to allocate it
     18        as a separate object on the heap anymore.
     19       
     20        (WebCore::RenderSVGResourceClipper::addRendererToClipper): Instead of doing
     21        double hash table lookups by calling HashMap::contains() and then HashMap::get(),
     22        we can use HashMap::add() instead.
     23
    1242015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
    225

  • trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp

    r180643 r180685  
    131131                                                      const FloatRect& repaintRect, GraphicsContext* context)
    132132{
    133     ClipperData* clipperData = addRendererToClipper(renderer);
    134     ASSERT(clipperData);
    135     bool shouldCreateClipData = !clipperData->clipMaskImage;
     133    ClipperMaskImage& clipperMaskImage = addRendererToClipper(renderer);
     134    bool shouldCreateClipperMaskImage = !clipperMaskImage;
    136135
    137136    AffineTransform animatedLocalTransform = clipPathElement().animatedLocalTransform();
    138137
    139     if (shouldCreateClipData && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
     138    if (shouldCreateClipperMaskImage && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
    140139        return true;
    141140
    … …  
    143142    SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer, absoluteTransform);
    144143
    145     if (shouldCreateClipData && !repaintRect.isEmpty()) {
    146         if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperData->clipMaskImage, ColorSpaceDeviceRGB, Unaccelerated))
    147             return false;
    148 
    149         GraphicsContext* maskContext = clipperData->clipMaskImage->context();
     144    if (shouldCreateClipperMaskImage && !repaintRect.isEmpty()) {
     145        if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperMaskImage, ColorSpaceDeviceRGB, Unaccelerated))
     146            return false;
     147
     148        GraphicsContext* maskContext = clipperMaskImage->context();
    150149        ASSERT(maskContext);
    151150
    … …  
    162161                return false;
    163162
    164             succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox);
     163            succeeded = drawContentIntoMaskImage(clipperMaskImage, objectBoundingBox);
    165164            // The context restore applies the clipping on non-CG platforms.
    166165        } else
    167             succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox);
     166            succeeded = drawContentIntoMaskImage(clipperMaskImage, objectBoundingBox);
    168167
    169168        if (!succeeded)
    170             clipperData->clipMaskImage.reset();
    171     }
    172 
    173     if (!clipperData->clipMaskImage)
     169            clipperMaskImage.reset();
     170    }
     171
     172    if (!clipperMaskImage)
    174173        return false;
    175174
    176     SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, shouldCreateClipData);
     175    SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperMaskImage, shouldCreateClipperMaskImage);
    177176    return true;
    178177}
    179178
    180 bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData, const FloatRect& objectBoundingBox)
    181 {
    182     ASSERT(clipperData);
    183     ASSERT(clipperData->clipMaskImage);
    184 
    185     GraphicsContext* maskContext = clipperData->clipMaskImage->context();
     179bool RenderSVGResourceClipper::drawContentIntoMaskImage(const ClipperMaskImage& clipperMaskImage, const FloatRect& objectBoundingBox)
     180{
     181    ASSERT(clipperMaskImage);
     182
     183    GraphicsContext* maskContext = clipperMaskImage->context();
    186184    ASSERT(maskContext);
    187185
    … …  
    234232        // We have to pass the <use> renderer itself to renderSubtreeToImageBuffer() to apply it's x/y/transform/etc. values when rendering.
    235233        // So if isUseElement is true, refetch the childNode->renderer(), as renderer got overriden above.
    236         SVGRenderingContext::renderSubtreeToImageBuffer(clipperData->clipMaskImage.get(), isUseElement ? *child.renderer() : *renderer, maskContentTransformation);
     234        SVGRenderingContext::renderSubtreeToImageBuffer(clipperMaskImage.get(), isUseElement ? *child.renderer() : *renderer, maskContentTransformation);
    237235    }
    238236
    … …  
    258256}
    259257
    260 ClipperData* RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object)
    261 {
    262     if (!m_clipper.contains(&object))
    263         m_clipper.set(&object, std::make_unique<ClipperData>());
    264     return m_clipper.get(&object);
     258ClipperMaskImage& RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object)
     259{
     260    return m_clipper.add(&object, ClipperMaskImage()).iterator->value;
    265261}
    266262

  • trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h

    r180643 r180685  
    3232namespace WebCore {
    3333
    34 struct ClipperData {
    35     WTF_MAKE_FAST_ALLOCATED;
    36 public:
    37     std::unique_ptr<ImageBuffer> clipMaskImage;
    38 };
     34typedef std::unique_ptr<ImageBuffer> ClipperMaskImage;
    3935
    4036class RenderSVGResourceClipper final : public RenderSVGResourceContainer {
    … …  
    7066
    7167    bool pathOnlyClipping(GraphicsContext*, const AffineTransform&, const FloatRect&);
    72     bool drawContentIntoMaskImage(ClipperData*, const FloatRect& objectBoundingBox);
     68    bool drawContentIntoMaskImage(const ClipperMaskImage&, const FloatRect& objectBoundingBox);
    7369    void calculateClipContentRepaintRect();
    74     ClipperData* addRendererToClipper(const RenderObject&);
     70    ClipperMaskImage& addRendererToClipper(const RenderObject&);
    7571
    7672    FloatRect m_clipBoundaries;
    77     HashMap<const RenderObject*, std::unique_ptr<ClipperData>> m_clipper;
     73    HashMap<const RenderObject*, ClipperMaskImage> m_clipper;
    7874};
    7975
Note: See TracChangeset for help on using the changeset viewer.