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

Incorrect zoom coordinates when calling setZoom()/setZoomAnimated() #507

Closed
bartek-wesolowski opened this issue Jan 20, 2023 · 10 comments
Closed

Comments

@bartek-wesolowski
Copy link

I used an older version of TouchImageView in my map app. I have hardcoded point coordinates. I call setZoomAnimated() when someone searches for a particular point. When I updated to the newest version of TouchImageView then setZoomAnimated()/setZoom() started zooming on a different place even though the focus coordinates didn't change. On version 3.0.3 everything still works correctly. On 3.0.4 the focus coordinates are different.

Steps to reproduce
Use TouchImageView version 3.0.3 to show an image. Pan the image using setZoom(2f, 0.7f, 0.7f). Take a screenshot.
Do the same for version 3.0.4. The screenshots will be different even though the focus coordinates are the same.

@hannesa2
Copy link
Collaborator

It looks like that it's related to #329

To be honest, it's a community project and I'm happy to merge any pull request

@bartek-wesolowski
Copy link
Author

This change might have caused the problem.

@hannesa2
Copy link
Collaborator

hannesa2 commented Jan 20, 2023

This change and #329 are the same 😁

But how to proceed ? I've no clue. @SimFG do you have an idea ?

@bartek-wesolowski
Copy link
Author

You can use an image like the one below to test it. When I call setZoom(3f, 0.75f, 0.75f) I'd expect the focus to be on C7, and right now it's not there. Before the change it was working fine.

test_grid

@bartek-wesolowski
Copy link
Author

TouchImageView comparison

@hannesa2
Copy link
Collaborator

hannesa2 commented Jan 22, 2023

PR #509 doesn't fix the issue, but it helps hopefully later to find a solution with a test for zoom behavior.
At the bottom of the job you'll find Touch-Screenshots and here you'll find

image

This is the current situation. And at least it's a baseground

image

@bartek-wesolowski
Copy link
Author

I think that the changes made in #329 should be reverted because the formula is incorrect. Isn't the situation described in #329 (comment) a feature and not a bug? If we focus on point 0,0, don't we expect the image to be moved down like that? If not, shouldn't we adjust getFixTrans() instead of setZoom()? I started working on it a bit, but I'm going on vacation tomorrow and won't have access to my computer for a week.

@bartek-wesolowski
Copy link
Author

BTW, using Paparazzi for screenshot tests would be nice.

@hannesa2
Copy link
Collaborator

I started working on it a bit, but I'm going on vacation tomorrow and won't have access to my computer for a week.

I would love to see some outcome in the future !

@hannesa2
Copy link
Collaborator

closed with #514

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

No branches or pull requests

2 participants