apple / swift-collections-benchmark

A benchmarking tool for Swift Collection algorithms
Apache License 2.0
340 stars 23 forks source link

Build fails off macOS due to missing AffineTransform in Foundation module #5

Closed brennanMKE closed 3 years ago

brennanMKE commented 3 years ago

When running the build for all platforms I found it fails due to AffineTransform only being available on macOS. See the docs below.

The error shows in CGTypes.swift in block which includes iOS, tvOS and watchOS which do not have AffineTransform in Foundation. The build target is set to build for all platform and this package is a dependency of Swift Collections which will also need to build across Apple's platforms and ideally Linux and Windows.

Information

Checklist

Steps to Reproduce

Expected behavior

There should be an equivalent type which can be used off macOS on the other platforms. I tried CoreGraphics.CGAffineTransform but it does not work quite the same.

Actual behavior

Due to the missing type the build fails for iOS. It appears it would only build for macOS.

brennanMKE commented 3 years ago

I did update CGTypes.swift to the code below.

#if os(macOS)
import CoreGraphics
import Foundation

public typealias CGFloat = CoreGraphics.CGFloat
public typealias CGPoint = CoreGraphics.CGPoint
public typealias CGSize = CoreGraphics.CGSize
public typealias CGRect = CoreGraphics.CGRect
public typealias AffineTransform = Foundation.AffineTransform
#elseif os(iOS) || os(watchOS) || os(tvOS)
import CoreGraphics
import Foundation

public typealias CGFloat = CoreGraphics.CGFloat
public typealias CGPoint = CoreGraphics.CGPoint
public typealias CGSize = CoreGraphics.CGSize
public typealias CGRect = CoreGraphics.CGRect
public typealias AffineTransform = CoreGraphics.CGAffineTransform
#else
import Foundation

public typealias CGFloat = Foundation.CGFloat
public typealias CGPoint = Foundation.CGPoint
public typealias CGSize = Foundation.CGSize
public typealias CGRect = Foundation.CGRect
public typealias AffineTransform = Foundation.AffineTransform
#endif

Then in Chart.swift I made these changes.

var chartTransform = AffineTransform.identity
#if os(iOS) || os(watchOS) || os(tvOS)
chartTransform.translatedBy(x: rect.minX, y: rect.minY)
chartTransform.scaledBy(x: rect.width, y: rect.height)
#else
chartTransform.translate(x: rect.minX, y: rect.minY)
chartTransform.scale(x: rect.width, y: rect.height)
#endif
#if os(iOS) || os(watchOS) || os(tvOS)
let ct = chartTransform.translatedBy(x: p.x, y: p.y)
return CGPoint(x: ct.tx, y: ct.ty)
#else
return chartTransform.transform(p)
#endif

The build runs and tests pass for macOS and iOS. I do not know if this change is valid though.

lorentey commented 3 years ago

Oh good catch! 👍

Hm; the platform differences between CGAffineTransform and AffineTransform shouldn't leak into the chart implementation.

I wonder if we should just create a platform-independent wrapper that forwards calls to whatever matrix implementation is available. (Or we could simply stop using CGFloat/CGPoint/CGSize/CGRect/AffineTransform, and roll our own implementation instead. This inconsistency (plus the whole CGFloat mess) is annoying enough that I've come this close 🤏 to doing that at several points during the implementation work.)