[scroll] use the exact visible size on macOS, instead of the clip view's size rounded to whole pixels

ComposeUI/Sources/ComposeUI/ComposeView/ComposeView.swift

@@ -1645,16 +1645,7 @@ open class ComposeView: BaseScrollView {
     // a legacy scroller shrinks the clip view (`visibleSize`), so the size comes from the view's bounds, where AppKit
     // tiles the clip view without scrollers. it then depends only on the bounds, so showing or hiding a scroller never
     // changes the layout.
-    // AppKit rounds the clip view's size, not its edges, to whole backing pixels, through any bounds scaling, so the size
-    // rounds the same way to match the area the view shows. each dimension rounds with the pixel scale along its own axis,
-    // the length of a converted unit vector, since converting the size itself would mix the dimensions under rotation.
-    let unitX = convertToBacking(CGSize(width: 1, height: 0))
-    let unitY = convertToBacking(CGSize(width: 0, height: 1))
-    let pixelsPerUnitX = hypot(unitX.width, unitX.height)
-    let pixelsPerUnitY = hypot(unitY.width, unitY.height)
-    let width = (self.bounds.width * pixelsPerUnitX).rounded() / pixelsPerUnitX
-    let height = (self.bounds.height * pixelsPerUnitY).rounded() / pixelsPerUnitY
-    return CGRect(origin: contentOffset, size: CGSize(width: width, height: height))
+    return CGRect(origin: contentOffset, size: self.bounds.size)
     #endif
 
     #if canImport(UIKit)

ComposeUI/Sources/ComposeUI/CrossPlatform/ScrollView/ScrollView+Scrolling.swift

@@ -58,24 +58,28 @@ public extension ScrollView {
     contentSize.height - visibleSize.height + adjustedContentInset.bottom
   }
 
+  // Scrolling comes to rest on whole pixels, since AppKit aligns the content to the window's pixels and UIKit snaps the
+  // offset to them, which can leave it up to about a pixel from an edge computed from the exact sizes. So the
+  // properties below count an offset within a pixel of an edge as at the edge.
+
   /// Whether the scroll view can scroll to the left.
   var canScrollToLeft: Bool {
-    contentOffset.x > minOffsetX
+    contentOffset.x > minOffsetX + pixelSize.width
   }
 
   /// Whether the scroll view can scroll to the right.
   var canScrollToRight: Bool {
-    contentOffset.x < maxOffsetX
+    contentOffset.x < maxOffsetX - pixelSize.width
   }
 
   /// Whether the scroll view can scroll to the top.
   var canScrollToTop: Bool {
-    contentOffset.y > minOffsetY
+    contentOffset.y > minOffsetY + pixelSize.height
   }
 
   /// Whether the scroll view can scroll to the bottom.
   var canScrollToBottom: Bool {
-    contentOffset.y < maxOffsetY
+    contentOffset.y < maxOffsetY - pixelSize.height
   }
 
   #if canImport(UIKit)

ComposeUI/Sources/ComposeUI/CrossPlatform/ScrollView/ScrollView.swift

@@ -85,15 +85,25 @@ open class ScrollView: NSScrollView {
   }
 
   /// The size of the visible area in content coordinates, like `UIScrollView`'s `visibleSize`.
+  ///
+  /// It's exact, while the clip view rounds its own size to whole pixels.
   public var visibleSize: CGSize {
-    contentView.bounds.size
+    let boundsSize = contentView.bounds.size
+    let frameSize = contentView.frame.size
+    guard let size = (contentView as? ScrollClipView)?.unroundedSize, frameSize.width > 0, frameSize.height > 0 else {
+      return boundsSize
+    }
+    // the clip view's bounds are its frame in content coordinates, which magnification or a caller can scale differently
+    // along each axis, so the size the clip view was given scales the same way
+    return CGSize(width: size.width * (boundsSize.width / frameSize.width), height: size.height * (boundsSize.height / frameSize.height))
   }
 
   override public init(frame: CGRect) {
     super.init(frame: frame)
 
     updateCommonSettings()
 
+    contentView = ScrollClipView()
     documentView = BaseView()
 
     startObservingBoundsChange()
@@ -265,10 +275,12 @@ open class ScrollView: NSScrollView {
   }
 
   private func updateScrollElasticity() {
+    // compare the document with the exact visible size: the clip view rounds its size to whole pixels, so content as
+    // large as the view would otherwise bounce by the leftover fraction
     if alwaysBounceHorizontal {
       horizontalScrollElasticity = .allowed
     } else {
-      if contentContainerView.frame.width.extends(beyond: super.contentSize.width) {
+      if contentContainerView.frame.width.extends(beyond: visibleSize.width) {
         horizontalScrollElasticity = .allowed
       } else {
         horizontalScrollElasticity = .none
@@ -278,7 +290,7 @@ open class ScrollView: NSScrollView {
     if alwaysBounceVertical {
       verticalScrollElasticity = .allowed
     } else {
-      if contentContainerView.frame.height.extends(beyond: super.contentSize.height) {
+      if contentContainerView.frame.height.extends(beyond: visibleSize.height) {
         verticalScrollElasticity = .allowed
       } else {
         verticalScrollElasticity = .none
@@ -380,15 +392,18 @@ private final class ScrollSession {
 
 private extension ScrollView {
 
-  /// Whether the content offset is outside the scrollable range along either axis.
+  /// Whether the content offset is outside the scrollable range along either axis, by more than a pixel.
   ///
   /// Along an axis where the content is smaller than the visible size, the maximum offset is below the minimum, and the
   /// scrollable range is the minimum offset alone.
   var isContentOffsetOutsideScrollableRange: Bool {
     let offset = contentOffset
+    // AppKit aligns where scrolling comes to rest to the window's pixels, which can leave the offset up to about a
+    // pixel past an end of the exact range, so an offset within a pixel of the range counts as inside it
+    let pixel = pixelSize
     let minX = minOffsetX
     let minY = minOffsetY
-    return offset.x < minX || offset.x > max(minX, maxOffsetX) || offset.y < minY || offset.y > max(minY, maxOffsetY)
+    return offset.x < minX - pixel.width || offset.x > max(minX, maxOffsetX) + pixel.width || offset.y < minY - pixel.height || offset.y > max(minY, maxOffsetY) + pixel.height
   }
 
   /// Whether the view has a parent scroll view that satisfies the condition.
@@ -404,6 +419,19 @@ private extension ScrollView {
   }
 }
 
+/// A clip view that keeps the size AppKit tiles it to, before it rounds that size to whole pixels.
+private final class ScrollClipView: NSClipView {
+
+  /// The size AppKit last set, before the clip view rounded it.
+  private(set) var unroundedSize: CGSize?
+
+  override func setFrameSize(_ newSize: NSSize) {
+    unroundedSize = newSize
+
+    super.setFrameSize(newSize)
+  }
+}
+
 #endif
 
 // MARK: - UIKit
@@ -426,4 +454,22 @@ extension ScrollView {
     return self
     #endif
   }
+
+  /// The size of a pixel in content coordinates.
+  var pixelSize: CGSize {
+    #if canImport(AppKit)
+    // the clip view's coordinates are the content coordinates, so converting from them includes the magnification and
+    // any scaling of the bounds. each axis converts a unit vector and takes its length, since a rotation would mix the
+    // axes of a converted size.
+    let unitX = contentView.convertToBacking(CGSize(width: 1, height: 0))
+    let unitY = contentView.convertToBacking(CGSize(width: 0, height: 1))
+    return CGSize(width: 1 / hypot(unitX.width, unitX.height), height: 1 / hypot(unitY.width, unitY.height))
+    #endif
+
+    #if canImport(UIKit)
+    // the content offset is in the scroll view's own points, which zooming doesn't scale
+    let length = 1 / windowScaleFactor
+    return CGSize(width: length, height: length)
+    #endif
+  }
 }

ComposeUI/Tests/ComposeUITests/ComposeView/ComposeView+RenderBoundsTests.swift

@@ -445,7 +445,7 @@ class ComposeView_RenderBoundsTests: XCTestCase {
     expect(renderedFrames(in: view)) == (0 ..< 5).map { CGRect(x: 0, y: 65 + CGFloat($0) * 10, width: 400, height: 10) }
   }
 
-  func test_renderBounds_fractionalViewSize_roundsDownWithTheClipView() {
+  func test_renderBounds_fractionalViewSize_clipViewRoundsDown_staysExact() {
     // given: a 99.2 × 99.2 view in a window, with overlay scrollers, showing flexible rows that overflow vertically
     let window = TestWindow()
     let view = ComposeView {
@@ -460,19 +460,29 @@ class ComposeView_RenderBoundsTests: XCTestCase {
     view.scrollerStyle = .overlay
     window.contentView().addSubview(view)
 
-    // when: the view refreshes
+    // when: the view refreshes, and its scroll elasticity updates
     view.refresh(animated: false)
-
-    // then: the rows lay out for the size AppKit rounds the clip view down to, so the content is no wider than the
-    // visible area, and only the vertical scroller shows
-    expect(view.visibleSize) == CGSize(width: 99, height: 99)
-    expect(view.test.lastRenderBounds?.size) == CGSize(width: 99, height: 99)
-    expect(view.contentSize) == CGSize(width: 99, height: 400)
+    view.invalidateScrollElasticity()
+    RunLoop.main.run(until: Date(timeIntervalSinceNow: 1e-3))
+
+    // then: the rows lay out for the exact size, while AppKit rounds the clip view down to whole pixels, at both 1x and
+    // 2x
+    expect(view.visibleSize) == CGSize(width: 99.2, height: 99.2)
+    expect(view.test.lastRenderBounds?.size) == CGSize(width: 99.2, height: 99.2)
+    expect(view.contentView.frame.size) == CGSize(width: 99, height: 99)
+
+    // then: the document keeps the exact width, which fits the exact visible size even though it's wider than the clip
+    // view, so the maximum horizontal offset is 0, the view doesn't bounce sideways, and only the vertical scroller shows
+    expect(view.contentSize) == CGSize(width: 99.2, height: 400)
+    expect(view.maxOffsetX) == 0
+    expect(view.canScrollToRight) == false
+    expect(view.horizontalScrollElasticity) == .none
+    expect(view.verticalScrollElasticity) == .allowed
     expect(view.hasVerticalScroller) == true
     expect(view.hasHorizontalScroller) == false
   }
 
-  func test_renderBounds_fractionalViewSize_roundsUpWithTheClipView() {
+  func test_renderBounds_fractionalViewSize_clipViewRoundsUp_staysExact() {
     // given: a 99.8 × 99.8 view at (0.4, 0.4) in a window, with legacy scrollers, showing 99.7 × 99.7 content
     let window = TestWindow()
     let view = ComposeView {
@@ -485,20 +495,22 @@ class ComposeView_RenderBoundsTests: XCTestCase {
     // when: the view refreshes
     view.refresh(animated: false)
 
-    // then: the content lays out for the size AppKit rounds the clip view up to, which it fits, so the view neither
-    // scrolls nor shows scrollers
-    expectSize(view.visibleSize, approximatelyEquals: CGSize(width: 100, height: 100))
-    expectSize(view.test.lastRenderBounds?.size, approximatelyEquals: CGSize(width: 100, height: 100))
+    // then: the content lays out for the exact size, which it fits, while AppKit rounds the clip view up to whole pixels,
+    // and the document keeps the exact size, so the view neither scrolls nor shows scrollers
+    expect(view.visibleSize) == CGSize(width: 99.8, height: 99.8)
+    expect(view.test.lastRenderBounds?.size) == CGSize(width: 99.8, height: 99.8)
+    expectSize(view.contentView.frame.size, approximatelyEquals: CGSize(width: 100, height: 100))
+    expect(view.contentSize) == CGSize(width: 99.8, height: 99.8)
     expect(view.isScrollEnabled) == false
     expect(view.hasVerticalScroller) == false
     expect(view.hasHorizontalScroller) == false
   }
 
-  func test_renderBounds_scaledBounds_fractionalViewSize_matchesTheClipView() {
-    for (frameLength, contentLength) in [(CGFloat(99.2), CGFloat(148.8)), (99.8, 149.8)] {
-      // given: a view in a window with a fractional frame and bounds 1.5 times as large, showing content between the
-      // bounds' size and the size AppKit rounds the clip view to, at both 1x and 2x: down to 148.5 units for a 99.2 pt
-      // frame, and up to 150 for a 99.8 pt one
+  func test_renderBounds_scaledBounds_fractionalViewSize_fittingContentDoesNotScroll() {
+    for (frameLength, contentLength) in [(CGFloat(99.2), CGFloat(148.6)), (99.8, 149.6)] {
+      // given: a view in a window with a fractional frame and bounds 1.5 times as large, so AppKit rounds the clip view
+      // to whole pixels through the scaling, at both 1x and 2x: down to 148.5 units for a 99.2 pt frame, and up to 150
+      // for a 99.8 pt one, showing content that fits the exact bounds, even where it's larger than the rounded clip view
       let window = TestWindow()
       let view = ComposeView {
         LayerNode().frame(width: contentLength, height: contentLength)
@@ -510,10 +522,14 @@ class ComposeView_RenderBoundsTests: XCTestCase {
       // when: the view refreshes
       view.refresh(animated: false)
 
-      // then: the content lays out for the clip view's size, so the view scrolls exactly when the content doesn't fit it
-      let clipSize = view.contentView.frame.size
-      expectSize(view.test.lastRenderBounds?.size, approximatelyEquals: clipSize)
-      expect(view.isScrollEnabled) == (contentLength > clipSize.width)
+      // then: the content lays out for the exact bounds, and the document keeps the exact size, so the view doesn't
+      // scroll
+      let boundsLength = frameLength * 1.5
+      expectSize(view.test.lastRenderBounds?.size, approximatelyEquals: CGSize(width: boundsLength, height: boundsLength))
+      expectSize(view.contentSize, approximatelyEquals: CGSize(width: boundsLength, height: boundsLength))
+      expect(view.isScrollEnabled) == false
+      expect(view.maxOffsetX).to(beApproximatelyEqual(to: 0, within: 1e-9))
+      expect(view.maxOffsetY).to(beApproximatelyEqual(to: 0, within: 1e-9))
     }
   }