Bug 220915 – Use ScrollSnapOffsetsInfo in the scrolling tree

• Status: RESOLVED
• Resolution: FIXED
• Component: Scrolling

[Martin Robinson]

This bugs track using ScrollSnapOffsetsInfo to abstract away details of the scroll snap offsets in the scrolling tree.

[Martin Robinson]

Created attachment 418271 [details]
Patch

[Martin Robinson]

Created attachment 418274 [details]
Patch

[Martin Robinson]

Created attachment 418275 [details]
Patch

[Martin Robinson]

Created attachment 418279 [details]
Patch

[Martin Robinson]

Created attachment 418290 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 418290 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418290&action=review

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:118
> +        auto convertOffsetsToFloats = [deviceScaleFactor](const Vector<LayoutUnit>& offsets, Vector<float>& floatOffsets)
> +        {
> +            floatOffsets.reserveInitialCapacity(offsets.size());
> +            for (auto& offset : offsets)
> +                floatOffsets.uncheckedAppend(roundToDevicePixel(offset, deviceScaleFactor, false));
> +        };
> +
> +        auto convertOffsetRangesToFloat = [deviceScaleFactor](const Vector<ScrollOffsetRange<LayoutUnit>>& offsetRanges, Vector<ScrollOffsetRange<float>>& floatOffsetRanges)
> +        {
> +            floatOffsetRanges.reserveInitialCapacity(offsetRanges.size());
> +            for (auto& range : offsetRanges)
> +                floatOffsetRanges.uncheckedAppend({ roundToDevicePixel(range.start, deviceScaleFactor, false), roundToDevicePixel(range.end, deviceScaleFactor, false) });
> +        };
> +
> +        convertOffsetsToFloats(offsetInfo->horizontalSnapOffsets, floatInfo.horizontalSnapOffsets);
> +        convertOffsetsToFloats(offsetInfo->verticalSnapOffsets, floatInfo.verticalSnapOffsets);
> +        convertOffsetRangesToFloat(offsetInfo->horizontalSnapOffsetRanges, floatInfo.horizontalSnapOffsetRanges);
> +        convertOffsetRangesToFloat(offsetInfo->verticalSnapOffsetRanges, floatInfo.verticalSnapOffsetRanges);

All this conversion should be in some helpers on ScrollSnapOffsetsInfo. Can we do template specialization goop to just have it be a special operator=?

[Martin Robinson]

Created attachment 418411 [details]
Patch

[Martin Robinson]

(In reply to Simon Fraser (smfr) from comment #6)

> All this conversion should be in some helpers on ScrollSnapOffsetsInfo. Can
> we do template specialization goop to just have it be a special operator=?

Thanks for the review. I've moved all the conversion functions to 
ScrollSnapOffsetsInfo. I'm using a templated method, because we need to pass in an argument for the device scale when converting from LayoutUnits to floats. The method is called convertUnits.

[Simon Fraser (smfr)]

Comment on attachment 418411 [details]
Patch

Nice.

[EWS]

Committed r271937: <https://trac.webkit.org/changeset/271937>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418411 [details].

[Radar WebKit Bug Importer]

<rdar://problem/73652510>