Merge pull request #1447 from b-editor/feat/optimize-operation-observer

yuto-trd · web-flow · commit 7b9e0eacc5f1 · 2025-12-29T19:45:13.000+09:00
Optimize property tracking with CorePropertyOperationObserver
diff --git a/src/Beutl.Editor/Observers/CoreObjectOperationObserver.cs b/src/Beutl.Editor/Observers/CoreObjectOperationObserver.cs
@@ -14,8 +14,7 @@ public sealed class CoreObjectOperationObserver : IOperationObserver
 {
     private readonly ICoreObject _object;
     private readonly string _propertyPath;
-    private readonly Dictionary<int, CoreObjectOperationObserver> _childPublishers = new();
-    private readonly Dictionary<int, IOperationObserver> _collectionPublishers = new();
+    private readonly Dictionary<int, IOperationObserver> _corePropertyPublishers = new();
     private readonly Dictionary<string, IOperationObserver> _enginePropertyPublishers = new();
     private SplineEasingOperationObserver? _splineEasingPublisher;
     private NodeItemOperationObserver? _nodeItemPublisher;
@@ -47,7 +46,12 @@ public CoreObjectOperationObserver(
             .Where(i => !string.IsNullOrEmpty(i))
             .ToHashSet();
         InitializeChildPublishers();
-        obj.PropertyChanged += OnPropertyChanged;
+
+        // Easingの変更を監視
+        if (obj is KeyFrame)
+        {
+            obj.PropertyChanged += OnPropertyChanged;
+        }
 
         if (obj is EngineObject engineObject)
         {
@@ -61,12 +65,7 @@ public void Dispose()
     {
         _object.PropertyChanged -= OnPropertyChanged;
 
-        foreach (CoreObjectOperationObserver child in _childPublishers.Values)
-        {
-            child.Dispose();
-        }
-
-        foreach (IOperationObserver collection in _collectionPublishers.Values)
+        foreach (IOperationObserver collection in _corePropertyPublishers.Values)
         {
             collection.Dispose();
         }
@@ -92,7 +91,8 @@ private string BuildPropertyPath(string propertyName)
 
     private void InitializeChildPublishers()
     {
-        foreach (CoreProperty property in PropertyRegistry.GetRegistered(_object.GetType()))
+        var objectType = _object.GetType();
+        foreach (CoreProperty property in PropertyRegistry.GetRegistered(objectType))
         {
             if (Hierarchical.HierarchicalParentProperty.Id == property.Id) continue;
             if (_propertiesToTrack != null && !_propertiesToTrack.Contains(property.Name))
@@ -101,45 +101,37 @@ private void InitializeChildPublishers()
             }
 
             // Check if property is excluded from tracking
-            if (property.TryGetMetadata<CorePropertyMetadata>(_object.GetType(), out var metadata)
+            if (property.TryGetMetadata<CorePropertyMetadata>(objectType, out var metadata)
                 && !metadata.Tracked)
             {
                 continue;
             }
 
-            object? value = _object.GetValue(property);
             string childPath = BuildPropertyPath(property.Name);
 
-            if (value is IList list)
-            {
-                var elementType = ArrayTypeHelpers.GetElementType(list.GetType());
-                if (elementType == null) throw new InvalidOperationException("Could not determine the element type of the list.");
-                var observerType = typeof(CollectionOperationObserver<>).MakeGenericType(elementType);
+            var observerType = typeof(CorePropertyOperationObserver<>).MakeGenericType(property.PropertyType);
+            var propertyPublisher = (IOperationObserver)Activator.CreateInstance(
+                observerType,
+                _operations,
+                _object,
+                property,
+                _sequenceNumberGenerator,
+                childPath,
+                _propertyPathsToTrack)!;
 
-                var collectionPublisher = Activator.CreateInstance(observerType,
-                    _operations, list, _object,
-                    childPath, _sequenceNumberGenerator, _propertyPathsToTrack)!;
-                _collectionPublishers.Add(property.Id, (IOperationObserver)collectionPublisher);
-            }
-            else if (value is ICoreObject child)
-            {
-                var childPublisher = new CoreObjectOperationObserver(
-                    _operations,
-                    child,
-                    _sequenceNumberGenerator,
-                    childPath,
-                    _propertyPathsToTrack);
-                _childPublishers.Add(property.Id, childPublisher);
-            }
+            _corePropertyPublishers.Add(property.Id, propertyPublisher);
         }
 
-        InitializeSplineEasingPublisher();
+        RecreateSplineEasingPublisher();
         InitializeNodeItemPublisher();
     }
 
-    private void InitializeSplineEasingPublisher()
+    private void RecreateSplineEasingPublisher()
     {
-        if (_object is KeyFrame keyFrame && keyFrame.Easing is SplineEasing splineEasing)
+        _splineEasingPublisher?.Dispose();
+        _splineEasingPublisher = null;
+
+        if (_object is KeyFrame { Easing: SplineEasing splineEasing } keyFrame)
         {
             string easingPath = BuildPropertyPath(nameof(KeyFrame.Easing));
             _splineEasingPublisher = new SplineEasingOperationObserver(
@@ -191,89 +183,18 @@ private void InitializeEnginePropertyPublishers(EngineObject engineObject)
 
     private void OnPropertyChanged(object? sender, PropertyChangedEventArgs e)
     {
-        // Suppress publishing during remote operation application to prevent echo-back
-        if (PublishingSuppression.IsSuppressed)
-        {
+        if (PublishingSuppression.IsSuppressed) return;
+
+        if (e is not CorePropertyChangedEventArgs<Easing> args || sender is not CoreObject source)
             return;
-        }
 
-        if (e is not CorePropertyChangedEventArgs args ||
-            sender is not ICoreObject source ||
-            args.Property.Id == Hierarchical.HierarchicalParentProperty.Id)
-        {
+        if (args.Property.Id != KeyFrame.EasingProperty.Id)
             return;
-        }
 
         if (_propertiesToTrack != null &&
             !_propertiesToTrack.Contains(args.Property.Name))
-        {
             return;
-        }
-
-        // Check if property is excluded from tracking
-        if (args.Property.TryGetMetadata<CorePropertyMetadata>(_object.GetType(), out var metadata)
-            && !metadata.Tracked)
-        {
-            return;
-        }
-
-        if (_childPublishers.Remove(args.Property.Id, out var childPublisher))
-        {
-            childPublisher.Dispose();
-        }
-
-        if (_collectionPublishers.Remove(args.Property.Id, out var collectionPublisher))
-        {
-            collectionPublisher.Dispose();
-        }
-
-        string childPath = BuildPropertyPath(args.Property.Name);
-
-        if (args.NewValue is IList list)
-        {
-            var elementType = ArrayTypeHelpers.GetElementType(list.GetType());
-            if (elementType == null) throw new InvalidOperationException("Could not determine the element type of the list.");
-            var observerType = typeof(CollectionOperationObserver<>).MakeGenericType(elementType);
-
-            var newPublisher = Activator.CreateInstance(observerType,
-                _operations, list, source,
-                childPath, _sequenceNumberGenerator, _propertyPathsToTrack)!;
-            _collectionPublishers.Add(args.Property.Id, (IOperationObserver)newPublisher);
-        }
-        else if (args.NewValue is ICoreObject newChild)
-        {
-            var newPublisher = new CoreObjectOperationObserver(
-                _operations,
-                newChild,
-                _sequenceNumberGenerator,
-                childPath,
-                _propertyPathsToTrack);
-            _childPublishers.Add(args.Property.Id, newPublisher);
-        }
-
-        // Handle SplineEasing changes for KeyFrame
-        if (_object is KeyFrame keyFrame && args.Property.Id == KeyFrame.EasingProperty.Id)
-        {
-            _splineEasingPublisher?.Dispose();
-            _splineEasingPublisher = null;
-
-            if (args.NewValue is SplineEasing newSplineEasing)
-            {
-                _splineEasingPublisher = new SplineEasingOperationObserver(
-                    _operations,
-                    newSplineEasing,
-                    _sequenceNumberGenerator,
-                    keyFrame,
-                    childPath,
-                    _propertyPathsToTrack);
-            }
-        }
 
-        var operationType = typeof(UpdatePropertyValueOperation<>).MakeGenericType(args.Property.PropertyType);
-        var operation = (ChangeOperation)Activator.CreateInstance(
-            operationType,
-            source, childPath, args.NewValue, args.OldValue)!;
-        operation.SequenceNumber = _sequenceNumberGenerator.GetNext();
-        _operations.OnNext(operation);
+        RecreateSplineEasingPublisher();
     }
 }
diff --git a/src/Beutl.Editor/Observers/CorePropertyOperationObserver.cs b/src/Beutl.Editor/Observers/CorePropertyOperationObserver.cs
@@ -0,0 +1,110 @@
+﻿using System.Collections;
+using System.ComponentModel;
+using System.Reactive.Subjects;
+using Beutl.Editor.Operations;
+using Beutl.Serialization;
+
+namespace Beutl.Editor.Observers;
+
+public sealed class CorePropertyOperationObserver<T> : IOperationObserver
+{
+    private CoreObjectOperationObserver? _childPublisher;
+    private IOperationObserver? _collectionPublisher;
+    private readonly CoreProperty<T> _property;
+    private readonly ICoreObject _object;
+    private readonly Subject<ChangeOperation> _operations = new();
+    private readonly HashSet<string>? _propertyPathsToTrack;
+    private readonly string _propertyPath;
+    private readonly OperationSequenceGenerator _sequenceNumberGenerator;
+    private readonly IDisposable? _subscription;
+
+    public CorePropertyOperationObserver(
+        IObserver<ChangeOperation>? observer,
+        ICoreObject obj,
+        CoreProperty<T> property,
+        OperationSequenceGenerator sequenceNumberGenerator,
+        string propertyPath,
+        HashSet<string>? propertyPathsToTrack = null)
+    {
+        _object = obj;
+        _property = property;
+        _propertyPath = propertyPath;
+        _sequenceNumberGenerator = sequenceNumberGenerator;
+        _propertyPathsToTrack = propertyPathsToTrack;
+
+        if (observer != null)
+        {
+            _subscription = _operations.Subscribe(observer);
+        }
+
+        RecreateChildPublisher();
+        obj.PropertyChanged += OnPropertyChanged;
+    }
+
+    public IObservable<ChangeOperation> Operations => _operations;
+
+    public void Dispose()
+    {
+        _object.PropertyChanged -= OnPropertyChanged;
+
+        _childPublisher?.Dispose();
+        _childPublisher = null;
+
+        _collectionPublisher?.Dispose();
+        _collectionPublisher = null;
+
+        _subscription?.Dispose();
+        _operations.OnCompleted();
+    }
+
+    private void RecreateChildPublisher()
+    {
+        _childPublisher?.Dispose();
+        _childPublisher = null;
+
+        _collectionPublisher?.Dispose();
+        _collectionPublisher = null;
+
+        T value = _object.GetValue(_property);
+
+        switch (value)
+        {
+            case IList list:
+                var elementType = ArrayTypeHelpers.GetElementType(list.GetType());
+                if (elementType == null) throw new InvalidOperationException("Could not determine the element type of the list.");
+                var observerType = typeof(CollectionOperationObserver<>).MakeGenericType(elementType);
+
+                _collectionPublisher = (IOperationObserver)Activator.CreateInstance(observerType,
+                    _operations, list, _object,
+                    _propertyPath, _sequenceNumberGenerator, _propertyPathsToTrack)!;
+                break;
+            case ICoreObject child:
+                _childPublisher = new CoreObjectOperationObserver(
+                    _operations,
+                    child,
+                    _sequenceNumberGenerator,
+                    _propertyPath,
+                    _propertyPathsToTrack);
+                break;
+        }
+    }
+
+    public void OnPropertyChanged(object? sender, PropertyChangedEventArgs e)
+    {
+        if (PublishingSuppression.IsSuppressed) return;
+        if (e is not CorePropertyChangedEventArgs<T> args) return;
+        if (sender is not ICoreObject source) return;
+
+        // プロパティが同じか
+        if (args.Property.Id != _property.Id) return;
+
+        RecreateChildPublisher();
+
+        var operation = new UpdatePropertyValueOperation<T?>(
+            (CoreObject)source, _propertyPath, args.NewValue, args.OldValue)
+        {
+            SequenceNumber = _sequenceNumberGenerator.GetNext()
+        };
+        _operations.OnNext(operation);
+    }
+}
diff --git a/src/Beutl.Editor/Observers/EnginePropertyOperationObserver.cs b/src/Beutl.Editor/Observers/EnginePropertyOperationObserver.cs
@@ -129,11 +129,11 @@ private void OnExpressionChanged(IExpression<T>? obj)
             return;
         }
 
-        var operationType = typeof(UpdatePropertyValueOperation<>).MakeGenericType(typeof(IExpression<T>));
-        var operation = (ChangeOperation)Activator.CreateInstance(
-            operationType,
-            _object, $"{_propertyPath}.Expression", obj, _expression)!;
-        operation.SequenceNumber = _sequenceNumberGenerator.GetNext();
+        var operation = new UpdatePropertyValueOperation<IExpression<T>?>(
+            (CoreObject)_object, $"{_propertyPath}.Expression", obj, _expression)
+        {
+            SequenceNumber = _sequenceNumberGenerator.GetNext()
+        };
         _operations.OnNext(operation);
         _expression = obj;
     }
@@ -161,11 +161,11 @@ private void OnAnimationChanged(IAnimation<T>? newAnimation)
                 _propertyPathsToTrack);
         }
 
-        var operationType = typeof(UpdatePropertyValueOperation<>).MakeGenericType(typeof(IAnimation<T>));
-        var operation = (ChangeOperation)Activator.CreateInstance(
-            operationType,
-            _object, animationPath, newAnimation, _animation)!;
-        operation.SequenceNumber = _sequenceNumberGenerator.GetNext();
+        var operation = new UpdatePropertyValueOperation<IAnimation<T>?>(
+            (CoreObject)_object, animationPath, newAnimation, _animation)
+        {
+            SequenceNumber = _sequenceNumberGenerator.GetNext()
+        };
         _operations.OnNext(operation);
         _animation = newAnimation;
     }
@@ -183,11 +183,11 @@ private void OnValueChanged(object? sender, PropertyValueChangedEventArgs<T> e)
 
         InitializePublishers(e.NewValue);
 
-        var operationType = typeof(UpdatePropertyValueOperation<>).MakeGenericType(typeof(T));
-        var operation = (ChangeOperation)Activator.CreateInstance(
-            operationType,
-            _object, _propertyPath, e.NewValue, e.OldValue)!;
-        operation.SequenceNumber = _sequenceNumberGenerator.GetNext();
+        var operation = new UpdatePropertyValueOperation<T>(
+            (CoreObject)_object, _propertyPath, e.NewValue, e.OldValue)
+        {
+            SequenceNumber = _sequenceNumberGenerator.GetNext()
+        };
         _operations.OnNext(operation);
     }
 }
