[Skia / iOS] ShadowContainer re-layout issue

[Xiaoy312]

Originally posted by @kazo0 in #737 (review)

I'm seeing some strange behaviors if I add an extra shadow using the ShadowContainerSamplePage here (tested on Skia so far):

Before adding new shadow:

After adding new shadow:

It seems to also break on main but in a different way:
Before adding new shadow:

After adding new shadow:

Not sure if it's really related to these changes but something is different

[Xiaoy312]

@kazo0 @agneszitte
is this required for 4.10?

[kazo0]

Might not be highest prio since its contained to just Skia

[kazo0]

To recap:

We are seeing the following effect when adding a third shadow to the sample control when clicking the Add Shadow button on Skia.GTK

It should look like this:

[Marc-Antoine-Soucy]

Here's a summary of the result of my investigation:
1: The problem is related to the change in height of the child of the shadow container, only adding a shadow does not create any problem.

2: Even if the only thing you call in OnShadowCollectionChanged is OnShadowSizeChanged(); (after adding InvalidateFromShadowPropertyChange(); to OnShadowSizeChanged) , the shadow will disappear when you change the size of the child item, but will reappear when you add a shadow.

3: When you add numbers to the childheight in the OnShadowSizeChanged event, it will not have a meaningful impact on anything.

4: the bug is still Present on main, it's just that it's only present for a few frame.

[Xiaoy312]

@xy ===== UpdateCanvasSize
@xy _canvas.Size: 332.65625x334
@xy _shadowHost.Size: 392.65625x394
@xy _shadowHost.CanvasOffset: @-30,-30

@xy ===== UpdateCanvasSize
@xy _canvas.Size: 332.65625x461
@xy _shadowHost.Size: 392.65625x521
@xy _shadowHost.CanvasOffset: @-30,-30

@xy ===== InvalidateShadows

@xy ===== OnSurfacePainted
@xy _shadowHost.Size: 392.65625x521
@xy _shadowHost.ActualSize: 392.65625x394
@xy NeedsPaint() => true

@xy ===== OnSurfacePainted
@xy _shadowHost.Size: 392.65625x521
@xy _shadowHost.ActualSize: 392.65625x521
@xy NeedsPaint() => false

the issue happen when we quickly set _shadowHost.Width and Height in rapid succession
the actual size lags behind on the 1st paint pass, causing us to paint with the wrong values
and then, the 2nd paint pass (that would have the updated values) got skipped by !NeedsPaint(...) since the paint state didnt change since last pass

[Xiaoy312]

we can properly bypass the 1st call with:

if (_shadowHost.Width != _shadowHost.ActualWidth ||
	_shadowHost.Height != _shadowHost.ActualHeight)
{
	return;
}

^ would need to check if there are case that will cause Size!=ActualSize like other h/v-stretch mode?

[jeromelaban]

^ would need to check if there are case that will cause Size!=ActualSize like other h/v-stretch mode?

Couldn't the Width/Height be NaN in which case this particular set of condition always be true?

[Xiaoy312]

we do hard code them here:
https://github.com/unoplatform/uno.toolkit.ui/blob/main/src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs#L209C38-L209C38

[agneszitte]

Re-Open for backports

[Xiaoy312]

this issue issue was kept opened because:

let's merge this PR if it's approved by the rest of the team, but make sure to add a create a new PR right after that will add regression testing. I've removed the automatic closing of the issue so we can keep track of this.
-- Jerome

[agneszitte]

this issue issue was kept opened because:

let's merge this PR if it's approved by the rest of the team, but make sure to add a create a new PR right after that will add regression testing. I've removed the automatic closing of the issue so we can keep track of this.
-- Jerome

Here is the related issue we can follow for this @Xiaoy312
#791

Keeping this issue opened until last backport is merged

[agneszitte]

Closing issue now that all the backports are done.
Related Tests will be done with #791