Bugfix/#152 pan to center #158
Conversation
|
Thinking about this, could this also be an issue in |
| val extraWidth = contentWidth - containerWidth | ||
| val extraHeight = contentHeight - containerHeight | ||
| val extraWidth = contentWidth * realZoom - containerWidth | ||
| val extraHeight = contentHeight * realZoom - containerHeight |
There was a problem hiding this comment.
These are the important 2 lines.
There was a problem hiding this comment.
If it helps, anytime I go back to this repo after some time, I don't understand the code either. And that's because of bad naming I think (like, what the hell is a realZoom?). We should go through a big change and rethink/rename APIs at some point.
About this PR I'm a bit confused as to why it seemed to work just fine until now (the demo app correctly centers the image IIRC). But the changes you made seem to make sense to me, so if you know they fix the issue, let's merge this!
|
My guess is that the issue is related to the dimensions of the device, the image, or the fact that multiple images are applied in succession (setImageDrawable) or a combination of those things. |
|
I see. Feel free to merge when you're ready |
|
I have done some additional monkey testing and haven't found any issues, so I will merge and we will see if something explodes 😅 |
Honestly, I still can't quite wrap my head around how the matrix transformation stuff works, so I am not entirely sure if this is the right thing to do to fix the issue.
I tested the
ZoomImageViewon my device with different types of images of different size, and it works for all of them. I have not tested if code based functions likepanToormoveToare affected by this. If I should do so please let me know.It would probably be a good idea to add tests for these transformation functions since - at least for outside contributors - it is probably really hard to understand their intended and correct behavior, and a tiny change in code can have a massive impact on the functionality.
Solution
As I discovered that your proposal code for centering an image in #47 did not work I found the code in
computeTransformationPan()that did the exact same thing. Applying therealZoomto the content to match itswidthandheightvalues with thewidthandheightof the container fixed the issue.Again, I don't understand every line of code in this lib yet, so, please have a thorough look and let me know if anything about this change worries you.