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.

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

Merged
merged 40 commits into from
Jan 8, 2019
Merged

Conversation

markusressel
Copy link
Collaborator

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.

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
@markusressel markusressel self-assigned this Dec 28, 2018
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 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})")
Copy link
Owner

Choose a reason for hiding this comment

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

What will this do?

Copy link
Collaborator Author

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)
Copy link
Owner

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?

Copy link
Collaborator Author

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.

Copy link
Owner

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?

Copy link
Collaborator Author

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?

library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt Outdated Show resolved Hide resolved
library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt Outdated Show resolved Hide resolved
library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt Outdated Show resolved Hide resolved
library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt Outdated Show resolved Hide resolved
library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt Outdated Show resolved Hide resolved
library/src/main/java/com/otaliastudios/zoom/ZoomEngine.kt Outdated Show resolved Hide resolved
removed unneeded post() calls
minimized constuctor calls on pan/scaledPan/mCurrentPanCorrection
removed unused method
Copy link
Owner

@natario1 natario1 left a 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
@markusressel
Copy link
Collaborator Author

Since you already approved this PR and I only fixed a small dev-merge mistake I will merge this PR now.
If any merge conflicts come up in #74 because of this and you are unsure what to do just let me know.
Cheers! 🎉

@markusressel markusressel merged commit 4848cff into master Jan 8, 2019
@markusressel markusressel deleted the feature/#66_pan_while_zoom branch January 8, 2019 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow pan while pinching
2 participants