RasterizingTileLayer handles DataChanged now with: Clear Cache After SourceLayer changes #2106
Conversation
inforithmics
changed the title
… Cache was cleared, reduces flickering
|
Things done:
|
There was a problem hiding this comment.
Thanks for the improvements!
I think there is still a threading problem with the ObservableCollection not being thread save, even with the ToArray. This, may improve it but some of our users will still run into it.
Some general remarks about the ObservableCollection
I've had a lot of trouble with the ObservableCollection in the past. If you change a lot of items at the same time they will all trigger notifications while for most scenarios one notification would suffice, causing performance problems. Also these problems are often hard to track because of the distributed nature of the notification. I prefer a more centralized control for a map component.
A more general argument against the ObservableCollection. It is a pattern that was introduced for lists in users interfaces which are usually small in number and don't change a lot. In Mapsui two map drags can replace all items in the map. This makes it less suitable for the ObservableCollection. Since it is a lot of data changing fast it should take its design patters from game engines.
| @@ -35,7 +46,7 @@ public class ObservableMemoryLayer<T> : MemoryLayer | |||
| { | |||
| _observableCollection.CollectionChanged += DataSource_CollectionChanged; | |||
| _shadowCollection.Clear(); | |||
| foreach (var it in _observableCollection) | |||
| foreach (var it in _observableCollection.ToArray()) // collection has been changed. | |||
There was a problem hiding this comment.
If a 'collection has been changed' can happen while iterating over the items, it can also happen within the ToArray method, because to create the array you need to iterate over the items. Perhaps it is less likely if the ToArray takes less time.
There was a problem hiding this comment.
Maybe we need a ThreadSafeObservableCollection. or ConcurrentObservableCollection
There was a problem hiding this comment.
I would like to have a better understanding of the use case for the ObservableCollection.
|
|
||
| namespace Mapsui; | ||
|
|
||
| public class MPoint |
There was a problem hiding this comment.
Adding the IEquatable makes sense.
| public class RasterizingTileLayer : TileLayer, ISourceLayer, IAsyncDataFetcher | ||
| { | ||
| private MRect? _currentExtent; |
There was a problem hiding this comment.
You found a 'little excessive' 😁 tile fetching problem, which could be explained because the extent and resolutions were changed in two steps (immediately following each other) but on another thread a tiles fetch was triggered after the extent was set but before the resolution was set. In order to avoid that I introduces MSection (I could not think of a better name yet). It is immutable and extent and resolution will always be an atomic thing.
| { | ||
| ClearCache(); | ||
| DataHasChanged(); | ||
| if (_currentExtent != null && _currentResolution != null) |
There was a problem hiding this comment.
hmm, here the extent and resolution are used which are set in the GetFeatures request. There is no better solution in the current architecture, but it feels a bit like a workaround.
Refresh cache after Datachanges in Source Layer