[webkit-reviews] review granted: [Bug 92252] Grid Demo spends 1.5% of total time allocating Path objects in RenderBoxModelObject::paintBorderSides : [Attachment 155348] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Eric Seidel
<eric at webkit.org>'s request for review:
Bug 92252: Grid Demo spends 1.5% of total time allocating Path objects in
RenderBoxModelObject::paintBorderSides
https://bugs.webkit.org/show_bug.cgi?id=92252

Attachment 155348: Patch
https://bugs.webkit.org/attachment.cgi?id=155348&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=155348&action=review


> Source/WebCore/ChangeLog:31
> +	   I expect that platforms which do not run the pixel tests on
build.webkit.org may later discover pixel-only regressions
> +	   from this change, due to possible mistakes on my part while adding
null-path support.  I'm happy
> +	   to work with those platforms to correct any pixel changes they may
find.

I don't think you need to say this in the changelog.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1091
> +    // Why does clipping to an empty path do nothing?
> +    // Why is this different from GraphicsContext::clip(const Path&).
>      if (path.isEmpty())
>	   return;

Does svn history tell you anything?

> Source/WebCore/platform/graphics/cg/PathCG.cpp:91
> +    if (m_path)
> +	   CGPathRelease(m_path);
> +}
> +
> +PlatformPathPtr Path::ensurePlatformPath()
> +{
> +    if (!m_path)
> +	   m_path = CGPathCreateMutable();
> +    return m_path;

Shame we can't use a RetainPtr in the header.