From ec08955bb078409a176c7b953f3bec43f9092168 Mon Sep 17 00:00:00 2001 From: rcancro Date: Thu, 29 Mar 2018 11:13:26 -0700 Subject: [PATCH 1/2] [Issue 838] Update ASCeilPixelValue and ASRoundPixelValue Layouts can come back for 3x devices as a repeating decimal. Floats/doubles have a hard time representing these values and often the last digit or two will be garbage. This garbage can result in unexpected values from `ceil` or `round`. I try to fix this by subtracting `FLT_EPSILON` from the value before calling `ceil` or `round` https://github.com/TextureGroup/Texture/issues/838 --- Source/Private/ASInternalHelpers.m | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/Source/Private/ASInternalHelpers.m b/Source/Private/ASInternalHelpers.m index 478f8694b..4f068e3ec 100644 --- a/Source/Private/ASInternalHelpers.m +++ b/Source/Private/ASInternalHelpers.m @@ -193,16 +193,34 @@ CGSize ASCeilSizeValues(CGSize s) return CGSizeMake(ASCeilPixelValue(s.width), ASCeilPixelValue(s.height)); } +// With 3x devices layouts will often to compute to pixel bounds but +// include garbage values beyond the precision of a float/double. +// This garbage can result in a pixel value being rounded up when it isn't +// necessary. +// +// For example, imagine a layout that comes back with a height of 100.666666666669 +// for a 3x device: +// 100.666666666669 * 3 = 302.00000000000699 +// ceil(302.00000000000699) = 303 +// +// If we use FLT_EPSILON to get rid of the garbage at the end of the value, +// things work as expected: +// (100.666666666669 - FLT_EPSILON) * 3 = 301.99999964237912 +// ceil(301.99999964237912) = 302 +// +// For even more conversation around this, see: +// https://github.com/TextureGroup/Texture/issues/838 CGFloat ASCeilPixelValue(CGFloat f) { CGFloat scale = ASScreenScale(); - return ceil(f * scale) / scale; + return ceil((f - FLT_EPSILON) * scale) / scale; } +// See ASCeilPixelValue for an explanation of (f - FLT_EPSILON) CGFloat ASRoundPixelValue(CGFloat f) { CGFloat scale = ASScreenScale(); - return round(f * scale) / scale; + return round((f - FLT_EPSILON) * scale) / scale; } @implementation NSIndexPath (ASInverseComparison) From 2acffde9a7135cf9f813d51b9be7bcd9b8da823a Mon Sep 17 00:00:00 2001 From: ricky cancro Date: Fri, 30 Mar 2018 13:15:17 -0700 Subject: [PATCH 2/2] addressed comments on the pr --- CHANGELOG.md | 1 + Source/Private/ASInternalHelpers.m | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5beec7c2e..32708c063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## master * Add your own contributions to the next release on the line below this with your name. +- User FLT_EPSILON in ASCeilPixelValue and ASFloorPixelValue to help with floating point precision errors when computing layouts for 3x devices. [Ricky Cancro](https://github.com/rcancro) [#838](https://github.com/TextureGroup/Texture/pull/864) - Disable interface colescing and match to pre-colescing interface update behavior [Max Wang](https://github.com/wsdwsd0829) [#862](https://github.com/TextureGroup/Texture/pull/862) - [ASDisplayNode] Add safeAreaInsets, layoutMargins and related properties to ASDisplayNode, with full support for older OS versions [Yevgen Pogribnyi](https://github.com/ypogribnyi) [#685](https://github.com/TextureGroup/Texture/pull/685) - [ASPINRemoteImageDownloader] Allow cache to provide animated image. [Max Wang](https://github.com/wsdwsd0829) [#850](https://github.com/TextureGroup/Texture/pull/850) diff --git a/Source/Private/ASInternalHelpers.m b/Source/Private/ASInternalHelpers.m index 4f068e3ec..3912a66c7 100644 --- a/Source/Private/ASInternalHelpers.m +++ b/Source/Private/ASInternalHelpers.m @@ -177,10 +177,23 @@ CGSize ASFloorSizeValues(CGSize s) return CGSizeMake(ASFloorPixelValue(s.width), ASFloorPixelValue(s.height)); } +// See ASCeilPixelValue for a more thoroguh explanation of (f + FLT_EPSILON), +// but here is some quick math: +// +// Imagine a layout that comes back with a height of 100.66666666663 +// for a 3x deice: +// 100.66666666663 * 3 = 301.99999999988995 +// floor(301.99999999988995) = 301 +// 301 / 3 = 100.333333333 +// +// If we add FLT_EPSILON to normalize the garbage at the end we get: +// po (100.66666666663 + FLT_EPSILON) * 3 = 302.00000035751782 +// floor(302.00000035751782) = 302 +// 302/3 = 100.66666666 CGFloat ASFloorPixelValue(CGFloat f) { CGFloat scale = ASScreenScale(); - return floor(f * scale) / scale; + return floor((f + FLT_EPSILON) * scale) / scale; } CGPoint ASCeilPointValues(CGPoint p) @@ -202,11 +215,13 @@ CGSize ASCeilSizeValues(CGSize s) // for a 3x device: // 100.666666666669 * 3 = 302.00000000000699 // ceil(302.00000000000699) = 303 +// 303/3 = 101 // // If we use FLT_EPSILON to get rid of the garbage at the end of the value, // things work as expected: // (100.666666666669 - FLT_EPSILON) * 3 = 301.99999964237912 // ceil(301.99999964237912) = 302 +// 302/3 = 100.666666666 // // For even more conversation around this, see: // https://github.com/TextureGroup/Texture/issues/838 @@ -216,11 +231,10 @@ CGFloat ASCeilPixelValue(CGFloat f) return ceil((f - FLT_EPSILON) * scale) / scale; } -// See ASCeilPixelValue for an explanation of (f - FLT_EPSILON) CGFloat ASRoundPixelValue(CGFloat f) { CGFloat scale = ASScreenScale(); - return round((f - FLT_EPSILON) * scale) / scale; + return round(f * scale) / scale; } @implementation NSIndexPath (ASInverseComparison)