Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Detect Points changes in Xamarin.Forms.Shapes.Polyline #11848

Closed
vsfeedback opened this issue Aug 19, 2020 · 0 comments · Fixed by #11579
Closed

Detect Points changes in Xamarin.Forms.Shapes.Polyline #11848

vsfeedback opened this issue Aug 19, 2020 · 0 comments · Fixed by #11579
Assignees
Labels
a/brushes blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. e/2 🕑 2 in-progress This issue has an associated pull request that may resolve it! t/bug 🐛
Milestone

Comments

@vsfeedback
Copy link

This issue has been moved from a ticket on Developer Community.


[severity:I'm frustrated, but able to complete my work]
Xamarin.Forms.Shapes.Polyline.Points doesn't respond to CollectionChanged events.
I bound the property Points of Polyline to my own PointCollection and when changing the points in the PointCollection the UI does not update the drawing with the new points. Even recreating a new PointCollection does not change the UI.
When I change the Xaml to use a Polygon instead of a Polyline everything works fine in Xamarin Forms 4.8.
This issue has been fixed in Xamarin Forms 4.8 for Polygon according to GitHub, and seems to work.
However, the issue still exist for Polyline. I even tried Xamarin Forms 5.0.1.1276-nightly and the issue is still there.
This is what I saw on GitHub
PolygonRenderer.cs Detect Points changes in Polygon (#11576) 12 days ago
PolylineRenderer.cs [Enhancement] Shapes (#9218) 2 months ago

Is there a plan to fix it for Polyline and when?


Original Comments

Denis Ruffo on 8/10/2020, 08:33 PM:

Polygon does update, however it would crash depending when the Point collection is updated.

because the PolygoneRenderer call for each platform (UWP, iOS, Android) the Method UpdatePoints without checking if Element is null. It is actually null for a while until not.

void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)    
{   UpdatePoints(); }

For example in ios
void UpdatePoints()
{
Control.UpdatePoints(Element.Points.ToCGPoints());
}
Should be replaced with
void UpdatePoints()
{
if (Element != null && Element.Points!=null)
{
Control.UpdatePoints(Element.Points.ToCGPoints());
}
}

There is a change for PolyLine that has not been merged into Xamarin Forms 5.0
at https://github.com/xamarin/Xamarin.Forms/blob/polyline-points-changes/Xamarin.Forms.Platform.UAP/Shapes/PolylineRenderer.cs
and
Xamarin.Forms/Xamarin.Forms.Platform.Android/Shapes/PolylineRenderer.cs
and
Xamarin.Forms/Xamarin.Forms.Platform.iOS/Shapes/PolygonRenderer.cs

I created my own render based on this code and it seems to work, but also it suffers the same bug as PolylgoneRenderer in UpdatePoints. The code should check for Element not being null
void UpdatePoints()
{
if (Element != null && Element.Points!=null)
{
Control.UpdatePoints(Element.Points.ToCGPoints());
}
}

Feedback Bot on 8/11/2020, 01:47 AM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.

Denis Ruffo on 8/11/2020, 02:29 PM:

In addition the previous issue.

if I have the following in my XAML

<Polyline 
 Points="{Binding DataPoints}"
 Stroke="Red" 
 StrokeDashArray="1,1"
 StrokeDashOffset="6"
 VerticalOptions="FillAndExpand"  
 StrokeThickness="2" />

The height of the display is scaled to the the biggest height when DataPoints was first created so if DataPoints has values with small amplitude and was originally initialized to small values like below

private PointCollection _dataPoints = new PointCollection();

for (int x = 0; x < 100; x++)

{ DataPoints.Add(new Point(x, 0)); }

and then later I have DataPoints with Amplitude of 100, none of the data point would plot.

for (int x = 0; x < 100; x++)

{ DataPoints.Add(new Point(x, 100)); }

The only way I was able to fix it was to initialize DataPoints with the biggest amplitude possible


Original Solutions

(no solutions)

@samhouts samhouts added this to New in Triage Aug 19, 2020
@jsuarezruiz jsuarezruiz added a/brushes t/bug 🐛 s/unverified New report that has yet to be verified labels Aug 19, 2020
@jsuarezruiz jsuarezruiz added e/2 🕑 2 and removed s/unverified New report that has yet to be verified labels Aug 19, 2020
@jsuarezruiz jsuarezruiz moved this from New to Ready For Work in Triage Aug 19, 2020
@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Aug 19, 2020
@samhouts samhouts added this to In Progress in vNext+1 (5.0.0) Aug 19, 2020
@samhouts samhouts added this to the 5.0.0 milestone Aug 19, 2020
@samhouts samhouts added this to To do in Other Ready For Work Aug 26, 2020
@samhouts samhouts removed this from Ready For Work in Triage Aug 26, 2020
@samhouts samhouts moved this from To do to In progress in Other Ready For Work Aug 26, 2020
@samhouts samhouts removed this from the 5.0.0 milestone Nov 2, 2020
@PureWeen PureWeen added this to the 5.0.1 milestone Nov 5, 2020
@PureWeen PureWeen added this to To do in v5.0.1 via automation Nov 5, 2020
@PureWeen PureWeen removed this from In Progress in vNext+1 (5.0.0) Nov 5, 2020
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 5, 2020
@PureWeen PureWeen modified the milestones: 5.0.1, 5.0.0 Nov 5, 2020
@PureWeen PureWeen added this to To do in vNext+1 (5.0.0) via automation Nov 5, 2020
@PureWeen PureWeen removed this from To do in v5.0.1 Nov 5, 2020
@rmarinho rmarinho moved this from To do to In Progress in vNext+1 (5.0.0) Nov 13, 2020
@jsuarezruiz jsuarezruiz self-assigned this Nov 24, 2020
Other Ready For Work automation moved this from In progress to Done Nov 27, 2020
vNext+1 (5.0.0) automation moved this from In Progress to Done Nov 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/brushes blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. e/2 🕑 2 in-progress This issue has an associated pull request that may resolve it! t/bug 🐛
Projects
Development

Successfully merging a pull request may close this issue.

4 participants