ReactivePropertyとして定義したプロパティに値が反映されない

dhq-boiler commented 2 years ago

こんにちは。お世話になっております。

質問があります。

現在私のリポジトリ boiler's Graphics でラスター画像をベクター画像に変換する試みに取り組んでいます。

主要な処理は OpenCvSharp にほぼまかせてしまっているのですが、キャンパスにアイテムとして表示する際はどうしても追加の実装が必要になります。ひとまず実装してみました。

    //DiagramViewModel.cs

    VectorImagingCommand = new DelegateCommand(() =>
    {
        var selectedItem = SelectedItems.Value.First() as PictureDesignerItemViewModel;
        Remove(selectedItem);
        using (OpenCvSharp.Mat target = EnableImageEmbedding.Value && selectedItem.EmbeddedImage.Value != null ? selectedItem.EmbeddedImage.Value.ToMat() : new OpenCvSharp.Mat(selectedItem.FileName))
        using (OpenCvSharp.Mat output = new OpenCvSharp.Mat())
        {
            if (target.Type() == OpenCvSharp.MatType.CV_8UC3)
            {
                OpenCvSharp.Cv2.CvtColor(target, target, OpenCvSharp.ColorConversionCodes.BGR2BGRA);
            }
            const int MAX_CLUSTERS = 8;
            Kmeans(target, output, MAX_CLUSTERS, out var sets);
            SetAlpha255(output);
            var bag = new ConcurrentBag<SelectableDesignerItemViewModelBase>();
            ParallelOptions options = new ParallelOptions();
            options.MaxDegreeOfParallelism = Environment.ProcessorCount;
            Parallel.For(0, sets.Count(), options, i =>
            {
                Stopwatch sw = Stopwatch.StartNew();
                Debug.WriteLine($"{i} / {sets.Count()} BEGIN PROCESS");
                var color = sets.ElementAt(i);
                //extract
                var extracted = ExtractColor(output, color);
                //grayscale
                using (var grayscaled = new OpenCvSharp.Mat())
                {
                    OpenCvSharp.Cv2.CvtColor(extracted, grayscaled, OpenCvSharp.ColorConversionCodes.BGRA2GRAY);
                    //threshold
                    using (var thresholded = new OpenCvSharp.Mat())
                    {
                        OpenCvSharp.Cv2.Threshold(grayscaled, thresholded, 128, 255, OpenCvSharp.ThresholdTypes.Otsu);
                        //findcontours
                        OpenCvSharp.Cv2.FindContours(thresholded, out var contours, out var hierarchy, OpenCvSharp.RetrievalModes.List, OpenCvSharp.ContourApproximationModes.ApproxNone);
                        for (int j = 0; j < contours.Count(); ++j)
                        {
                            Stopwatch sw1 = Stopwatch.StartNew();
                            Debug.WriteLine($"{i} {j}/{contours.Count()} BEGIN");
                            var array = contours[j];
                            var polyBezier = new PolyBezierViewModel();
                            polyBezier.Owner = this;
                            for (int k = 0; k < array.Count(); k++)
                            {
                                var contour = array[k];
                                polyBezier.Points.Add(new Point(contour.X, contour.Y));
                            }
                            polyBezier.EdgeBrush.Value = Brushes.Transparent;
                            polyBezier.EdgeThickness.Value = 0;
                            polyBezier.LeftTop.Value = new Point(polyBezier.Points.Select(x => x.X).Min() - polyBezier.Owner.EdgeThickness.Value.Value / 2, polyBezier.Points.Select(x => x.Y).Min() - polyBezier.Owner.EdgeThickness.Value.Value / 2);
                            polyBezier.ZIndex.Value = Layers.SelectRecursive<LayerTreeViewItemBase, LayerTreeViewItemBase>(x => x.Children.Value).Count();
                            polyBezier.IsSelected.Value = true;
                            polyBezier.IsVisible.Value = true;
                            var union = Combine(GeometryCombineMode.Union, polyBezier);
                            var fillbrush = new SolidColorBrush(Color.FromRgb(color.Item2, color.Item1, color.Item0));
                            fillbrush.Freeze();
                            union.FillBrush.Value = fillbrush;
                            union.IsSelected.Value = false;
                            union.PathGeometry.Value.Freeze();
                            bag.Add(union);
                            sw1.Stop();
                            Debug.WriteLine($"{i} {j}/{contours.Count()} END {sw1.ElapsedMilliseconds}ms");
                        }
                    }
                }
                sw.Stop();
                Debug.WriteLine($"{i} / {sets.Count()} end process {sw.ElapsedMilliseconds}ms");
            });
            List<LayerTreeViewItemBase> l = new List<LayerTreeViewItemBase>();
            //ConcurrentBagからListに詰め替え、SelectableDesignerItemViewModelBaseをLayerItemに変換する
            while (bag.TryTake(out var item))
            {
                Stopwatch sw = Stopwatch.StartNew();
                Debug.WriteLine($"adding item. remain:{bag.Count()}");
                var i = new LayerItem(item.Clone() as SelectableDesignerItemViewModelBase, SelectedLayers.Value.First(), Name.GetNewLayerItemName(this));
                i.Color.Value = Randomizer.RandomColor(new Random());
                i.IsVisible.Value = true;
                i.IsSelected.Value = false;
                l.Add(i);
                sw.Stop();
                Debug.WriteLine($"added item. {sw.ElapsedMilliseconds}ms");
            }
            DisposeProperties();
            InitializeProperties_Layers(isPreview);
            AddNewLayer(mainWindowViewModel, false);
            var firstSelectedLayer = SelectedLayers.Value.First();
            firstSelectedLayer.Children.Value = new ObservableCollection<LayerTreeViewItemBase>(l); //ObservableCollectionを置き換え（※）
            InitializeProperties_Items(isPreview);
            SetSubscribes(false);
        }
    });

