-
-
Notifications
You must be signed in to change notification settings - Fork 148
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.
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
Feature/#66 pan while zoom #68
Conversation
use Float.NaN instead of float comparison using epsilon always animate when overscroll/overpinch is enabled since newZoom will always be > 0
…Pan method (still a bug when animating back from an overpinched state)
added fields for initial and current target of pinch gesture improved zoom target coordinate selection based on which side of the view has been overscrolled during pinch gesture only fix overscroll and/or overpinch if necessary parameter renaming some code cleanup
extracted pivot point calculation to method
slight optimization
extracted viewCoordinateToAbsolutePan to method fixed initial jump when zooming in fast fixed strange wobble when zooming in and out fast
…of X and Y values at the same time added simple JUnit5 test added helper/extension functions
…ll be overscrolled due to overpinch adjustments (doesn't jump on corners)
added unaryMinus operator replaced resolvePan and unresolvePan methods with kotlin extension functions added separate tests for absolutePoint and ScaledPoint
use Number as input parameter for operators so Int and Float can be used make more use of AbsolutePoint instead of two variables
… from underpinch (zoom value lower than minZoom)
make more use of ScaledPoint instead of two variables
more ScaledPan usage
…more so a variable for those dependencies doesn't make sense added named parameters to many boolean parameters (to make easier to understand without IDE support like on GitHub)
replaced resolveZoom method with Float extension function
#61 renamed function to match its usecase use androids ValueAnimator for animations instead of pure Runnables and manual interpolation
@@ -10,6 +10,8 @@ android { | |||
targetSdkVersion rootProject.ext.targetSdkVersion | |||
versionCode 1 | |||
versionName "1.0" | |||
|
|||
setProperty("archivesBaseName", "ZoomLayout_v${versionName}_(${versionCode})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When building an apk (release or debug) the resulting apk file will be called this way.
I have too much app-debug.apk
files in my life already 😛
* The current pan as an [AbsolutePoint] | ||
*/ | ||
private val pan: AbsolutePoint | ||
get() = AbsolutePoint(panX, panY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these pan and scaledPan values accessed frequently?
If they are it might be worth to avoid all these new instances by caching the value (have a private _pan and use _pan.set() in this getter), since touch events are a lot.
By the way I thought from another comment that these two were public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made them public now, don't know why they weren't.
Yes using the same Object is probably better for performance - I changed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know you could do what you did :-) just one last change, can they be vals of ZoomApi overriden in the engine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well the pan
val can, the scaledPan
depends on mTransformedRect
so I don't think so?
removed unneeded post() calls minimized constuctor calls on pan/scaledPan/mCurrentPanCorrection removed unused method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good IMO! It was a great addition, thanks.
…le_zoom # Conflicts: # library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt
Since you already approved this PR and I only fixed a small dev-merge mistake I will merge this PR now. |
fixes #66
Wow so this took quite a while for me to get it to work but I'm quite happy with the results.
The hardest part was animating back to a non-overscrolled and non-overpinched state at the same time.
I use a "simulated" zoom state to calculate the pan fixes needed to be applied to get back into a valid state when the zoom is also changing. I will add a review comment to the mentioned line, please let me know if you know a better way to do this.