Bug 100413 – [cg] RenderBlock::selectionGaps() is extremely slow when there are many floats

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

<rdar://problem/12544626>

To reproduce: navigate to the URL and select one of the category names in red.

Created attachment 170730 [details]
Add GraphicsContext::clipOut(Vector<IntRect>&) and deploy it in RenderBlock::selectionGaps()

[Darin Adler]

Comment on attachment 170730 [details]
Add GraphicsContext::clipOut(Vector<IntRect>&) and deploy it in RenderBlock::selectionGaps()

View in context: https://bugs.webkit.org/attachment.cgi?id=170730&action=review

I’m OK with this, but I suspect there may be two other variations that work equally well or perhaps even better.

> Source/WebCore/ChangeLog:10
> +        RenderBlock::selectionGaps() calls GraphicsContext::clipOut(const IntRect&) for each float.
> +        With Core Graphics, A function that takes a vector of rectangles and clips them all out at
> +        once is faster than multiple calls to the function that clips out a single rectangle.

Is the issue here the performance of repeatedly calling the CGContextGetClipBoundingBox function?

If so, could we fix this performance issue another way by simply using a wide open rectangle instead of calling CGContextGetClipBoundingBox?

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1080
> +        CGContextBeginPath(context);
> +        CGContextAddRects(context, cgRects, 2);
> +        CGContextEOClip(context);

Can the calls to CGContextBeginPath and CGContextEOClip be moved outside the loop? I don’t entirely understand the semantics here, but if all the rectangles are exclusive or'ed with each other, it seems that it could all be done with a single path.

(In reply to comment #2)
> (From update of attachment 170730 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170730&action=review
> 
> I’m OK with this, but I suspect there may be two other variations that work equally well or perhaps even better.
> 
> > Source/WebCore/ChangeLog:10
> > +        RenderBlock::selectionGaps() calls GraphicsContext::clipOut(const IntRect&) for each float.
> > +        With Core Graphics, A function that takes a vector of rectangles and clips them all out at
> > +        once is faster than multiple calls to the function that clips out a single rectangle.
> 
> Is the issue here the performance of repeatedly calling the CGContextGetClipBoundingBox function?

That’s the most expensive part. There’s also some function call overhead, but I didn’t profile this so I don’t know if it’s significant.

> 
> If so, could we fix this performance issue another way by simply using a wide open rectangle instead of calling CGContextGetClipBoundingBox?

I am going to try that.

> > Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1080
> > +        CGContextBeginPath(context);
> > +        CGContextAddRects(context, cgRects, 2);
> > +        CGContextEOClip(context);
> 
> Can the calls to CGContextBeginPath and CGContextEOClip be moved outside the loop? I don’t entirely understand the semantics here, but if all the rectangles are exclusive or'ed with each other, it seems that it could all be done with a single path.

We’d need to know that all rects in the input vector are pairwise disjoint.

Created attachment 170741 [details]
Remove an unnecessary call to CGContextGetClipBoundingBox from GraphicsContext::clipOut()

Fixed in <http://trac.webkit.org/r132545>.

[Nate Whetsell]

*** Bug 39380 has been marked as a duplicate of this bug. ***