Bug 122622 – [CSS Shapes] Shape-Image-Threshold should be animatable

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Bear Travis]

The shape-image-threshold property should respond to dynamic changes (add/modify/remove) and to CSS animations.

[Bear Travis]

Created attachment 213943 [details]
Initial Patch

[Darin Adler]

Comment on attachment 213943 [details]
Initial Patch

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

> Source/WebCore/rendering/RenderBox.cpp:399
> +        ShapeOutsideInfo::ensureInfo(this)->dirtyShapeSize();

I am surprised that we would ensureInfo just so we can call dirtyShapeSize. Is there really something to dirty if there is no info? Is that really how things work? Did I already ask this in another context?

[Bear Travis]

(In reply to comment #2)
> (From update of attachment 213943 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=213943&action=review
> 
> > Source/WebCore/rendering/RenderBox.cpp:399
> > +        ShapeOutsideInfo::ensureInfo(this)->dirtyShapeSize();
> 
> I am surprised that we would ensureInfo just so we can call dirtyShapeSize. Is there really something to dirty if there is no info? Is that really how things work? Did I already ask this in another context?

Yep. You saw this in Bug 122524. ShapeOutsideInfos are stored in a global map, and ShapeOutsideInfo::ensureInfo will make sure a ShapeOutsideInfo exists for the given key. But it may also return an existing ShapeOutsideInfo if one is already in the map. It's this case we cover with dirtyShapeSize(). We could optimize by checking in ShapeOutsideInfo::dirtyShapeSize whether or not the shape is already dirty, but it currently just clears a RefPtr, which does not seem too costly.

[WebKit Commit Bot]

Comment on attachment 213943 [details]
Initial Patch

Clearing flags on attachment: 213943

Committed r157309: <http://trac.webkit.org/changeset/157309>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.