Skip to content

Conversation

@Lehonti
Copy link
Contributor

@Lehonti Lehonti commented Sep 8, 2025

For now it's <= 100, but later on we could try to allow percentages beyond 100 and/or moving the center of rotation by using CairoExtensions.GetBilinearSampleClamped, CairoExtensions.GetBilinearSampleWrapped and so on

@cameronwhite
Copy link
Member

Going beyond 100% seems reasonable to me if you want to expand the size of the twist effect more

I'm not sure if I'm clear why that implies that bilinear sampling is necessary (although maybe we should be doing that regardless, especially since there are sub pixel samples for antialiasing)
Is that more needed just for the clamping of locations outside the image bounds?

@Lehonti
Copy link
Contributor Author

Lehonti commented Sep 10, 2025

@cameronwhite, yes, I am referring to clamping (or reflecting/wrapping/etc.) the locations outside the image bounds.

But there is a caveat when it comes to bilinear sampling: the methods in CairoExtensions.Samples.cs only accept float coordinates, but the calculations inside TwistEffect are in double. We could:

  • Wait until the methods have a double version
  • Convert the coordinates to float now, and then remove the conversion when the methods have a double version

@cameronwhite
Copy link
Member

I'd lean towards just switching those methods over to double entirely - it's odd having them as float when PointD is our standard type for non-integer coordinates

That can be a separate PR in the future, though, so this change seems good to merge

@cameronwhite cameronwhite merged commit e3463c7 into PintaProject:master Sep 12, 2025
7 checks passed
@Lehonti Lehonti deleted the feature/twist_radius_percentage branch September 12, 2025 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants