Changeset 182628 in webkit


Ignore:
Timestamp:
Apr 10, 2015 3:51:01 AM (9 years ago)
Author:
svillar@igalia.com
Message:

[CSS Grid Layout] Fix raw function pointer usages
​https://bugs.webkit.org/show_bug.cgi?id=143481

Reviewed by Antti Koivisto.

The argument list of
resolveContentBasedTrackSizingFunctionsForItems() became too long
and rather incomprehensible as it included up to 6 function
pointers. This replaces all of them by an enum which describes the
phase of the algorithm that is currently running. With that phase
we have enough information to select the right function to call.

In order not to tangle up too much the method, the new explicit
switch statements where moved to static helper functions.

No new tests as this is just a code refactoring.

  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::computeUsedBreadthOfGridTracks):
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
(WebCore::RenderGrid::trackSizeForTrackSizeComputationPhase): New helper function.
(WebCore::RenderGrid::shouldProcessTrackForTrackSizeComputationPhase): Ditto.
(WebCore::RenderGrid::trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase): Ditto.
(WebCore::RenderGrid::updateTrackSizeForTrackSizeComputationPhase): Ditto.
(WebCore::RenderGrid::currentItemSizeForTrackSizeComputationPhase): Ditto.
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctionsForItems):
(WebCore::RenderGrid::distributeSpaceToTracks):

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

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r182627 r182628  
     12015-04-09  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Fix raw function pointer usages
     4        https://bugs.webkit.org/show_bug.cgi?id=143481
     5
     6        Reviewed by Antti Koivisto.
     7
     8        The argument list of
     9        resolveContentBasedTrackSizingFunctionsForItems() became too long
     10        and rather incomprehensible as it included up to 6 function
     11        pointers. This replaces all of them by an enum which describes the
     12        phase of the algorithm that is currently running. With that phase
     13        we have enough information to select the right function to call.
     14
     15        In order not to tangle up too much the method, the new explicit
     16        switch statements where moved to static helper functions.
     17
     18        No new tests as this is just a code refactoring.
     19
     20        * rendering/RenderGrid.cpp:
     21        (WebCore::RenderGrid::computeUsedBreadthOfGridTracks):
     22        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
     23        (WebCore::RenderGrid::trackSizeForTrackSizeComputationPhase): New helper function.
     24        (WebCore::RenderGrid::shouldProcessTrackForTrackSizeComputationPhase): Ditto.
     25        (WebCore::RenderGrid::trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase): Ditto.
     26        (WebCore::RenderGrid::updateTrackSizeForTrackSizeComputationPhase): Ditto.
     27        (WebCore::RenderGrid::currentItemSizeForTrackSizeComputationPhase): Ditto.
     28        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctionsForItems):
     29        (WebCore::RenderGrid::distributeSpaceToTracks):
     30        * rendering/RenderGrid.h:
     31
    1322015-04-10  Adam Bergkvist  <adam.bergkvist@ericsson.com>
    233

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

    r182472 r182628  
    373373        }
    374374
    375         distributeSpaceToTracks(tracksForDistribution, nullptr, &GridTrack::baseSize, availableLogicalSpace);
     375        distributeSpaceToTracks<DistributeSpaceToBaseSizes>(tracksForDistribution, nullptr, availableLogicalSpace);
    376376
    377377        for (auto* track : tracksForDistribution)
    … …  
    643643    while (it != end) {
    644644        GridItemsSpanGroupRange spanGroupRange = { it, std::upper_bound(it, end, *it) };
    645         resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, spanGroupRange, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::baseSize, &GridTrack::baseSize, &GridTrack::setBaseSize, &GridTrackSize::hasMinContentMinTrackBreadthAndMinOrMaxContentMaxTrackBreadth);
    646         resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, spanGroupRange, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::baseSize, &GridTrack::baseSize, &GridTrack::setBaseSize, &GridTrackSize::hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth);
    647         resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, spanGroupRange, &GridTrackSize::hasMinOrMaxContentMaxTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::growthLimit, &GridTrack::growthLimitIfNotInfinite, &GridTrack::setGrowthLimit);
    648         resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, spanGroupRange, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::growthLimit, &GridTrack::growthLimitIfNotInfinite, &GridTrack::setGrowthLimit);
     645        resolveContentBasedTrackSizingFunctionsForItems<ResolveIntrinsicMinimums>(direction, sizingData, spanGroupRange);
     646        resolveContentBasedTrackSizingFunctionsForItems<ResolveMaxContentMinimums>(direction, sizingData, spanGroupRange);
     647        resolveContentBasedTrackSizingFunctionsForItems<ResolveIntrinsicMaximums>(direction, sizingData, spanGroupRange);
     648        resolveContentBasedTrackSizingFunctionsForItems<ResolveMaxContentMaximums>(direction, sizingData, spanGroupRange);
    649649        it = spanGroupRange.rangeEnd;
    650650    }
    … …  
    673673}
    674674
    675 void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, const GridItemsSpanGroupRange& gridItemsWithSpan, FilterFunction filterFunction, SizingFunction sizingFunction, AccumulatorGetter trackSize, AccumulatorGetter correctedTrackSize, AccumulatorGrowFunction trackGrowthFunction, FilterFunction growAboveMaxBreadthFilterFunction)
     675const LayoutUnit& RenderGrid::trackSizeForTrackSizeComputationPhase(TrackSizeComputationPhase phase, GridTrack& track, TrackSizeRestriction restriction)
     676{
     677    switch (phase) {
     678    case ResolveIntrinsicMinimums:
     679    case ResolveMaxContentMinimums:
     680        return track.baseSize();
     681        break;
     682    case ResolveIntrinsicMaximums:
     683    case ResolveMaxContentMaximums:
     684        return restriction == AllowInfinity ? track.growthLimit() : track.growthLimitIfNotInfinite();
     685        break;
     686    }
     687
     688    ASSERT_NOT_REACHED();
     689    return track.baseSize();
     690}
     691
     692bool RenderGrid::shouldProcessTrackForTrackSizeComputationPhase(TrackSizeComputationPhase phase, const GridTrackSize& trackSize)
     693{
     694    switch (phase) {
     695    case ResolveIntrinsicMinimums:
     696        return trackSize.hasMinOrMaxContentMinTrackBreadth();
     697    case ResolveMaxContentMinimums:
     698        return trackSize.hasMaxContentMinTrackBreadth();
     699    case ResolveIntrinsicMaximums:
     700        return trackSize.hasMinOrMaxContentMaxTrackBreadth();
     701    case ResolveMaxContentMaximums:
     702        return trackSize.hasMaxContentMaxTrackBreadth();
     703    }
     704
     705    ASSERT_NOT_REACHED();
     706    return false;
     707}
     708
     709bool RenderGrid::trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase(TrackSizeComputationPhase phase, const GridTrackSize& trackSize)
     710{
     711    switch (phase) {
     712    case ResolveIntrinsicMinimums:
     713        return trackSize.hasMinContentMinTrackBreadthAndMinOrMaxContentMaxTrackBreadth();
     714    case ResolveMaxContentMinimums:
     715        return trackSize.hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth();
     716    case ResolveIntrinsicMaximums:
     717    case ResolveMaxContentMaximums:
     718        return true;
     719    }
     720
     721    ASSERT_NOT_REACHED();
     722    return false;
     723}
     724
     725void RenderGrid::updateTrackSizeForTrackSizeComputationPhase(TrackSizeComputationPhase phase, GridTrack& track)
     726{
     727    switch (phase) {
     728    case ResolveIntrinsicMinimums:
     729    case ResolveMaxContentMinimums:
     730        track.setBaseSize(track.plannedSize());
     731        return;
     732    case ResolveIntrinsicMaximums:
     733    case ResolveMaxContentMaximums:
     734        track.setGrowthLimit(track.plannedSize());
     735        return;
     736    }
     737
     738    ASSERT_NOT_REACHED();
     739}
     740
     741LayoutUnit RenderGrid::currentItemSizeForTrackSizeComputationPhase(TrackSizeComputationPhase phase, RenderBox& gridItem, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
     742{
     743    switch (phase) {
     744    case ResolveIntrinsicMinimums:
     745    case ResolveIntrinsicMaximums:
     746        return minContentForChild(gridItem, direction, columnTracks);
     747    case ResolveMaxContentMinimums:
     748    case ResolveMaxContentMaximums:
     749        return maxContentForChild(gridItem, direction, columnTracks);
     750    }
     751
     752    ASSERT_NOT_REACHED();
     753    return 0;
     754}
     755
     756template <RenderGrid::TrackSizeComputationPhase phase>
     757void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, const GridItemsSpanGroupRange& gridItemsWithSpan)
    676758{
    677759    Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
    678760    for (const auto& trackIndex : sizingData.contentSizedTracksIndex) {
    679761        GridTrack& track = tracks[trackIndex];
    680         track.setPlannedSize((track.*trackSize)());
     762        track.setPlannedSize(trackSizeForTrackSizeComputationPhase(phase, track, AllowInfinity));
    681763    }
    682764
    … …  
    693775            const GridTrackSize& trackSize = gridTrackSize(direction, trackPosition.toInt());
    694776            GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackPosition.toInt()] : sizingData.rowTracks[trackPosition.toInt()];
    695             spanningTracksSize += (track.*correctedTrackSize)();
    696             if (!(trackSize.*filterFunction)())
     777            spanningTracksSize += trackSizeForTrackSizeComputationPhase(phase, track, ForbidInfinity);
     778            if (!shouldProcessTrackForTrackSizeComputationPhase(phase, trackSize))
    697779                continue;
    698780
    699781            sizingData.filteredTracks.append(&track);
    700782
    701             if (!growAboveMaxBreadthFilterFunction || (trackSize.*growAboveMaxBreadthFilterFunction)())
     783            if (trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase(phase, trackSize))
    702784                sizingData.growBeyondGrowthLimitsTracks.append(&track);
    703785        }
    … …  
    706788            continue;
    707789
     790        LayoutUnit extraSpace = currentItemSizeForTrackSizeComputationPhase(phase, gridItemWithSpan.gridItem(), direction, sizingData.columnTracks) - spanningTracksSize;
    708791        // Specs mandate to floor extraSpace to 0. Instead we directly avoid the function call in those cases as it will be
    709792        // a noop in terms of track sizing.
    710         LayoutUnit extraSpace = (this->*sizingFunction)(gridItemWithSpan.gridItem(), direction, sizingData.columnTracks) - spanningTracksSize;
    711793        if (extraSpace > 0) {
    712794            auto* tracksToGrowBeyondGrowthLimits = sizingData.growBeyondGrowthLimitsTracks.isEmpty() ? &sizingData.filteredTracks : &sizingData.growBeyondGrowthLimitsTracks;
    713             distributeSpaceToTracks(sizingData.filteredTracks, tracksToGrowBeyondGrowthLimits, correctedTrackSize, extraSpace);
     795            switch (phase) {
     796            case ResolveIntrinsicMinimums:
     797            case ResolveMaxContentMinimums:
     798                distributeSpaceToTracks<DistributeSpaceToBaseSizes>(sizingData.filteredTracks, tracksToGrowBeyondGrowthLimits, extraSpace);
     799                break;
     800            case ResolveIntrinsicMaximums:
     801            case ResolveMaxContentMaximums:
     802                distributeSpaceToTracks<DistributeSpaceToGrowthLimits>(sizingData.filteredTracks, tracksToGrowBeyondGrowthLimits, extraSpace);
     803                break;
     804            }
    714805        }
    715806    }
    … …  
    717808    for (const auto& trackIndex : sizingData.contentSizedTracksIndex) {
    718809        GridTrack& track = tracks[trackIndex];
    719         (track.*trackGrowthFunction)(track.plannedSize());
     810        updateTrackSizeForTrackSizeComputationPhase(phase, track);
    720811    }
    721812}
    … …  
    734825}
    735826
    736 void RenderGrid::distributeSpaceToTracks(Vector<GridTrack*>& tracks, const Vector<GridTrack*>* growBeyondGrowthLimitsTracks, AccumulatorGetter trackGetter, LayoutUnit& availableLogicalSpace)
     827template <RenderGrid::DistributeSpacePhase phase>
     828void RenderGrid::distributeSpaceToTracks(Vector<GridTrack*>& tracks, const Vector<GridTrack*>* growBeyondGrowthLimitsTracks, LayoutUnit& availableLogicalSpace)
    737829{
    738830    ASSERT(availableLogicalSpace > 0);
    … …  
    743835    for (unsigned i = 0; i < tracksSize; ++i) {
    744836        GridTrack& track = *tracks[i];
    745         const LayoutUnit& trackBreadth = (track.*trackGetter)();
     837        LayoutUnit trackBreadth;
     838        switch (phase) {
     839        case DistributeSpaceToBaseSizes:
     840            trackBreadth = track.baseSize();
     841            break;
     842        case DistributeSpaceToGrowthLimits:
     843            trackBreadth = track.growthLimitIfNotInfinite();
     844            break;
     845        }
    746846        bool infiniteGrowthPotential = track.growthLimitIsInfinite();
    747847        LayoutUnit trackGrowthPotential = infiniteGrowthPotential ? track.growthLimit() : track.growthLimit() - trackBreadth;

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

    r182472 r182628  
    8888    void clearGrid();
    8989
    90     typedef LayoutUnit (RenderGrid::* SizingFunction)(RenderBox&, GridTrackSizingDirection, Vector<GridTrack>&);
    91     typedef const LayoutUnit& (GridTrack::* AccumulatorGetter)() const;
    92     typedef void (GridTrack::* AccumulatorGrowFunction)(LayoutUnit);
    93     typedef bool (GridTrackSize::* FilterFunction)() const;
     90    enum TrackSizeRestriction {
     91        AllowInfinity,
     92        ForbidInfinity,
     93    };
     94    enum TrackSizeComputationPhase {
     95        ResolveIntrinsicMinimums,
     96        ResolveMaxContentMinimums,
     97        ResolveIntrinsicMaximums,
     98        ResolveMaxContentMaximums,
     99    };
     100    enum DistributeSpacePhase {
     101        DistributeSpaceToBaseSizes,
     102        DistributeSpaceToGrowthLimits,
     103    };
     104    static const LayoutUnit& trackSizeForTrackSizeComputationPhase(TrackSizeComputationPhase, GridTrack&, TrackSizeRestriction);
     105    static bool shouldProcessTrackForTrackSizeComputationPhase(TrackSizeComputationPhase, const GridTrackSize&);
     106    static bool trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase(TrackSizeComputationPhase, const GridTrackSize&);
     107    static void updateTrackSizeForTrackSizeComputationPhase(TrackSizeComputationPhase, GridTrack&);
     108    LayoutUnit currentItemSizeForTrackSizeComputationPhase(TrackSizeComputationPhase, RenderBox&, GridTrackSizingDirection, Vector<GridTrack>& columnTracks);
     109
    94110    typedef struct GridItemsSpanGroupRange GridItemsSpanGroupRange;
    95111    void resolveContentBasedTrackSizingFunctionsForNonSpanningItems(GridTrackSizingDirection, const GridCoordinate&, RenderBox& gridItem, GridTrack&, Vector<GridTrack>& columnTracks);
    96     void resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection, GridSizingData&, const GridItemsSpanGroupRange&, FilterFunction, SizingFunction, AccumulatorGetter trackSize, AccumulatorGetter correctedTrackSize, AccumulatorGrowFunction, FilterFunction growAboveMaxBreadthFilterFunction = nullptr);
    97     void distributeSpaceToTracks(Vector<GridTrack*>&, const Vector<GridTrack*>* growBeyondGrowthLimitsTracks, AccumulatorGetter, LayoutUnit& availableLogicalSpace);
     112    template <TrackSizeComputationPhase> void resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection, GridSizingData&, const GridItemsSpanGroupRange&);
     113    template <DistributeSpacePhase> void distributeSpaceToTracks(Vector<GridTrack*>&, const Vector<GridTrack*>* growBeyondGrowthLimitsTracks, LayoutUnit& availableLogicalSpace);
    98114
    99115    double computeNormalizedFractionBreadth(Vector<GridTrack>&, const GridSpan& tracksSpan, GridTrackSizingDirection, LayoutUnit availableLogicalSpace) const;
Note: See TracChangeset for help on using the changeset viewer.