|
|
Created:
3 years, 7 months ago by jfernandez Modified:
3 years, 7 months ago CC:
chromium-reviews, jfernandez, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, Manuel Rego, leviw+renderwatch, jchaffraix+rendering, blink-reviews, eae+blinkwatch Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[css-grid] Avoid the relayout forced on orthogonal grid items
Blink performs a pre-layout of any orthogonal box present in the layout
tree. This operation helps to figure out the orthogonal box's height
used during the container's intrinsic or preferred width computation.
However, we decided to remove grid items from the candidates to perform
such prelayout operation. Check out https://crrev.com/2698663003 for
more details, but basically we have done that because grid items use the
Grid Area abstraction as container, which is not defined at the time the
prelayout operation is performed.
In order to make the Grid's layout logic repeatable, we were clearing
the items' overrideSize so it doesn't interfere in the subsequent layout
executions (eg, scrolling forces an additional call to the layout
algorithm). This logic still applies.
However, due to the prelayout operation, we needed to force a layout of
the orthogonal item after clearing the override sizes. Since we don't
execute the prelayout on orthogonal grid items anymore, we don't need to
force a new layout.
Review-Url: https://codereview.chromium.org/2856163002
Cr-Commit-Position: refs/heads/master@{#469964}
Committed: https://chromium.googlesource.com/chromium/src/+/b55d5e08cb57256f0c80eca93c5beaf9d4c821aa
Patch Set 1 #Patch Set 2 : Do no clear overrideContainingBlock #Messages
Total messages: 20 (12 generated)
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out the change made in r460774 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clering the overrideSize and containerOverrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clering the overrideSize and containerOverrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ==========
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clering the overrideSize and containerOverrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clering the overrideSize and containerOverrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ==========
jfernandez@igalia.com changed reviewers: + cbiesinger@chromium.org, rego@igalia.com, svillar@igalia.com
PTAL
This patch unconditionally clears the containing block override sizes forcing all the items to be laid out even if nothing has really changed.
On 2017/05/03 17:16:14, svillar wrote: > This patch unconditionally clears the containing block override sizes forcing > all the items to be laid out even if nothing has really changed. Does it? Clearing an override size does not itself mark it as needing layout.
On 2017/05/04 20:48:51, cbiesinger wrote: > On 2017/05/03 17:16:14, svillar wrote: > > This patch unconditionally clears the containing block override sizes forcing > > all the items to be laid out even if nothing has really changed. > > Does it? Clearing an override size does not itself mark it as needing layout. Yes because we have this code in our LayoutGridItems() method: if (old_override_containing_block_content_logical_width != override_containing_block_content_logical_width || (old_override_containing_block_content_logical_height != override_containing_block_content_logical_height && child->HasRelativeLogicalHeight())) child->SetNeedsLayout(LayoutInvalidationReason::kGridChanged); The old_override would be initialized to LayoutUnit() and the new one would be always different (unless it's zero). In practice we'll always call SetNeedsLayout for every single grid item.
PTAL
lgtm. Please fix the bug description. The 3rd paragraph is not accurate any more.
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clering the overrideSize and containerOverrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ==========
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ==========
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ==========
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). Due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout; hence, this patch removes such unneeded logic. ==========
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout; hence, this patch removes such unneeded logic. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout; hence, this patch removes such unneeded logic. ==========
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout; hence, this patch removes such unneeded logic. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ==========
The CQ bit was checked by jfernandez@igalia.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494234655894670, "parent_rev": "0cd83681ad239907cf6fc7c07cfcca1262474529", "commit_rev": "b55d5e08cb57256f0c80eca93c5beaf9d4c821aa"}
Message was sent while issue was closed.
Description was changed from ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. ========== to ========== [css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. Review-Url: https://codereview.chromium.org/2856163002 Cr-Commit-Position: refs/heads/master@{#469964} Committed: https://chromium.googlesource.com/chromium/src/+/b55d5e08cb57256f0c80eca93c5b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/b55d5e08cb57256f0c80eca93c5b... |