Changeset 223955 in webkit


Ignore:
Timestamp:
Oct 25, 2017 9:38:14 AM (6 years ago)
Author:
jfernandez@igalia.com
Message:

[css-grid] Avoid clearing the overrideContainingBlockWidth if possible
​https://bugs.webkit.org/show_bug.cgi?id=178260

Reviewed by Sergio Villar Senin.

Since the intrinsic width computation uses the same logic than the
track sizing algorithm we are clearing the overrideContainingBlockWidth
of some grid items that are required to laid out them properly.

It's very uncommon that any intrinsic size computation isn't performed
as part of a layout process. However, if it happens, once cleared the
overrideContainingBlockWidth it may lead to an incorrect layout of the
affected grid items.

This change is a defensive approach to avoid the issues caused by
such off-layout preferred size requests, which may imply recomputing
the grid container intrinsic size.

No new tests, because we are only removing some redundant logic.

  • rendering/GridTrackSizingAlgorithm.cpp:

(WebCore::GridTrackSizingAlgorithmStrategy::minContentForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::maxContentForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild const):
(WebCore::IndefiniteSizeStrategy::minLogicalWidthForChild const):
(WebCore::DefiniteSizeStrategy::minLogicalWidthForChild const):

  • rendering/GridTrackSizingAlgorithm.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r223954 r223955  
     12017-10-25  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-grid] Avoid clearing the overrideContainingBlockWidth if possible
     4        https://bugs.webkit.org/show_bug.cgi?id=178260
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        Since the intrinsic width computation uses the same logic than the
     9        track sizing algorithm we are clearing the overrideContainingBlockWidth
     10        of some grid items that are required to laid out them properly.
     11
     12        It's very uncommon that any intrinsic size computation isn't performed
     13        as part of a layout process. However, if it happens, once cleared the
     14        overrideContainingBlockWidth it may lead to an incorrect layout of the
     15        affected grid items.
     16
     17        This change is a defensive approach to avoid the issues caused by
     18        such off-layout preferred size requests, which may imply recomputing
     19        the grid container intrinsic size.
     20
     21        No new tests, because we are only removing some redundant logic.
     22
     23        * rendering/GridTrackSizingAlgorithm.cpp:
     24        (WebCore::GridTrackSizingAlgorithmStrategy::minContentForChild const):
     25        (WebCore::GridTrackSizingAlgorithmStrategy::maxContentForChild const):
     26        (WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
     27        (WebCore::GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild const):
     28        (WebCore::IndefiniteSizeStrategy::minLogicalWidthForChild const):
     29        (WebCore::DefiniteSizeStrategy::minLogicalWidthForChild const):
     30        * rendering/GridTrackSizingAlgorithm.h:
     31
    1322017-10-25  Gustavo Noronha Silva  <gustavo.noronha@collabora.co.uk>
    233

  • trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp

    r222556 r223955  
    743743    GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(renderGrid(), child, ForColumns);
    744744    if (direction() == childInlineDirection) {
    745         // If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is
    746         // what we are interested in here. Thus we need to set the override logical width to std::nullopt (no possible resolution).
    747         if (shouldClearOverrideContainingBlockContentSizeForChild(child, ForColumns))
    748             setOverrideContainingBlockContentSizeForChild(child, childInlineDirection, std::nullopt);
    749 
    750745        // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
    751746        // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
    … …  
    763758    GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(renderGrid(), child, ForColumns);
    764759    if (direction() == childInlineDirection) {
    765         // If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is
    766         // what we are interested in here. Thus we need to set the inline-axis override size to -1 (no possible resolution).
    767         if (shouldClearOverrideContainingBlockContentSizeForChild(child, ForColumns))
    768             setOverrideContainingBlockContentSizeForChild(child, childInlineDirection, std::nullopt);
    769 
    770760        // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
    771761        // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
    … …  
    790780        return minContentForChild(child);
    791781
    792     bool overrideSizeHasChanged = updateOverrideContainingBlockContentSizeForChild(child, childInlineDirection);
    793 
     782    LayoutUnit gridAreaSize = m_algorithm.gridAreaBreadthForChild(child, childInlineDirection);
    794783    if (isRowAxis)
    795         return minLogicalWidthForChild(child, childMinSize, childInlineDirection);
    796 
     784        return minLogicalWidthForChild(child, childMinSize, gridAreaSize);
     785
     786    bool overrideSizeHasChanged = updateOverrideContainingBlockContentSizeForChild(child, childInlineDirection, gridAreaSize);
    797787    layoutGridItemForMinSizeComputation(child, overrideSizeHasChanged);
    798788
    … …  
    800790}
    801791
    802 bool GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild(RenderBox& child, GridTrackSizingDirection direction) const
    803 {
    804     LayoutUnit overrideSize = m_algorithm.gridAreaBreadthForChild(child, direction);
     792bool GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild(RenderBox& child, GridTrackSizingDirection direction, std::optional<LayoutUnit> overrideSize) const
     793{
     794    if (!overrideSize)
     795        overrideSize = m_algorithm.gridAreaBreadthForChild(child, direction);
    805796    if (hasOverrideContainingBlockContentSizeForChild(child, direction) && overrideContainingBlockContentSizeForChild(child, direction) == overrideSize)
    806797        return false;
    … …  
    816807
    817808private:
    818     LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, GridTrackSizingDirection) const override;
     809    LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, LayoutUnit availableSize) const override;
    819810    void layoutGridItemForMinSizeComputation(RenderBox&, bool overrideSizeHasChanged) const override;
    820811    void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) override;
    … …  
    823814};
    824815
    825 LayoutUnit IndefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, GridTrackSizingDirection childInlineDirection) const
    826 {
    827     return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, overrideContainingBlockContentSizeForChild(child, childInlineDirection).value_or(0), *renderGrid(), nullptr) + marginIntrinsicLogicalWidthForChild(renderGrid(), child);
     816LayoutUnit IndefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
     817{
     818    return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, availableSize, *renderGrid(), nullptr) + marginIntrinsicLogicalWidthForChild(renderGrid(), child);
    828819}
    829820
    … …  
    913904
    914905private:
    915     LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, GridTrackSizingDirection) const override;
     906    LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, LayoutUnit availableSize) const override;
    916907    void layoutGridItemForMinSizeComputation(RenderBox&, bool overrideSizeHasChanged) const override;
    917908    void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) override;
    … …  
    920911};
    921912
    922 LayoutUnit DefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, GridTrackSizingDirection childInlineDirection) const
     913LayoutUnit DefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
    923914{
    924915    LayoutUnit marginLogicalWidth =
    925         computeMarginLogicalSizeForChild(childInlineDirection, *renderGrid(), child);
    926     return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, overrideContainingBlockContentSizeForChild(child, childInlineDirection).value_or(0), *renderGrid(), nullptr) + marginLogicalWidth;
     916        computeMarginLogicalSizeForChild(flowAwareDirectionForChild(renderGrid(), child, ForColumns), *renderGrid(), child);
     917    return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, availableSize, *renderGrid(), nullptr) + marginLogicalWidth;
    927918}
    928919

  • trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h

    r223728 r223955  
    229229        : m_algorithm(algorithm) { }
    230230
    231     virtual LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, GridTrackSizingDirection) const = 0;
     231    virtual LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, LayoutUnit availableSize) const = 0;
    232232    virtual void layoutGridItemForMinSizeComputation(RenderBox&, bool overrideSizeHasChanged) const = 0;
    233233
    234234    LayoutUnit logicalHeightForChild(RenderBox&) const;
    235     bool updateOverrideContainingBlockContentSizeForChild(RenderBox&, GridTrackSizingDirection) const;
     235    bool updateOverrideContainingBlockContentSizeForChild(RenderBox&, GridTrackSizingDirection, std::optional<LayoutUnit> = std::nullopt) const;
    236236
    237237    // GridTrackSizingAlgorithm accessors for subclasses.
Note: See TracChangeset for help on using the changeset viewer.