Skip to content

Conversation

gn-dave-a
Copy link
Collaborator

@gn-dave-a gn-dave-a commented Oct 3, 2025

The matrix concat operation was doing B . A for A.concatenating(B) which was causing issues when combining transformations.

@gn-dave-a gn-dave-a force-pushed the dave/fix-polyfill-matrix-concat branch from 7864239 to de77dba Compare October 3, 2025 11:13
@gn-dave-a gn-dave-a requested a review from khoi October 3, 2025 11:16
@gn-dave-a gn-dave-a force-pushed the dave/fix-polyfill-matrix-concat branch from e0baaf2 to 9095f21 Compare October 3, 2025 11:32
@khoi
Copy link
Collaborator

khoi commented Oct 3, 2025

AffineConcatComparison.zip

Can u take a look at this test project and the unit tests in there.

  • concatenating_org vs concatenating_new (the logic in this PR)

It compares the 2 implementations with the original Apple's CoreGraphics one. and it seems like the new logic is failing?

@gn-dave-a gn-dave-a force-pushed the dave/fix-polyfill-matrix-concat branch from 9095f21 to 17a11e3 Compare October 3, 2025 12:13
@gn-dave-a gn-dave-a requested a review from khoi October 3, 2025 12:13
@gn-dave-a
Copy link
Collaborator Author

Had removed that comment from the wrong file - sorted now.

@gn-dave-a
Copy link
Collaborator Author

AffineConcatComparison.zip

Can u take a look at this test project and the unit tests in there.

* concatenating_org vs concatenating_new (the logic in this PR)

It compares the 2 implementations with the original Apple's CoreGraphics one. and it seems like the new logic is failing?

@khoi are you sure the tests are right? Why do any of them check for a delta bigger than the tolerance given the use of abs? If you swap all the assertions to check for an abs delta < the tolerance, the new implemenation passes and the original one fails.

@gn-dave-a gn-dave-a force-pushed the dave/fix-polyfill-matrix-concat branch from 17a11e3 to 886701b Compare October 3, 2025 12:48
@khoi
Copy link
Collaborator

khoi commented Oct 3, 2025

oops, you're right. Verified the changes and it looks correct now. Thanks for fixing

@khoi khoi merged commit b381dfb into main Oct 3, 2025
2 checks passed
@khoi khoi deleted the dave/fix-polyfill-matrix-concat branch October 3, 2025 12:50
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