search

flavioarfaria / KenBurnsView

Android ImageViews animated by Ken Burns Effect
Apache License 2.0
2.74k stars 437 forks source link

haveSameAspectRatio #17

Closed alaeri closed 8 years ago

alaeri commented 9 years ago

I had a weird crash when I rotated my tablet several times with the view enabled. I have pinpointed it to the MathUtils.haveSameAspectRatio() function, and the truncation...

If my previous destRect had a ratio of 0.637 and my new viewPort has a rect ratio of 0.625 The function will say that the viewPortRatio has not changed. (even if we can tell right now that the difference between the two is more than 0.01)

If the random rect generator, provided with a viewport with a 0.625 ratio then returns a rect with a ratio of 0.624999, this will raise the IncompatibleRatioException in the Transition constructor.

I believe the truncating is not that good of an idea when you are comparing the aspect ratios. Imho, it should at least be one decimal further than the comparison itself.

flavioarfaria commented 9 years ago

Hi alaeri,

Thank you for your PR. I just took a look at your solution and this is something that I already tried before. It'll sure solve the problem you're having now, but it also introduces some problems in other scenarios too. I agree with you that truncating is not a very good solution for the problem, since it is caused by float point precision errors. I'm still thinking about a better way to solve it, but I'll need to dive into all the computation details of the transitions. You're invited to try to come up with this better solution as well. ;)

alaeri commented 9 years ago

Hi Flavioarfaria, It might introduce other issues later... but I believe it is a solution for this function which was mathematically unsound :) Wouldn't it be better to integrate this one and then fix the other issues which arise?

EricSiegNW commented 9 years ago

I haven't been able to repro this, but another dev got a stack trace that also threw this exception. @flavioarfaria can you describe the downsides of adding another decimal point of precision? I'm tempted to do that, but don't understand the downsides of doing so.