Path Geometry update fix for 4748 bug rerender on change for paths segments

[EvanRuiz]

What does the pull request do?

Fixes #4748 [BUG] Rerender on change for Paths, Segments and e.g.

I found this issue independently when trying to bind to PolyLineSegment.Points. Ex:

<ComboBox x:Name="EasingsComboBox">
    <ComboBox.ItemTemplate>
        <DataTemplate>
            <StackPanel Orientation="Horizontal">
                <Path Stroke="{DynamicResource CatalogBaseHighColor}" Fill="{DynamicResource CatalogBaseHighColor}">
                    <Path.Data>
                        <PathGeometry>
                            <PathFigure StartPoint="0,30" IsClosed="True">
                                <PolyLineSegment Points="{Binding ., Converter={StaticResource EasingPointsConverter}, ConverterParameter=30}"/>
                            </PathFigure>
                        </PathGeometry>
                    </Path.Data>
                </Path>
                <TextBlock Text="{Binding .}" VerticalAlignment="Center" Margin="10, 0, 0, 0"></TextBlock>
            </StackPanel>              
        </DataTemplate>
    </ComboBox.ItemTemplate>
</ComboBox>

I would not see the path update in the combo box selection after picking another item.

It appears to be the same root cause as #4748

What is the current behavior?

Updates to path geometry (figures, segments, etc) do not trigger anInvalidateGeometry() call so the parent path is not re-rendered as expected. Specifically changes to PathGeometry.Figures do not trigger OnFiguresChanged.

What is the updated/expected behavior with this PR?

Updates to PathGeometry.Figures should now trigger OnFiguresChanged where the existing code can then properly call InvalidateGeometry() as appropriate.

How was the solution implemented (if it's not obvious)?

I initially thought this was a more involved issue that needed changes to all the geometry classes where child geometry would need to invalidate their parent on property changes through an AffectsGeometry<>(Properties) type solution.

However, in further debugging and testing the root issue turns out to only need a one-liner fix. Using the property Figures instead of the private member _figures directly when setting the initial PathFigures object in PathGeometry allows the existing code to monitor the changes properly and invalidate the parent path geometry.

Checklist

• Added unit tests
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Fixed issues

Fixes #4748 [BUG] Rerender on change for Paths, Segments and e.g.

Additional Work

I had made some additions to the ControlCatalog demonstrating various path geometries and bindings as part of testing this bug. Would those be considered a feature change and need a new ticket for discussion?

I also have additional tests written (not included in this PR) for verifying the internal InvalidateGeometry() calls are made on all geometry and property changes. This was done initially to validate my initial solution before finding this one-liner fix. These additional tests currently pass separate from this fix, as the PathGeometry.Figures is the only link in the chain that appears broken in triggering the final InvalidateGeometry(). The additional tests are currently written using using Moq.Protected to verify the InvalidateGeometry() calls. Would you like those additional coverage tests added and considered as part of the PR, or best to consider those as a separate PR for additional test coverage and keep this PR simple?

In this PR, I've only included the single test targeted to the root cause that fails and then passes as part of this PR.

[avaloniaui-bot]

You can test this PR using the following package version. 11.3.999-cibuild0054447-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[cla-avalonia]

• All contributors have signed the CLA.

[EvanRuiz]

@cla-avalonia agree

[maxkatz6]

Good find

[maxkatz6]

I had made some additions to the ControlCatalog demonstrating various path geometries and bindings as part of testing this bug. Would those be considered a feature change and need a new ticket for discussion?

We have some Path samples in RenderDemo project IIRC. Probably not necessarily overall.

I also have additional tests written (not included in this PR) for verifying the internal InvalidateGeometry() calls are made on all geometry and property changes.

InvalidateGeometry is rather an implementation detail, which is expected to be eventually phased out, as we plan to move such invalidation to the rendering compositor resources.
But even so, additional tests to cover invalidation scenarios would be useful for sure. We don't use Moq, ideally to avoid it.

[EvanRuiz]

Good find

Thanks!

We have some Path samples in RenderDemo project IIRC. Probably not necessarily overall.

Got it.

InvalidateGeometry is rather an implementation detail, which is expected to be eventually phased out, as we plan to move such invalidation to the rendering compositor resources.
But even so, additional tests to cover invalidation scenarios would be useful for sure. We don't use Moq, ideally to avoid it.

I was thinking it wasn't ideal to need to look into protected method invocation for the tests either, I was initially just focused on that is the method to be sure invalidation occurs in the current implementation. I can change them all to checking for Changed events instead. Would you like a separate PR for those additional coverage tests?