[Enhancement] Shapes (Path) #9264
Conversation
samhouts
added
p/Tizen
p/gtk
p/WPF
a/Xaml </>
a/visual
t/enhancement ➕
proposal-open
labels
Implemented Path in Android and iOS
There was a problem hiding this comment.
- Need to fix namespaces to Xamarin.Forms.Shapes
- I'm not sure how else we would implement the Clip, but I do know that we try to avoid overriding
Drawand its platform equivalents because it can cause performance degradation and potentially wipe out any custom effects that customers may have implemented.
Otherwise, looks great!
| @@ -219,6 +220,13 @@ protected override void OnLayout(bool changed, int left, int top, int right, int | |||
| _hasLayoutOccurred = true; | |||
| } | |||
|
|
|||
| public override void Draw(Canvas canvas) | |||
There was a problem hiding this comment.
Do we have a measure for how this is going to affect performance? (same question for all new override Draw instances)
There was a problem hiding this comment.
I added a new sample in Core Gallery.
The sample load hundreds of images first and then the same images using EllipseGeometry to Clip it.
The results from iOS are the following (Debug Mode):
- 100 images: 18.82ms slower using Clip.
- 1000 images: 212.87ms slower using Clip.
On Android, the differences are lower (practically same times). I will take more measurements in addition to sharing data from other platforms.
There was a problem hiding this comment.
I need to think about Custom Effects too, it can have an impact.
| @@ -0,0 +1,140 @@ | |||
| namespace Xamarin.Forms.Shapes | |||
| { | |||
| public sealed class CompositeTransform : Transform | |||
There was a problem hiding this comment.
can't we reuse the Transform we already have, that's also hardware accelerated in some cases ?
|
|
||
| void OnTransformPropertyChanged() | ||
| { | ||
| TransformGroup xformGroup = new TransformGroup |
There was a problem hiding this comment.
shouldn't this be a pre-computed MatrixTransform ?
| BindableProperty.Create(nameof(RadiusX), typeof(double), typeof(EllipseGeometry), 0.0); | ||
|
|
||
| public static readonly BindableProperty RadiusYProperty = | ||
| BindableProperty.Create(nameof(RadiusY), typeof(double), typeof(EllipseGeometry), 0.0); |
There was a problem hiding this comment.
I prefer the Geometry API over the Shape ones. can't we unify ?
There was a problem hiding this comment.
Yes, we can review and unify. Right now the Spec is based on the WPF API (allow porting code, etc). For this reason, Line has X1, X2, Y1, Y2 properties: https://docs.microsoft.com/en-us/dotnet/api/system.windows.shapes.line?view=netcore-3.1 while LineGeometry has two points: https://docs.microsoft.com/en-us/dotnet/api/system.windows.media.linegeometry?view=netcore-3.1
In this case, personally I like more to have the two points.
| public static readonly BindableProperty EndPointProperty = | ||
| BindableProperty.Create(nameof(EndPoint), typeof(Point), typeof(LineGeometry), new Point()); | ||
|
|
||
| public Point StartPoint |
There was a problem hiding this comment.
that's even nicer than X!,X2
Description of Change
Shapes support is added:
This PR is a continuation of #9218
Issues Resolved
fixes (partially) #2452
fixes #9178
API Changes
Added:
Segment:
Geometry:
Path Transformations:
Platforms Affected
Behavioral/Visual Changes
None
Screenshots
Path Transformations Playground:
(Clip Views)Testing Procedure
Launch Core Gallery and navigate to the new Shapes Gallery.
PR Checklist