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

Fix perspective transform usage #598

Merged
merged 5 commits into from
Jun 7, 2023
Merged

Fix perspective transform usage #598

merged 5 commits into from
Jun 7, 2023

Conversation

MatkovIvan
Copy link
Member

@MatkovIvan MatkovIvan commented Jun 5, 2023 •
edited
Loading

Proposed Changes

  • matrix[2, 2] should not be zeroed, and perspective transform should be applied only in case of rotation.
  • Third column and row are irrelevant for 2D space. Zeroing required to get correct inverse transformation matrix.
  • Test that applying inverseMatrix do opposite than applying matrix
  • Add graphicsLayer settings page to mpp
Screenshot 2023-06-07 at 13 22 51

Testing

Test: run code from JetBrains/compose-multiplatform#3230 (comment)
Unit tests: run tests from SkiaLayerTest

Issues Fixed

Fixes JetBrains/compose-multiplatform#3230

@MatkovIvan MatkovIvan requested a review from eymar June 5, 2023 18:41
@MatkovIvan MatkovIvan marked this pull request as ready for review June 5, 2023 18:45
@MatkovIvan MatkovIvan mentioned this pull request Jun 5, 2023
Closed
@eymar
Copy link
Member

eymar commented Jun 6, 2023

Just some thoughts about a test:
A test would apply a graphicsLayer + apply the scale and click on a Box at a position beyond the original box rectangle. It would expect that the click is received.
So it simply mimics the reproducer of JetBrains/compose-multiplatform#3230

PointerIconTest - is an example with pointer input in tests.

eymar
eymar approved these changes Jun 6, 2023
View reviewed changes
Copy link
Member

@eymar eymar left a comment

Choose a reason for hiding this comment

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

If a test requires an extra look, please let me know

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaLayer.skiko.kt Show resolved Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaLayer.skiko.kt Show resolved Hide resolved
@MatkovIvan MatkovIvan requested review from eymar and igordmn June 7, 2023 11:25
@MatkovIvan MatkovIvan force-pushed the ivan.matkov/fix-input-transform branch from ea76bb6 to 1f63516 Compare June 7, 2023 11:54
@MatkovIvan MatkovIvan force-pushed the ivan.matkov/fix-input-transform branch from 1f63516 to 5586d14 Compare June 7, 2023 12:19
@eymar

This comment was marked as resolved.

Sign in to view
@MatkovIvan
Copy link
Member Author

It fails in jb-main branch, but It passes in your branch.

I already added tests that fails on main branch, but let's add extra one, sure

@MatkovIvan MatkovIvan merged commit b09f611 into jb-main Jun 7, 2023
@MatkovIvan MatkovIvan deleted the ivan.matkov/fix-input-transform branch June 7, 2023 13:51
@MatkovIvan
Copy link
Member Author

BTW I guess it fixes JetBrains/compose-multiplatform#1718 too

@MatkovIvan MatkovIvan mentioned this pull request Jun 7, 2023
Closed
MatkovIvan added a commit that referenced this pull request Jun 8, 2023
@MatkovIvan
Fix perspective transform usage (#598)
0151fd3 
* Fix perspective transform usage

* Add graphicsLayer settings page to mpp

* Add inverseMatrix.map tests

* Fix inverseMatrix for rotated layer
# Conflicts:
#	compose/mpp/demo/src/commonMain/kotlin/androidx/compose/mpp/demo/App.kt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eymar eymar eymar approved these changes

@igordmn igordmn Awaiting requested review from igordmn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[JVM] graphicsLayer modifier causes problems with pointer input on Desktop
2 participants
@MatkovIvan @eymar