Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WinUI] Optimize TransformationExtensions #22481

Merged
merged 1 commit into from
May 23, 2024
Merged

[WinUI] Optimize TransformationExtensions #22481

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/Core/src/Platform/Windows/TransformationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ public static class TransformationExtensions
{
public static void UpdateTransformation(this FrameworkElement frameworkElement, IView view)
{
double anchorX = view.AnchorX;
double anchorY = view.AnchorY;
double rotationX = view.RotationX;
double rotationY = view.RotationY;
double rotation = view.Rotation;
Expand All @@ -18,9 +16,6 @@ public static void UpdateTransformation(this FrameworkElement frameworkElement,
double scaleX = view.Scale * view.ScaleX;
double scaleY = view.Scale * view.ScaleY;

frameworkElement.RenderTransformOrigin = new global::Windows.Foundation.Point(anchorX, anchorY);
frameworkElement.RenderTransform = new ScaleTransform { ScaleX = scaleX, ScaleY = scaleY };
Comment on lines -21 to -22
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically this is set to be unset on L27 and L28.

Additionaly, my best guess is that unsetting RenderTransformOrigin is not necessary if RenderTransform is not null.


if (rotationX % 360 == 0 && rotationY % 360 == 0 && rotation % 360 == 0 &&
translationX == 0 && translationY == 0 && scaleX == 1 && scaleY == 1)
Comment on lines 19 to 20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are all default, this if should be true.

But are there cases were we need to round? What if someone did some math in their code and rotationX is 359.99999999?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

I'm not too sure what conventions MAUI or graphics libraries have in this regard. Naively I would say that 359.999999 is not really a big issue because it probably happens quite rarely and function-wise the result will be acceptable (yet slower in performance).

If you have a specific suggestion, please suggest it here. Otherwise, I would like to avoid mixing multiple things in one PR (because it can add months to get this merged). This PR is mostly meant for elements that does no transformations at all (my use case) and I believe it's very very common.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe someone like @PureWeen or @mattleibow know of an example where we round on other platforms.

I imagine an extension method for double like: IsNearly(0, translationX) or IsNearly(1, scaleX) if we already do this somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine an extension method for double like: IsNearly(0, translationX) or IsNearly(1, scaleX) if we already do this somewhere.

Sure, I understand the idea. I'm just not sure how exactly to implement it so that it can be considered "always correct" (like how many decimals, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed #22513 for this.

{
Expand All @@ -29,6 +24,12 @@ public static void UpdateTransformation(this FrameworkElement frameworkElement,
}
else
{
double anchorX = view.AnchorX;
double anchorY = view.AnchorY;

frameworkElement.RenderTransformOrigin = new global::Windows.Foundation.Point(anchorX, anchorY);
frameworkElement.RenderTransform = new ScaleTransform { ScaleX = scaleX, ScaleY = scaleY };
MartyIX marked this conversation as resolved.
Show resolved Hide resolved

// PlaneProjection removes touch and scrollwheel functionality on scrollable views such
// as ScrollView, ListView, and TableView. If neither RotationX or RotationY are set
// (i.e. their absolute value is 0), a CompositeTransform is instead used to allow for
Expand Down
Loading