これで上手くいくかと思ったのですが、上記の処理が実装されているDiagramViewModelクラスのAllItemsプロパティに※で置き換えたアイテムが反映されないのです。

AllItemsプロパティは以下のようになっています。


        //DiagramViewModel.cs

        public ReadOnlyReactivePropertySlim<SelectableDesignerItemViewModelBase[]> AllItems { get; set; }

        :

        private void SetAllItems()
        {
            AllItems = Layers.CollectionChangedAsObservable()
                             .Select(_ => Layers.Select(x => x.LayerItemsChangedAsObservable()).Merge()
                                                .Merge(this.ObserveProperty(y => y.BackgroundItem.Value).ToUnit()))
                             .Switch()
                             .Do(_ => Debug.WriteLine(String.Concat("debug ", string.Join(", ", Layers.Select(x => x?.ToString() ?? "null")))))
                             .Select(_ => Layers.SelectRecursive<LayerTreeViewItemBase, LayerTreeViewItemBase>(x => x.Children.Value)
                                                .Where(x => x.GetType() == typeof(LayerItem))
                                                .Select(y => (y as LayerItem).Item.Value)
                                                .Union(new SelectableDesignerItemViewModelBase[] { BackgroundItem.Value })
                                                .ToArray())
                             .ToReadOnlyReactivePropertySlim(Array.Empty<SelectableDesignerItemViewModelBase>());
        }

LayerItemsChangedAsObservableメソッドは以下の通りです。


        //LayerTreeViewItemBase.cs

        public IObservable<Unit> LayerItemsChangedAsObservable()
        {
            var ox1 = Children.Value.ObserveElementObservableProperty(x => (x as LayerItem).Item)
                        .ToUnit()
                        .Merge(Children.Value
                                       .CollectionChangedAsObservable()
                                       .Where(x => x.Action == NotifyCollectionChangedAction.Remove || x.Action == NotifyCollectionChangedAction.Reset)
                                       .ToUnit());
            var ox2 = Children.Value.ObserveElementObservableProperty(x => (x as LayerItem).Item)
                        .ToUnit()
                        .Merge(Children.Value.Select(x => x.LayerItemsChangedAsObservable()).Merge());
            var ox3 = this.ObserveProperty(x => x.Children.Value).ToUnit();
            return ox1.Merge(ox2).Merge(ox3);
        }

