Changing shape-margin dynamically should cause relayout. Shape-margin should also be animatable via CSS.
Created attachment 213720 [details] Initial patch
Comment on attachment 213720 [details] Initial patch View in context: https://bugs.webkit.org/attachment.cgi?id=213720&action=review > Source/WebCore/ChangeLog:3155 > + [CSS Shapes] Basic shapes should be animatable for shape-outside > + https://bugs.webkit.org/show_bug.cgi?id=122343 > + > + Reviewed by NOBODY (OOPS!). > + > + Test: fast/shapes/shape-outside-floats/shape-outside-animation.html > + > + Add shape outside to the list of animatable properties. The infrastructure > + is already in place for animating basic shapes on shape-inside and clipping > + paths. See https://bugs.webkit.org/show_bug.cgi?id=101854. > + > + * page/animation/CSSPropertyAnimation.cpp: > + (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap): Extra change log here. Oops! > Source/WebCore/rendering/RenderBox.cpp:391 > // FIXME: A future optimization would do a deep comparison for equality. (bug 100811) This comment makes more sense just before the "==". It’s a little strange to have it paragraphed with code that just sets up shapeOutside locals. > Source/WebCore/rendering/RenderBox.cpp:393 > + const ShapeValue* oldShapeOutside = oldStyle ? oldStyle->shapeOutside() : 0; Please use nullptr instead of 0. > Source/WebCore/rendering/RenderBox.cpp:396 > + const Length shapeMargin = style->shapeMargin(); > + const Length oldShapeMargin = oldStyle ? oldStyle->shapeMargin() : RenderStyle::initialShapeMargin(); The const don’t add much value here. > Source/WebCore/rendering/RenderBox.cpp:405 > ShapeOutsideInfo* shapeOutsideInfo = ShapeOutsideInfo::ensureInfo(this); > shapeOutsideInfo->dirtyShapeSize(); This would read better without the local variable. Also, is it really necessary to create the info with an “ensure” function just to dirty the size? Seems that if there was no info there is nothing to dirty. > Source/WebCore/rendering/RenderBox.h:645 > - void updateShapeOutsideInfoAfterStyleChange(const ShapeValue* shapeOutside, const ShapeValue* oldShapeOutside); > + void updateShapeOutsideInfoAfterStyleChange(const RenderStyle*, const RenderStyle* oldStyle); First argument should be const RenderStyle& since it’s guaranteed to never be null. > LayoutTests/ChangeLog:733 > + [CSS Shapes] Basic shapes should be animatable for shape-outside > + https://bugs.webkit.org/show_bug.cgi?id=122343 > + > + Reviewed by NOBODY (OOPS!). > + > + Add tests checking that shape-outside basic shape values correctly tween > + between values. > + > + * animations/resources/animation-test-helpers.js: > + (getPropertyValue): Add shape-outside to list of properties that do not parse > + with the default behavior. > + (comparePropertyValue): Compare shape-outsides after parsing their shape notation. > + * fast/shapes/shape-outside-floats/shape-outside-animation-expected.txt: Added. > + * fast/shapes/shape-outside-floats/shape-outside-animation.html: Added. Extra change log here.
Created attachment 213819 [details] Incorporating feedback Making suggested changes. The ensureInfo is called because there may be a ShapeInfo we are going to reuse, in which case we need to dirty the size bit.
Created attachment 214026 [details] Updated patch Merging to master/trunk
Comment on attachment 214026 [details] Updated patch Clearing flags on attachment: 214026 Committed r157400: <http://trac.webkit.org/changeset/157400>
All reviewed patches have been landed. Closing bug.