Media Invalidation

[TomEdwardsEnscape]

What does the pull request do?

This PR introduces Media Invalidation, a fast and low-maintenance system which consumes changes to AvaloniaObject instances which exist outside the visual tree, locates the Visual objects which depend on them for their rendered appearance, and calls the Visual.InvalidateVisual method on each one. Object nesting and collections are supported. These changes fix #8767.

Here is a scenario that Media Invalidation adds support for:

<Image>
  <DrawingImage>
    <DrawingGroup>
      <GeometryDrawing>
        <GeometryDrawing.Brush>
          <SolidColorBrush Color="Green"/>
        </GeometryDrawing.Brush>
        <RectangleGeometry Rect="0,0,120,120"/>
      </GeometryDrawing>
    </DrawingGroup>
  </DrawingImage>
</Image>

(See the MediaInvalidationTests class for real implementations similar to this sample.)

With Media Invalidation, changes to the value of SolidColorBrush.Color or RectangleGeometryRect both result in the Image control being invalidated and redrawing. Previously, the Image would never update unless manually forced to. Invalidation occurs even though the value that changed was deeply nested underneath the Image, and the path between the two passes through a DrawingGroup collection.

What is the current behavior?

Currently, invalidation of media objects is implemented in a very haphazard way. There are multiple bespoke systems in multiple classes, often involving complicated WeakEvent subscriptions. These systems are not recursive; links between parents and children must be explicitly made, and they are generally not.

One such system raises the event Pen.Invalidated. The event is not subscribed to, so I removed it and all the supporting code. The other systems I found do actually have effects, so I left them in place. They should be reviewed before being removed as some may also need to support recursive invalidation, and/or could execute without an associated AvaloniaProperty change.

Currently, tests which check for the old media invalidation systems are failing because they expect one invalidation but get two, due to both systems running at the same time. These failures will clear as the old systems are removed.

What is the updated/expected behavior with this PR?

Media Invalidation is an automated and low-maintenance solution for all Avalonia properties. Recursion is automatically handled if the property's value type is an AvaloniaObject. The only steps required are to register each relevant AvaloniaProperty with the Media Invalidation system, to ensure that property changed events are always raised (not guaranteed with DirectProperty) and to use collections which derive from MediaCollection<T>.

How was the solution implemented?

The core of the PR is the static class MediaInvalidation. This has a public method called AffectsMediaRender, which accepts a params array of AvaloniaProperty objects. It subscribes to changes on each property, and registers (with weak references, to prevent GC rooting) the property owner as a "media parent" of any AvaloniaObject which is assigned. Note that unlike objects in the visual and logical trees, a media object can have multiple parents, and can even have the same parent twice (e.g. Fill and Stroke being the same brush).

When the value of a registered media property changes, the media parents tree of the target object is recursively searched for all Visual objects, and each of these objects is visually invalidated.

A second important type is the abstract class MediaCollection<T>. This handles the invalidation of AvaloniaObject collections, by linking each collection member to each of the collection's parents. Everything is handled automatically once the the base type is implemented and assigned to a property registered with the media invalidation system.

Performance note: theme resources

A given AvaloniaObject can always change, so the Media Invalidation system must always track its parents. This can become a problem when a theme resource is frequently used, such as the standard Foreground brush. When I start the Avalonia control gallery, I see that a SolidColorBrush with the colour "Black" has over 1000 parents! This tree won't be searched unless the brush changes, but there is still a runtime and memory cost to all this book-keeping.

To avoid this situation, a new property called AvaloniaObject.IsSealed could be added. Changes to any AvaloniaProperty of a sealed object would be rejected, allowing MediaInvalidation to ignore it. This is exactly what WPF does to solve the same issue: media objects always derive from Freezable, which sets DependencyObject.IsSealed when its Freeze method is called. This happens automatically when such an object is inserted into a resource dictionary.