SelectRecursive拡張メソッドは以下のようになっています。


        //Extensions.cs

        public static IEnumerable<T2> SelectRecursive<T1, T2>(this IEnumerable<T1> source, Func<T2, IEnumerable<T2>> selector) where T1 : class where T2 : class
        {
            var ret = new ConcurrentBag<T2>();
            Parallel.For(0, source.Count(), i =>
            {
                var parent = source.ElementAt(i);
                ret.Add(parent as T2);

                var children = selector(parent as T2);
                var c = SelectRecursive(children, selector);
                for (int j = 0; j < c.Count(); j++)
                {
                    var child = c.ElementAt(j);
                    ret.Add(child);
                }
            });
            return ret;
        }

テスト用アプリ

アプリのミニマムバージョンを用意しています。場所はQuestion20220226.slnです。ミニマムバージョンでは2つのツール、四角形描画ツールと画像ファイル描画ツールを実装しています。また画像オブジェクトを右クリックすると、「ベクターオブジェクトに変換」というコンテキストメニューがあります。これが本件の処理です。この処理は重いので完了までに時間がかかると思います。

ユニットテスト

また、ユニットテストを用意しています。ReactivePropertyTest.csです。２つテストメソッドがあります。

Children代入
ReactiveCollection初期化

1が今一番成功させたいテストメソッドです。現状、これがテスト実行時に失敗してしまいます。AllItems.Valueはemptyと出てしまいます。

追伸

2は※でObservableCollectionをnewしつつListオブジェクトで初期化していますが、当初はこれはReactiveCollectionでした。しかし、ReactiveCollectionにアイテムを一つ一つ追加していくと膨大な時間がかかったので、(10000+個のアイテムを追加しようとしていて1個当たり1000msかかった記憶があります)工夫しようと試みました。最初、ReactiveCollectionをnewして初期化時にコンストラクタにまとめて渡す方法を試しました。2でも動作することを確認していますが、プロダクトコード上でやってみると10000+のアイテムを渡しているはずなのに0個になってしまうのです。原因はToObservableメソッドにあると思うのですが、結局わからず、次にObservableCollectionをnewする方法に切り替えました。ReactivePropertyの作者の皆さんからすると、あまりヘビーな処理はReactivePropertyには向いていないと思っているかもしれませんが、リソースを駆使すればどうにかする方法があるのではないかと思っています。

まとめ

話は長くなりましたが、AllItemsが正しく反映されるようになることを私は期待しています。何か心当たりはないでしょうか。

よろしくお願いいたします。

runceel commented 2 years ago

@dhq-boiler すいません。ただ、書いたコードがバグっているので直してほしいという投稿に見えます。具体的にReactivePropertyのなんの機能に関する質問（もしくはバグや改善要望）ですか？？

dhq-boiler commented 2 years ago

わかりにくい質問をしてしまいどうもすみません。

具体的に質問しますと、ObservePropertyでReactivePropertySlimクラスのValueを指定した時に、ここにnewされて生成された新規のObserveCollectionが代入された場合、正しく通知が行われれますか？

プロパティの定義

public ReactivePropertySlim<ObservableCollection<UserClass>> Children { get; set; } = new ReactivePropertySlim<ObservableCollection<UserClass>>();

監視の開始

ObserveProperty(x => x.Children)
または
ObserveProperty(x => x.Children.Value)

上記のコードが実行された後に、Children.Valueに新規にインスタンス化したObservableCollecitonオブジェクトを代入します。

Children.Value = new ObservableCollection<UserClass>(array);

私のコード上で試していますが、Children.Valueに代入されたことが監視先のReactivePropertyに伝わっていないような気がしますが、これが自分のコードにバグがあるのか、それともReactivePropertyにバグがあるのか判断つかず質問させていただきました。

