We are progressively adding support on grid for orthogonal flows, however, stretching is not possible yet in these cases.
Created attachment 284691 [details] Patch
Comment on attachment 284691 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284691&action=review > Source/WebCore/rendering/RenderGrid.cpp:2190 > + bool childHasAutoSizeInColumnAxis = isHorizontalMode ? childStyle.height().isAuto() : childStyle.width().isAuto(); > + bool childHasAutoSizeInRowAxis = isHorizontalMode ? childStyle.width().isAuto() : childStyle.height().isAuto(); > + bool allowedToStretchChildAlongColumnAxis = childHasAutoSizeInColumnAxis && !hasAutoMarginsInColumnAxis(child); > + bool allowedToStretchChildAlongRowAxis = childHasAutoSizeInRowAxis && !hasAutoMarginsInRowAxis(child); > + bool stretchingAlongRowAxis = childStyle.resolvedJustifySelf(gridStyle, selfAlignmentNormalBehavior).position() == ItemPositionStretch; > + bool stretchingAlongColumnAxis = childStyle.resolvedAlignSelf(gridStyle, selfAlignmentNormalBehavior).position() == ItemPositionStretch; Inefficient to compute both axes and then only use the one that is needed below. Maybe these can be functions instead of booleans so we can call only the one that is needed. Also inefficient to compute stretchingAlongAxis when allowedToStretchChildAlongAxis is already false. Early return style would work for that. > Source/WebCore/rendering/RenderGrid.cpp:2203 > + if (allowedToStretchChildBlockSize) { > + LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(overrideContainingBlockContentSizeForChild(child, childBlockDirection).value(), child); > + LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight, LayoutUnit(-1)); > + child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight()); > + if (desiredLogicalHeight != child.logicalHeight()) { > + // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905. > + child.setLogicalHeight(LayoutUnit()); > + child.setNeedsLayout(); > } > } Could be redone to use early exit instead of nesting the having he main function flow go into two nested if statements.
Comment on attachment 284691 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284691&action=review >> Source/WebCore/rendering/RenderGrid.cpp:2190 >> + bool stretchingAlongColumnAxis = childStyle.resolvedAlignSelf(gridStyle, selfAlignmentNormalBehavior).position() == ItemPositionStretch; > > Inefficient to compute both axes and then only use the one that is needed below. Maybe these can be functions instead of booleans so we can call only the one that is needed. > > Also inefficient to compute stretchingAlongAxis when allowedToStretchChildAlongAxis is already false. Early return style would work for that. I understand, I was aiming for a clearer code but it's true we can improve in the points you made. Thanks for the review, I'll apply the suggested chances in the patch for landing. >> Source/WebCore/rendering/RenderGrid.cpp:2203 >> } > > Could be redone to use early exit instead of nesting the having he main function flow go into two nested if statements. Yeah, I'll do that.
Created attachment 286611 [details] Patch
Comment on attachment 286611 [details] Patch Clearing flags on attachment: 286611 Committed r204734: <http://trac.webkit.org/changeset/204734>
All reviewed patches have been landed. Closing bug.