sindresorhus / Defaults

💾 Swifty and modern UserDefaults
https://swiftpackageindex.com/sindresorhus/Defaults/documentation/defaults
MIT License
2k stars 120 forks source link

Color space is not preserved for `Color` type on iOS #97

Closed sindresorhus closed 2 years ago

sindresorhus commented 2 years ago

It is however preserved for UIColor.

Report: https://twitter.com/jordibruin/status/1516728958695354369

I have added some tests and this one fails on iOS: https://github.com/sindresorhus/Defaults/commit/3535f3d088113cf24705014eec6a17f0fd73237f#diff-d780be73215173c5c7564ee04feb252d30d24c3842b4d9f4aada400be8f37352R21

This seems to be a bug in when SwiftUI translates between UIColor and Color?

sindresorhus commented 2 years ago

@hank121314 Any ideas?

hank121314 commented 2 years ago

The behavior of UIColor seems weird.

I had tested with the following code

let uiColor = UIColor(displayP3Red: 1, green: 0.3, blue: 0.7, alpha: 1)
print(uiColor.cgColor.colorSpace!) // sRGB
let p3 = CGColorSpace(name: CGColorSpace.displayP3)!
let cgColor = uiColor.cgColor.converted(to: p3, intent: .defaultIntent, options: nil)!
print(cgColor.colorSpace!) // P3
let fixture = UIColor(cgColor: cgColor)
print(fixture.cgColor.colorSpace!) // P3

Even if we create UIColor with init(displayP3Red:green:blue:alpha:) the color space is still sRGB. And also when using NSSecureCodingBridge to serialize and deserialize UIColor, color space will also return to sRGB. There is need for further investigation 🤓 .

sindresorhus commented 2 years ago

UIColor always converts to and stores the color internally as "extended sRGB", but I had thought that it would store the original color space.

Maybe we could subclass UIColor and add a color space property. I'm not sure whether that would be archived or not though.

hank121314 commented 2 years ago

It is really hard to add a color space property in UIColor subclass. Reference: You Can't Subclass UIColor. Is it acceptable to create a class like this?

public class DefaultsUIColor {
    private var _colorSpace: Color.RGBColorSpace = .sRGB
    public var colorSpace: Color.RGBColorSpace { return _colorSpace }

    public let color: UIColor
    public init(_ colorSpace: Color.RGBColorSpace, red: Double, green: Double, blue: Double, alpha: Double)
}
sindresorhus commented 2 years ago

Actually, the issue is not with UIColor. UIColor does not preserve the color space, which is fine. Defaults should not try to work around that. The color value itself is correctly preserved.

The problem is that a SwiftUI.Color in contrast to UIColor does store the color space, but it's not preserved because we convert to UIColor. So I think the solution would be to use your above wrapper as internal storage when storing SwiftUI.Color.

hank121314 commented 2 years ago

Here is another approach to preserving the color space. If SwiftUi.Color has cgColor property, Defaults will serialize and store its colorspace and components, then deserialize it with these properties. If not, Defaults will use NSSecureCodingBridge to do the serialization and deserialization.

Example code:

extension Defaults {
    @available(iOS 15.0, macOS 11.0, tvOS 15.0, watchOS 8.0, iOSApplicationExtension 15.0, macOSApplicationExtension 11.0, tvOSApplicationExtension 15.0, watchOSApplicationExtension 8.0, *)
    public struct ColorBridge: Bridge {
        public typealias Value = Color
        public typealias Serializable = Any

        #if os(macOS)
        private typealias NativeColor = NSColor
        #else
        private typealias NativeColor = UIColor
        #endif

        public func serialize(_ value: Value?) -> Serializable? {
            guard let value = value else {
                return nil
            }
            guard
                let cgColor = value.cgColor,
                let colorSpace = cgColor.colorSpace?.name as? String,
                let components = cgColor.components
            else {
                return NativeColor.bridge.serialize(NativeColor(value))
            }

            return [colorSpace, components]
        }

        public func deserialize(_ object: Serializable?) -> Value? {
            if let object = object as? NativeColor.Serializable {
                guard let nativeColor = NativeColor.bridge.deserialize(object) else {
                    return nil
                }

                return Value(nativeColor)
            }

            guard
                let object = object as? Array<Any>,
                let rawColorspace = object[0] as? String,
                let colorspace = CGColorSpace(name: rawColorspace as CFString),
                let components = object[1] as? [CGFloat],
                let cgColor = CGColor(colorSpace: colorspace, components: components)
            else {
                return nil
            }

            return Value(cgColor: cgColor)
        }
    }
}
sindresorhus commented 2 years ago

@hank121314 That sound like a good solution. 👍

jordibruin commented 1 year ago

Just wanted to thank @hank121314 and @sindresorhus again for fixing this 🙂