runceel commented 2 years ago

@dhq-boiler

以下のようなプログラムを書いて動かすと「変更通知を受け取りました」という表示が3つ表示されます。

using Reactive.Bindings;
using Reactive.Bindings.Extensions;
using System.Collections.ObjectModel;
using System.ComponentModel;

var viewModel = new SomeViewModel();

var rp = viewModel.ObserveProperty(x => x.Children.Value)
    .ToReadOnlyReactivePropertySlim();
rp.Subscribe(x => Console.WriteLine("変更通知を受け取りました"));

viewModel.Children.Value = new ObservableCollection<UserClass>();
viewModel.Children.Value = new ObservableCollection<UserClass>();

class SomeViewModel : INotifyPropertyChanged
{
    public ReactivePropertySlim<ObservableCollection<UserClass>> Children { get; } = new ReactivePropertySlim<ObservableCollection<UserClass>>();

    public event PropertyChangedEventHandler? PropertyChanged;
}

class UserClass
{
}

1 つ目の表示は Subscribe したときのもので、あとの 2 回が Children の値が更新されたときのものです。これを見る限りは変更通知はされているように見えます。

runceel commented 2 years ago

@dhq-boiler 少しコードを変更して PropertyChanged も起きているかどうかという点と、実際に設定された値がわたってきているかが確認できるようにしました。

using Reactive.Bindings;
using Reactive.Bindings.Extensions;
using System.Collections.ObjectModel;
using System.ComponentModel;

var viewModel = new SomeViewModel();

var rp = viewModel.ObserveProperty(x => x.Children.Value)
    .ToReadOnlyReactivePropertySlim();
rp.Subscribe(x => Console.WriteLine($"Subscribe: 要素数 {x?.Count}"));
rp.PropertyChanged += (_, e) =>
{
    Console.WriteLine($"PropertyChanged: 要素数 {rp.Value.Count}");
};

viewModel.Children.Value = new ObservableCollection<UserClass>(new[] { new UserClass() });
viewModel.Children.Value = new ObservableCollection<UserClass>(new[] { new UserClass(), new UserClass() });

class SomeViewModel : INotifyPropertyChanged
{
    public ReactivePropertySlim<ObservableCollection<UserClass>> Children { get; } = new ReactivePropertySlim<ObservableCollection<UserClass>>();

    public event PropertyChangedEventHandler? PropertyChanged;
}

class UserClass
{
}

実行結果は以下のようになります。ちゃんと変更通知が行われて、設定されている値がわたってきていることが確認できます。

Subscribe: 要素数
Subscribe: 要素数 1
PropertyChanged: 要素数 1
Subscribe: 要素数 2
PropertyChanged: 要素数 2

問題が確認できるミニマムバージョンのコードですが、これくらい問題の起きる機能に絞ったものを期待しています。今回はコンソールアプリケーションで作りましたが、WPFなどの特定のプラットフォームでしか問題が再現しない場合は、画面等があっても大丈夫です。開発中のアプリケーションの不要な機能を削ったものではないです。

もし、ReactiveProperty の動作として問題がある部分を見つけたら教えていただければ対応できますが、現在いただいた情報を見る限りだと @dhq-boiler さんの書かれたコードの中に何かしらバグがあるのではないかという疑いの方が強いと思います。アプリのコードベースも大きすぎて私の方では確認する時間をとることは出来ません。

runceel commented 2 years ago

あと、私のところでテスト実行したのですが全部成功しました。

dhq-boiler commented 2 years ago

お疲れさまです。すみません、昨日夜、検証していたのですが、自分のコードにバグがありました。該当箇所を修正したところ、テストはすべて通ったので ReactivePropertyに問題はありませんでした。いろいろと検証してくださりありがとうございました。

runceel / ReactiveProperty

ReactivePropertyとして定義したプロパティに値が反映されない #352

テスト用アプリ

ユニットテスト

追伸

まとめ