RESOLVED FIXED 70557
avoid unnecessary layouts of flex items during the flex pass 
https://bugs.webkit.org/show_bug.cgi?id=70557
Summary avoid unnecessary layouts of flex items during the flex pass
Tony Chang
Reported 2011-10-20 15:44:01 PDT
avoid unnecessary layouts of flex items during the flex pass
Attachments
Patch (6.60 KB, patch)
2011-10-20 15:49 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (6.61 KB, patch)
2011-10-25 11:45 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch for landing (6.57 KB, patch)
2011-10-25 12:14 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Tony Chang
Comment 1 2011-10-20 15:49:14 PDT
Created attachment 111862 [details] Patch
Ojan Vafai
Comment 2 2011-10-20 18:12:08 PDT
Comment on attachment 111862 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111862&action=review Patch looks good to me. > Source/WebCore/rendering/RenderFlexibleBox.cpp:496 > + Length fixedWidth = isHorizontalFlow() ? child->style()->width() : child->style()->height(); Nit: I think the code would be simpler if you added flowAwareWidthStyleForChild instead of isFlowAwareLogicalWidthAutoForChild.
Dave Hyatt
Comment 3 2011-10-25 11:17:09 PDT
Comment on attachment 111862 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111862&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:224 > + Length width = isHorizontalFlow() ? child->style()->width() : child->style()->height(); > + return width.isAuto(); Please name the local variable logicalWidth and not just width. > Source/WebCore/rendering/RenderFlexibleBox.cpp:497 > + return fixedWidth.calcMinValue(flowAwareContentLogicalWidth()); Adding flowAwareLogicalWidthLengthForChild as Ojan suggests seems cleaner.
Tony Chang
Comment 4 2011-10-25 11:45:00 PDT
Created attachment 112368 [details] Patch
Ojan Vafai
Comment 5 2011-10-25 12:11:03 PDT
Comment on attachment 112368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112368&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:506 > + if (flowAwareLogicalWidthLengthForChild(child).isAuto()) { > LayoutUnit logicalWidth = hasOrthogonalFlow(child) ? child->logicalHeight() : child->maxPreferredLogicalWidth(); > return logicalWidth - logicalBorderAndPaddingWidthForChild(child) - logicalScrollbarHeightForChild(child); > } > - return isHorizontalFlow() ? child->contentWidth() : child->contentHeight(); > + Length fixedWidth = isHorizontalFlow() ? child->style()->width() : child->style()->height(); > + return fixedWidth.calcMinValue(flowAwareContentLogicalWidth()); nit: i would do this like so: Length logicalWidthLength = flowAwareLogicalWidthLengthForChild(child); if (logicalWidthLength.isAuto()) { ... } return logicalWidthLength.calcMinValue(...);
Tony Chang
Comment 6 2011-10-25 12:14:33 PDT
Created attachment 112372 [details] Patch for landing
Tony Chang
Comment 7 2011-10-25 12:25:04 PDT
Committed r98373: <http://trac.webkit.org/changeset/98373>
Note You need to log in before you can comment on or make changes to this bug.