Fix a bug causing keyboard view offset to be incorrect

[dbburgess]

In some rare cases, if the height of the view is a fractional point
(i.e., not a whole number), it would cause the views to not be offset
despite the keyboard being shown on top of them. This does not happen
with every fractional height. Different devices also behave a little
differently in seemingly identical layouts, due to their pixel density
being different.

The base issue is that, due to floating point rounding errors, two
values that should be identical and pass the guard fail to do so,
because the lack of precision results in them not being equal. By
flooring the values, we can ignore really minor differences and ensure
rounding errors don't cause this issue.

I added an example test case in the ChattoApp example project on
the keyboard-issue branch of my fork. You can see the bug in action here:

The sample project illustrates the issue, outputting this when showing the keyboard:
maxY: 458.333333333333, height: 458.333333333333, equal?: false
Despite the values being nearly identical, they aren't quite equal (thanks floating point numbers!), resulting in the guard catch and returning 0, when it shouldn't. Flooring the values fixes this.

In my testing with the example, it only replicated on the iPhone Plus, but other devices seemed to be okay. I think this is due to different resolutions / densities resulting in a specific use case on that device. It could also possibly happen on other devices, this is just the specific instance I ran into. In any case, I think the fix should resolve it for all devices.

[codecov-io]

Current coverage is 63.56% (diff: 66.66%)

Merging #204 into master will increase coverage by 0.02%

@@             master       #204   diff @@
==========================================
  Files            62         63     +1   
  Lines          3486       3488     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2215       2217     +2   
  Misses         1271       1271          
  Partials          0          0          

Powered by Codecov. Last update e881ae9...86c1c79

[diegosanchezr]

Hi @dbburgess, thanks for the contribution.

This is a nitpick, but I think it's more correct to use a pixel aware rounding instead of the plain floor, as we do in ChattoAdditions with bma_round() to avoid pixel misalignment.

Could you create Utils.swift in Chatto/Source with

private let scale = UIScreen.mainScreen().scale
private let scaleInv = 1 / scale

extension CGFloat {
    func bma_round() -> CGFloat {
        let pixelCount = round(self * scale)
        return pixelCount * scaleInv
    }
}

And then do

guard rectInView.maxY.bma_round() >= self.view.bounds.height.bma_round() else { return 0 }

Note that bma_round() in ChattoAdditions is ceil(self * scale) * (1.0 / scale) which is wrong: 0.333334.bma_round() and 0.333333.bma_round() would return completely different values on iPhone 6+. Can you update also ChattoAdditions.bma_round() to use the same version?

Thanks again!

[dbburgess]

Thanks @diegosanchezr, that's a good suggestion! Nitpicky or not, I appreciate your feedback, it certainly improves things. I made the change and verified it all seemed work correctly still.

One minor detail is because I added Utils.swift via Xcode, the hash for it is shorter than the other files. If I ran pod install to let CocoaPods add the file and update the project file, the diff was just pretty crazy with lots of re-organization and other changes. I opted to avoid that to keep the diff simple, but can do otherwise if desired.

[diegosanchezr]

Argh, let's not do this. It breaks size calculation 😢

I does make sense to round up enclosing rects...

Instead, I suggest doing:

Chatto/Source/Utils.swift:

private let scale = UIScreen.mainScreen().scale

infix operator >=~ { }
func >=~ (lhs: CGFloat, rhs: CGFloat) -> Bool {
    return round(lhs * scale) >= round(rhs * scale)
}

guard rectInView.maxY >=~ self.view.bounds.height else { return 0 }

And revert all changes in ChattoAdditions.

Don't forget to add Utils.swift to Chatto project in Chatto.xcworkspace too, otherwise the branch won't compile in Travis and Carthage builds will break

Thanks!

[dbburgess]

Doh! That's unfortunate, good catch. I've made the suggested changes. Hopefully the build passes this time. 👍

[diegosanchezr]

Thanks again!