Bug 92164

Summary: [CSS Shapes] Shapes should not cause relayout when they are equivalent
Product: WebKit Reporter: Bear Travis <betravis>
Component: CSSAssignee: Bear Travis <betravis>
Status: RESOLVED LATER    
Severity: Normal CC: ahmad.saleem792, bjonesbe, commit-queue, donggwan.kim, esprehn+autocc, glenn, koivisto, zalan, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89256    
Attachments:
Description Flags
Initial patch none

Bear Travis
Reported 2012-07-24 14:36:17 PDT
RenderStyle::diff should not cause relayout and RenderBlock::styleDidChange should not generate a new WrapShapeInfo for two different but equivalent WrapShapes Currently, just pointers are used for comparison, but eventually they should use a deep compare
Attachments
Initial patch (6.08 KB, patch)
2013-06-14 16:10 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Bem Jones-Bey
Comment 1 2013-02-12 14:58:08 PST
This also affects shape-outside, not just shape-inside.
Nico Weber
Comment 2 2013-02-12 15:07:30 PST
*** Bug 100811 has been marked as a duplicate of this bug. ***
Bear Travis
Comment 3 2013-06-14 16:10:43 PDT
Created attachment 204744 [details] Initial patch
Sam Weinig
Comment 4 2013-06-15 20:23:37 PDT
Comment on attachment 204744 [details] Initial patch View in context: https://bugs.webkit.org/attachment.cgi?id=204744&action=review > Source/WebCore/ChangeLog:11 > + The two layouts are equivalent, so there are no new tests. If this is being done for performance, do you have any perf results that this made better? Is there a benchmark you are using or a test case you are investigating? > Source/WebCore/rendering/style/BasicShapes.h:98 > + virtual bool operator==(const BasicShape& other) You should add OVERRIDE here (and on all the others below). Given that this is a virtual function, does this really need to be inline? > Source/WebCore/rendering/style/BasicShapes.h:211 > + for (size_t i = 0; i < values().size(); i++) > + if (getXAt(i) != otherPolygon->getXAt(i) || getYAt(i) != otherPolygon->getYAt(i)) > + return false; The for-loop needs braces.
Zoltan Horvath
Comment 5 2014-02-07 10:15:00 PST
Comment on attachment 204744 [details] Initial patch Pulling this out from the review queue, since it's need to be updated. (Changelog: [CSS Exclusions])
Zoltan Horvath
Comment 6 2014-02-07 10:16:04 PST
Closing bug until we have a perf test to verify the improvement.
Ahmad Saleem
Comment 7 2023-08-01 11:38:09 PDT
Just to add we do have FIXME still in our code referring to bug, which is marked duplicate of this: https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderBox.cpp#447 // FIXME: A future optimization would do a deep comparison for equality. (bug 100811) if (shapeOutside == oldShapeOutside && shapeMargin == oldShapeMargin && shapeImageThreshold == oldShapeImageThreshold) return;
Note You need to log in before you can comment on or make changes to this bug.