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

[Bug] Path rendering crashing on GeometryExtensions due to unexpected PathSegment configuration #11653

Closed
MitchBomcanhao opened this issue Aug 4, 2020 · 4 comments
Assignees
Labels
a/shapes e/2 🕑 2 in-progress This issue has an associated pull request that may resolve it! t/bug 🐛
Milestone

Comments

@MitchBomcanhao
Copy link

Description

It appears to be possible to make a Path that has a segment with a number of points that will cause the app to crash.

example on Android: If the PolyBezierSegment has a small enough number of points, it will fail.

// PolyBezierSegment
else if (pathSegment is PolyBezierSegment)
{
    PolyBezierSegment polyBezierSegment = pathSegment as PolyBezierSegment;
    PointCollection points = polyBezierSegment.Points;

    for (int i = 0; i < points.Count; i += 3)
    {
        path.CubicTo(
            density * (float)points[i + 0].X, density * (float)points[i + 0].Y,
            density * (float)points[i + 1].X, density * (float)points[i + 1].Y,
            density * (float)points[i + 2].X, density * (float)points[i + 2].Y);
    }

    lastPoint = points[points.Count - 1];
}

There are other types of segments that will have similar failures.
This issue may depend per platform as each one has a slightly different implementation for each segment type.

Steps to Reproduce

  1. Create a Path with something like this
<Path Aspect="Uniform">
    <Path.Data>
        <PathGeometry>
            <PathGeometry.Figures>
                <PathFigureCollection>
                    <PathFigure IsClosed="True" StartPoint="10,100">
                        <PathFigure.Segments>
                            <PathSegmentCollection>
                                <PolyBezierSegment Points="100,100" />
                            </PathSegmentCollection>
                        </PathFigure.Segments>
                    </PathFigure>
                </PathFigureCollection>
            </PathGeometry.Figures>
        </PathGeometry>
    </Path.Data>
</Path>

Expected Behavior

Not sure, should it be blank because the path is not as expected?

Actual Behavior

App crashes with out of range exception

Basic Information

  • Version with issue: 4.7.0.1239
  • Last known good version: n/a
@MitchBomcanhao MitchBomcanhao added s/unverified New report that has yet to be verified t/bug 🐛 labels Aug 4, 2020
@samhouts samhouts added this to New in Triage Aug 4, 2020
@PureWeen
Copy link
Contributor

PureWeen commented Aug 4, 2020

@jsuarezruiz FYI

@jsuarezruiz jsuarezruiz removed the s/unverified New report that has yet to be verified label Aug 21, 2020
@jsuarezruiz jsuarezruiz moved this from New to Ready For Work in Triage Aug 21, 2020
@jsuarezruiz
Copy link
Contributor

@MitchBomcanhao Thanks for the feedback. I have created a PR with the fix here: #11873

About doubt if will/should render something or not, nothing will be rendered with that example. In other frameworks (WPF, UWP) the behavior would be the same.

Captura de pantalla 2020-08-21 a las 13 59 11

@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Aug 21, 2020
@samhouts samhouts added this to In Progress in vCurrent (4.8.0) Aug 21, 2020
@samhouts samhouts added this to To do in Other Ready For Work Aug 21, 2020
@samhouts samhouts removed this from Ready For Work in Triage Aug 21, 2020
@samhouts samhouts moved this from To do to In progress in Other Ready For Work Aug 21, 2020
@MitchBomcanhao
Copy link
Author

@MitchBomcanhao Thanks for the feedback. I have created a PR with the fix here: #11873
About doubt if will/should render something or not, nothing will be rendered with that example. In other frameworks (WPF, UWP) the behavior would be the same.

Nice - I did prevent the problem that I was having by ensuring that my code stopped trying to create a path with potentially an invalid point count, but this fix is most welcome.

rmarinho added a commit that referenced this issue Aug 25, 2020
* Updated Shapes Segments unit tests to add more cases

* Fixed the issue

Co-authored-by: Rui Marinho <me@ruimarinho.net>
@samhouts samhouts moved this from In Progress to Done in vCurrent (4.8.0) Aug 25, 2020
@samhouts samhouts added this to the 5.0.0 milestone Aug 25, 2020
@samhouts
Copy link
Member

closed by #11873

Other Ready For Work automation moved this from In progress to Done Aug 26, 2020
@samhouts samhouts removed this from Done in Other Ready For Work Nov 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/shapes e/2 🕑 2 in-progress This issue has an associated pull request that may resolve it! t/bug 🐛
Projects
No open projects
Development

No branches or pull requests

4 participants