ScrollBox calculates incorrect scroll-offset value for its pixel coordinated space

[Undisclosed-Information]

There is currently a bug with ScrollBoxes when using elements that have a static extent defined (e.g. scrollView:SetElementExtent(25)), leading to an incorrect dataIndex being calculated in the ScrollBoxListViewMixin.CalculateDataIndices function.

The bug causes the following issues with the calculated dataIndex:

1. if dataIndexBegin is affected it becomes dataIndexBegin - 1
2. if dataIndexEnd is affected it becomes dataIndexEnd + 1

Which further leads to having (most of time) 1 extra-not-visible element updated. I say "most of the times", because the underlying error is caused by floating point arithmetics returned from the ScrollBoxBaseMixin.GetDerivedScrollOffset function:

function ScrollBoxBaseMixin:GetDerivedScrollOffset()
     return self:GetDerivedScrollRange() * self:GetScrollPercentage(); --this does not represent WoW's pixel coordinated space!
end

Note that this bug is not easy to find without proper debugging tools, because the standard LUA function print (old fashioned debugtool for addon-authors) rounds a number to 14 digits before displaying it. So basically 1099.99999999999 (more than 14 digits), is displayed as 1100.

Let me explain this bug more in detail:

1. When a call is made to the ScrollBox.Update function, it checks if the data-range of the elements has changed.
2. This data-range is calculated (for static element extents) in the ScrollBoxListViewMixin.CalculateDataIndices function.
3. The relevant code for said function is shown below or see github link:

--ScrollBoxListViewMixin.CalculateDataIndice

local dataIndexBegin;
local scrollOffset = scrollBox:GetDerivedScrollOffset(); --the 1st culprit!
local upperPadding = scrollBox:GetUpperPadding();
local extentBegin = upperPadding;

if self:HasIdenticalElementExtents() then
     local extentWithSpacing = self:GetIdenticalElementExtents() + spacing;
     local intervals = math.floor(math.max(0, scrollOffset - upperPadding) / extentWithSpacing); --the 2nd culprit!
     dataIndexBegin = 1 + (intervals * stride);
     local extentTotal = (1 + intervals) * extentWithSpacing;
     extentBegin = extentBegin + extentTotal;
     ....

5. The 1st culprit is not representing exactly as an integer (e.g. 1099.99999999999 instead of 1100).
6. If our elements have an extent of 25, then the 45th element should start at 25 * 45 = offset 1100.
7. The 2nd culprit invokes math.floor on the fake integer. If we use the example above, we now have 1099 instead of 1100.
8. We are now missing (1100 - 1099 =) 1 pixel in the offset to the first visible element (code thinks 1 pixel is visible of the previous element), leading to the issue explained above:

if dataIndexBegin is affected it becomes `dataIndexBegin - 1

Solution

The bug can be fixed by changing the ScrollBoxBaseMixin.GetDerivedScrollOffset function from this:

function ScrollBoxBaseMixin:GetDerivedScrollOffset()
     return self:GetDerivedScrollRange() * self:GetScrollPercentage(); --this does not represent WoW's pixel coordinated space!
end

to this:

function ScrollBoxBaseMixin:GetDerivedScrollOffset()
     return Round(self:GetDerivedScrollRange() * self:GetScrollPercentage()); --Now we adhere to WoW's pixel coordinated space!
end

Why should we use Round for the calculation and not Math.Ceil or Math.Floor?

This is because pixel calculations in WoW are based of the PixelUtil.GetNearestPixelSize function:

function PixelUtil.GetNearestPixelSize(uiUnitSize, layoutScale, minPixels)
    if uiUnitSize == 0 and (not minPixels or minPixels == 0) then
        return 0;
    end

    local uiUnitFactor = PixelUtil.GetPixelToUIUnitFactor();
    local numPixels = Round((uiUnitSize * layoutScale) / uiUnitFactor);
    if minPixels then
        if uiUnitSize < 0.0 then
            if numPixels > -minPixels then
                numPixels = -minPixels;
            end
        else
            if numPixels < minPixels then
                numPixels = minPixels;
            end
        end
    end

    return numPixels * uiUnitFactor / layoutScale;
end

More specifically, this is the line that matters:

local numPixels = Round((uiUnitSize * layoutScale) / uiUnitFactor); --see the usage of 'Round' here? Thats why we use it!

[Undisclosed-Information]

Added a GIF of the bug.