(Since opening the PR I have spotted #6387, which proposes transforming resources into immutable forms with a XAML directive. That would also resolve the problem.)

Benchmarks

There is a benchmark called SetPropertyThatAffectsRender, but it's not measuring anything on master. It sets and then clears the value of a Pen property on a test object, but because Pen does not implement IAffectsRender no subscriptions are made by Visual.AffectsRender. The only special behaviour being benchmarked is the InvalidateVisual call itself...which also does nothing, because the test object does not have a visual root.

So I fixed the benchmark locally by making it instead set and unset a Brush property. There is no need to commit this change as the changes in this PR cause the unmodified benchmark to become valid.

master

(After correcting the benchmark)

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
SetPropertyThatAffectsRender	1.059 us	0.0211 us	0.0187 us	0.0858	0.0067	0.0019	713 B

MediaInvalidation

(After correcting the benchmark and redirecting Visual.AffectsRender to MediaInvalidation.AffectsMediaRender)

Method	Mean	Error	StdDev	Gen 0	Allocated
SetPropertyThatAffectsRender	384.5 ns	0.59 ns	0.55 ns	0.0229	192 B

That's 62% faster, which is pretty good considering all the extra functionality that this PR adds! Instrumentation shows that after the InvalidateVisual call itself, the next biggest chunks of work are allocating and freeing weak GC handles, in that order.

Checklist

• Added unit tests
• Added XML documentation to any related classes
• What happens to a property created with AddOwner?
• Remove old notification systems (this will fix the failing tests)
• Investigate Memory Leak - DashStyle instances leak from CollectionChanged even handlers #5813
• Handle theme resources with many parents
• Handle GlyphRun, which is mutable but not an AvaloniaObject

Event handlers to investigate

• Visual.AffectsRender (turn into a pass-through)
• Visual.RenderTransformChanged
• Geometry.TransformChanged
• Geometry.AffectsGeometryInvalidate (should this receive recursive events?)
• GeometryCollection.Invalidate
• PathGeometry.InvalidateGeometryFromSegments
• Path.GeometryChangedHandler
• GradientBrush.GradientStopsChanged / GradientBrush.GradientStopChanged

Fixed issues

Fixes #8767

[dnfadmin]

All CLA requirements met.

[kekekeks]

The general plan was to move media invalidation to the render thread by introducing render-thread representations for media objects (the same way it's done in UWP). That would also remove the need to call Render and re-build the list of drawing commands.

[TomEdwardsEnscape]

That sounds like a sensible long-term plan. In the immediate to medium term, this PR fixes bugs and substantially improves performance. It's also an opportunity to normalise everything into a single system which can be further adapted later.

Lastly, I'd vote for always having eventing on the CPU side, so that user code can respond to changes.

If this PR is something you would in principle accept into Avalonia, I will start working on removing the old invalidation events. :)

[robloo]

A unified invalidating system like this is a really, really good idea! Like the detail in this PR too. Hope this gets in for 11.0

About the name: I would keep using the term 'Visual'. So VisualInvalidation.AffectsRender() for example. I don't think it is a good idea to introduce new terminology. Media is also a misnomer in some cases (Glyph run, etc.) and much less generic.

[kekekeks]

MediaParentsBag and MediaCollection look like a good design in general, I think those can be adapted for use on the render thread.

However I'm not sure that we want to merge this PR as is if we are going to implement CompositionBrush, CompositionShape and friends, since in that case we'll have to remove the changes of this PR later.

I'd definitely would like to merge this if we won't get composition objects to work in time before the release.

The general roadmap is:

• remove DeferrredRenderer/ImmediateRenderer, so that Compositor is always used
• change Visual to synchronize properties instead of calling InvalidateVisual all the time.
• implement media objects that live on the render thread

I hope to get that done before November this year.

BTW, there is also a similar problem with UI-thread animations: right now they don't track visual tree attachment at all, so if you have some brush with animation in resources, it would tick even if brush isn't used anywhere.

[TomEdwardsEnscape]

Thanks for the positive feedback, everyone!

I'll leave this PR alone for now, because it sounds like the existing invalidation events are already due to be removed/refactored by people who understand them better than I do.

[robloo]

I'll leave this PR alone for now, because it sounds like the existing invalidation events are already due to be removed/refactored

Personally, I'm sorry to hear that but it makes sense. Your approach here was very clean and thorough and is much better than the code we have now. Honestly, I'm not sure the other planned changes are going to be as good yet.

I certainly hope what was done here will be used where possible by @kekekeks. Even in a UWP-style composition system an invalidation system just like this PR is going to be required even if further composition primitives are added. In the end it might actually be better to finish this PR then modify it later with the composition-required changes. I don't think they are 1-and-1 equivalent.

[TomEdwardsEnscape]

@kekekeks it's now nearly the end of November, what is the status of your composition project?

[Whiletru3]

Any chance to backport this PR in the stable 0.10.x branch ?

We are facing this issue with drawing images used as icons in out desktop application. Those are dependant of a Material.Avalonia color theme MaterialDesignBody.

When we change the theme from light to dark (or vice versa), the images are not updated.
Example :

<DrawingGroup x:Key="AddBookmarkDrawing">
        <DrawingGroup.Children>
            <GeometryDrawing Geometry="F1 M 16,9.98975L 5.98291,9.98975L 5.98291,26.0171L 22.0103,26.0171L 22.0103,16">
                <GeometryDrawing.Pen>
                    <Pen Thickness="1.04178" LineJoin="Round" Brush="{DynamicResource MaterialDesignBody}" />
                </GeometryDrawing.Pen>
            </GeometryDrawing>
            <GeometryDrawing Geometry="F1 M 18.0034,9.98975L 26.0171,9.98975">
                <GeometryDrawing.Pen>
                    <Pen Thickness="1.04178" LineJoin="Round" Brush="{DynamicResource MaterialDesignBody}" />
                </GeometryDrawing.Pen>
            </GeometryDrawing>
            <GeometryDrawing Geometry="F1 M 22.0103,5.98291L 22.0103,13.9966">
                <GeometryDrawing.Pen>
                    <Pen Thickness="1.04178" LineJoin="Round" Brush="{DynamicResource MaterialDesignBody}" />
                </GeometryDrawing.Pen>
            </GeometryDrawing>
            <GeometryDrawing Brush="{DynamicResource MaterialDesignBody}"
                             Geometry="F1 M 15.5993,14.5575L 12.2336,14.5575C 11.9932,14.5575 11.6726,14.8781 11.6726,15.1986L 11.6726,21.3692L 13.9966,19.3657L 16.2404,21.3692L 16.2404,15.1986C 16.1603,14.8781 15.8397,14.5575 15.5993,14.5575 Z " />
        </DrawingGroup.Children>
    </DrawingGroup>

    <DrawingImage x:Key="AddBookmarkImage" Drawing="{DynamicResource AddBookmarkDrawing}" />

[kekekeks]

Since compositor-side brushes aren't likely to land in 11.0, we'll probably be using the approach from this PR for the time being.

[maxkatz6]

@Whiletru3 very unlikely. As this PR has high change to introduce a regression, and we want to avoid affecting 0.10 at this point.

[kekekeks]

Brushes/pens are now automatically updating render-thread resources - #11340

Geometries will use the same infrastructure later.

[TomEdwardsEnscape]

@kekekeks #11340 does not fix #8767, in which a brush is changed. The brush is on a geometry element though; is that what you mean by "geometries will use the same infrastructure"?

[robloo]

@tomenscape I saw #11378 was opened for this. Doesnt look like it is planned for 11.0 though...

[kekekeks]

We are currently wrapping up the required breaking changes so we could get a release and safely continue fixing stuff later. Otherwise 11.0 will be getting postponed indefinitely .

[lindexi]

@kekekeks Reopen this?

[kekekeks]

We've discontinued IAffectsRender usage. Every resource is responsible of updating its server counterpart by itself.