Bug 54365 – GraphicsContext: Add clip(IntRect) overload for Qt

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

[Andreas Kling]

Qt has special code paths for clipping to integer rectangles.
Add a GraphicsContext::clip() overload to allow us to take advantage of the optimized code.

[Andreas Kling]

Created attachment 82278 [details]
Proposed patch

[Darin Adler]

Comment on attachment 82278 [details]
Proposed patch

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

> Source/WebCore/platform/graphics/GraphicsContext.h:326
> +#if PLATFORM(QT)
> +        void clip(const IntRect&);
> +#endif

I think we should add this cross-platform. For the other platforms we can the graphics context code convert the rectangle to floating point. You could put the version for all other platforms into GraphicsContextQt.cpp inside an #if !PLATFORM(QT) block.

We really don’t want to start putting PLATFORM(QT) if statements into call sites!

[Andreas Kling]

(In reply to comment #2)
> (From update of attachment 82278 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=82278&action=review
> 
> > Source/WebCore/platform/graphics/GraphicsContext.h:326
> > +#if PLATFORM(QT)
> > +        void clip(const IntRect&);
> > +#endif
> 
> I think we should add this cross-platform. For the other platforms we can the graphics context code convert the rectangle to floating point. You could put the version for all other platforms into GraphicsContextQt.cpp inside an #if !PLATFORM(QT) block.

Fair enough. The reason I didn't do it was because most (all?) call sites automatically started using the new overload since they were using the implicit FloatRect(IntRect) constructor before.

[Andreas Kling]

Created attachment 82354 [details]
Proposed patch

Updated patch making GraphicsContext::clip(IntRect) cross-port per Darin's suggestion.

[Darin Adler]

Comment on attachment 82354 [details]
Proposed patch

Seems OK. I’d like to see the patch that makes use of this ASAP, though, because until then this is dead code.

[Andreas Kling]

(In reply to comment #5)
> (From update of attachment 82354 [details])
> Seems OK. I’d like to see the patch that makes use of this ASAP, though, because until then this is dead code.

It's already in place, some example users:

RenderBlock::paintColumnContents()
RenderBox::pushContentsClip()
RenderBoxModelObject::paintFillLayerExtended()
RenderLayer::beginTransparencyLayers()
ScrollView::paint()

They were previously calling clip() with implicitly constructed FloatRects.

[Andreas Kling]

Comment on attachment 82354 [details]
Proposed patch

Clearing flags on attachment: 82354

Committed r78503: <http://trac.webkit.org/changeset/78503>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.