Polygon.Points Binding TwoWay
So, I have a class that has a PointCollection, that is used to store some polygon geometry. I have a DataTemplate that I am using to display the polygon data, and am databinding its Points collection to my object's Pointcollection. This works well for displaying the polygon the first time, but if I change any of the points in the object's PointCollection, then the polygon is not updated with the new shape information. The weird part, is that if I just set the Points value of the polygon to the object's points property, it works as expected. I was able to create a custom shape to get around this problem, but I was wondering if this is a bug. Here are the important bits. By the way, I am running RC1.
<DataTemplate DataType="{x:Type data:PolygonItem}">
<Polygon x:Name="shape"
StrokeThickness="1.5"
Fill="Blue"
Stroke="Blue"
Points="{Binding Path=Points, Mode=TwoWay}" />
</DataTemplate>
namespace Data
{
public class PolygonItem
{
private PointCollection _points = new PointCollection();
/// <summary>
/// Gets the list of points that make up the polygon
/// </summary>
public PointCollection Points
{
get { return _points; }
set
{
if (value != _points)
{
_points = value;
}
}
}
}
}
Does this work if you use a DependencyProperty for PolygonItem.Points rather than just a CLR property?
-Adam Smith [MS]
Actually, that does work. However, I was under the impression that I could implement INotifyPropertyChanged, and that would take care of the bindings. I tried to have the PolygonItem implement INotifyPropertyChanged, but the same behavior occurred, which is why I didn't include the interface in my example. So, the following code doesn't work either, but I was wondering why the DependencyProperty will work, but not the INotifyPropertyChanged version.
namespace Data
{
public class PolygonItem : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged(string propName)
{
if (this.PropertyChanged != null)
this.PropertyChanged(this, new PropertyChangedEventArgs(propName));
}
private PointCollection _points = null;
/// <summary>
/// Gets the list of points that make up the polygon
/// </summary>
public PointCollection Points
{
get
{
if (_points == null)
Points = new PointCollection();
return _points;
}
protected set
{
if (value != _points)
{
_points = value;
if(_points != null)
_points.Changed += new EventHandler(_points_Changed);
OnPropertyChanged("Points");
}
}
}
void _points_Changed(object sender, EventArgs e)
{
OnPropertyChanged("Points");
}
}
}
Hi Abraham,
In your solution, you are notifying that the Points property changed in its setter. This means that if the collection is replaced with a completely different one, your application will be notified of that fact. However, if you simply add or delete an item, the UI is not getting notified. This is why your solution with INotifyPropertyChanged doesn't work.
DependencyProperties are smart when it comes to change notifications. Typically, I would advise against using DPs in your data source because it leads to coupling between Avalon and data. Ideally, all data sources should be completely independent on Avalon. This will pay off when you're porting your application to the next UI framework technology, because you won't have to worry about how your data source is implemented, you will only have to port the UI. However, in your case, you already have that coupling since PointCollection is an Avalon collection. For this reason, if you are planning to keep the source this way, having a DP is a good solution (it's not adding any dependencies that you don't already have).
If you want to achieve complete separation of data and UI, you could store your collection of points in some other collection type that is independent on Avalon. Then you could add a converter to your Binding that is able to convert this independent collection into a PointCollection. Here is a converter that does that:
public class Converter : IValueConverter
{
public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
{
ReadOnlyCollection<MyPoint> collection = value as ReadOnlyCollection<MyPoint>;
PointCollection points = new PointCollection();
foreach (MyPoint myPoint in collection)
{
points.Add(new Point(myPoint.X, myPoint.Y));
}
return points;
}
public object ConvertBack(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
{
throw new NotSupportedException("ConvertBack should never be called");
}
}
The one thing to keep in mind is that you need to raise property change notifications when you make changes to the collection. I added support for this by adding an Add method to PolygonItem that raises a property change notification and adds the item to the Points collection. I also made Points of type ReadOnlyCollection<MyPoint>, to prevent users of this class to add items directly to Points, and bypass the change notification.
public class PolygonItem : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged(string propertyName)
{
if (this.PropertyChanged != null)
{
this.PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
}
}
private List<MyPoint> _points = new List<MyPoint>();
public ReadOnlyCollection<MyPoint> Points
{
get { return new ReadOnlyCollection<MyPoint>(_points); }
set
{
_points = new List<MyPoint>(value);
OnPropertyChanged("Points");
}
}
public void Add(MyPoint point)
{
this._points.Add(point);
OnPropertyChanged("Points");
}
public PolygonItem()
{
_points.Add(new MyPoint(100, 50));
_points.Add(new MyPoint(50, 100));
_points.Add(new MyPoint(150, 50));
}
}
You can find a complete solution with this code here: http://www.beacosta.com/Forum/PolygonBinding.zip.
I suspect this will be a fairly common scenario, so let me know if my explanation is clear. I will be happy to reply to any follow up questions on this.
Thanks,
Bea
Thanks for your reply, Beatriz. I agree that your solution is probably more future proof, and I will certainly consider the benefits of doing so. However, one thing still doesn't seem to be working right. With the code example I gave, I raise the property change notification when the points collection changes. Here is the code I am speaking of:
public PointCollection Points
{
get
{
if (_points == null)
Points = new PointCollection();
return _points;
}
set
{
if (value != _points)
{
_points = value;
if(_points != null)
_points.Changed += new EventHandler(_points_Changed);
OnPropertyChanged("Points");
}
}
}
void _points_Changed(object sender, EventArgs e)
{
OnPropertyChanged("Points");
}You can see that I raise my event (in the OnPropertyChanged function), when the Points collection changes. So, I am still unsure as to why the Polygon on screen doesn't update when I raise the change event.
While this isn't a show stopper, and I can use other collections, I guess this isn't all that important. However, it does seem to me that it should still work if I am telling the framework that the property has changed. My guess is that it is just checking to see if the instance of the collection has changed, not the actual data in the collection.
Thanks again for all your help!
Hi Abraham,
You are completely right, I overlooked the line where you hook up the handler for the Changed event.
I think your assumption of why this doesn't work is correct. Here is my guess of what is going on:
- We explicitly raise the PropertyChanged notification when an item is added.
- The Binding listens to this notification and tells the property system to set the new value.
- The property system realizes that the reference for this new value is the same as the existing one and ignores it. I am not an expert in the property system, but my understanding is that this behavior is intended.
I was also thinking more about the alternative solution I showed you. It's a good solution if you're making big changes in the collection of points, but it is not very efficient if you're only adding or deleting one item at a time. Any solution that relies on property change notifications will have to replace the current collection with a new one everytime a change happens. My solution doesn't run into the same issue as yours because I am creating a new collection in the converter, every time a change happens, so the reference is different. If you decide to use my solution, it's best to batch changes to the collection and send one property change notification for a group of changes.
Do you need data binding for you scenario? Freezables support change notifications, so if you change the PointCollection directly, without data binding, it should work. This would be the most efficient way of propagating changes to the UI in this scenario. I am still thinking of ways to make this scenario work with data binding such that 1) Collection change notifications are propagated 2) It's efficient if you change just on item 3) There is a clean separation between data and UI. If I come up with something, I will post it in my blog and reply to this thread with the link.
Let me know if you have any other questions.
Thanks,
Bea
The problem with the original poster's code is that he uses PointCollection in his PolygonItem class, but PointCollection doesn't implement INotifyCollectionChanged, actually you can use ObservableCollection<Point> instead, and the two way data binding can still work here, or alternatively you can create your custom PointCollection which implements all those observer logic, but that will take more work to do.
Sheva
While changing the PointsCollection to an ObservableCollection<Point> and handling change notifications from the collection and raising the PropertyChanged event will work correctly, I don't think it is working because of the ObservableCollection. On the contrary, this works because it has to use a converter, which then copies all the items in the original collection into a new PointCollection, which is then given to the Polygon to draw. The same behavior can be achieved with any collection, if you raise the PropertyChanged event when the collection is altered. Since that is the case, the ObservableCollection<Point> isn't the solution I am looking for.
Beatriz - Since this is intended behavior for the Polygon item, I have implemented my own Polygon shape that handles these change notifications from the PointCollection. It was not too difficult to implement, and I implemented the public face of the original Polygon. But since Polygon is sealed, I had to start from the Shape class. This is probably not the best way of implementing this scenario, but I was able to learn how to create a custom Shape.
As far as needing DataBinding, I am pretty sure that it is required, since I am defining a DataTemplate to display my items inside of a ListView. My ListView has a collection of items that are many different classes, each with their own DataTemplate, so I do not know of a way to get rid of the bindings.
Thanks again for all of your help!
Hey Abraham, the ObservableCollection works here is not because he "sends" a new instance of itself to the WPF's property system, what he actually does is to send CollectionChanged event to the WPF's property sytem, when the WPF's property system receives this message, he will update the property which is data bound with the ObservableCollection, so this is what ObserableCollection is designed for, it implements the collection changed notifcation. I still fails to understand why ObservableCollection doesn't fit into your need.
Sheva
Hi Sheva,
Thanks for your message. What you explained would be true if the target of the binding was an ItemsControl, but that is not the case here. The item generator from ItemsControl knows how to listen to collection change notifications. Since there is no ItemsControl involved in this scenario, raising collection change notifications does not make a difference. Even though there is a collection involved here, we are not using the item generator to expand it, which makes this scenario somewhat unique.
For this reason, I actually advise against using an ObservableCollection<Point> as the source collection to avoid adding unnecessary complexity.
Thank you,
Bea
Hi, Bea, you are right, I think I just shoot the gun a bit early without carefully examining the issue here, actually PointCollection and ObserableCollection are different collection type, we should still use Converter, since when we Convert the collection, a new collection will be generated and assigned to the PointCollection DP, then every kinda collection can work here
Sheva
Sheva, you got it!
Abraham, your solution sounds like it should work well. I'm sorry it's so hard for you to work around this. The good news is that this scenario was added to our V2 discussions, so we may be able to do something about it for V2.
Excellent! Thanks again for all of your help.
