New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More Extensions #8
Conversation
Sources/CIImage+Transformation.swift
Outdated
|
||
/// Returns a new image that represents the original image after scaling it by the given factors in x- and y-direction. | ||
/// | ||
/// The interpolation used for the sampling technique used by the image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be incomplete or wrong here
Sources/CIImage+Text.swift
Outdated
/// Generates an image that contains the given text. | ||
/// - Parameters: | ||
/// - text: The string of text to render. | ||
/// - fontName: The name of the font that should be used for rendering the text. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, add the default values into the doc string
/// - white: The unpremultiplied component value that should be use for all three color channels. | ||
/// - alpha: The alpha (opacity) component of the color. | ||
/// - colorSpace: The color space in which to create the new color. This color space must conform to the `CGColorSpaceModel.rgb` color space model. | ||
convenience init?(white: CGFloat, alpha: CGFloat = 1.0, colorSpace: CGColorSpace? = nil) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the parameter called white
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it's the amount of white in a gray color. I was actually transferring an API that is available for UIColor
and NSColor
to CIColor
.
Sources/CIColor+Extensions.swift
Outdated
/// | ||
/// - Parameters: | ||
/// - white: The unpremultiplied component value that should be use for all three color channels. | ||
/// This value can be of the `[0...1]` SDR range to create an EDR color. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing outside
Sources/CIColor+Extensions.swift
Outdated
/// | ||
/// - Parameters: | ||
/// - r: The unpremultiplied red component value. | ||
/// This value can be of the `[0...1]` SDR range to create an EDR color. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above (here and below)
Sources/CIColor+Extensions.swift
Outdated
self.init(red: r, green: g, blue: b, alpha: a, colorSpace: colorSpace) | ||
} | ||
|
||
/// Returns a color that provide a high contrast to the receiver. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
provides
Sources/CIKernel+MetalSource.swift
Outdated
} | ||
|
||
/// Compiles a Core Image kernel at runtime from the given Metal `source` string. | ||
/// If this feature is not supported by the OS, the legacy Core Image Kernel Language `ciklSource` are used instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is used instead
Sources/CIImage+Transformation.swift
Outdated
/// Returns a new image that represents the original image after moving the center of its extent to the given point. | ||
/// - Parameter point: The new center point of the image. | ||
/// - Returns: A moved image with the new center point. | ||
func centered(in point: CGPoint) -> CIImage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
centered(at point: CGPoint )
?
/// - white: The unpremultiplied component value that should be use for all three color channels. | ||
/// This value can be of the `[0...1]` SDR range to create an EDR color. | ||
/// - alpha: The alpha (opacity) component of the color. | ||
convenience init?(extendedWhite white: CGFloat, alpha: CGFloat = 1.0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That means we have to know the color's range at build time to call the right init. Would it make sense to use the one above that checks if the passed white is out of the range and uses the extended color space then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only a convenience method that inits the color with a specific color space.
I probably could add checks to the above API, but then I'd need to manipulate the passed color space there. And I can't do the same for the RGB initializers since they are built-in. That's why I decided for more explicit initializers here.
Sources/CIColor+Extensions.swift
Outdated
/// | ||
/// The returned color is either black or white, depending on which has the better visibility | ||
/// when put over the receiver color. | ||
var contrastColor: CIColor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not exactly the contrast color but a contrast label color, because the result is only either black or white
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually had it named like that, but then I figured that you can also use that for icons and other graphics that you'd overlay over an image, so I went for the more generic name. Should I change it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe something like visibleColor
or contrastOverlayColor
?
public extension CGColorSpace { | ||
|
||
/// The standard Red Green Blue (sRGB) color space. | ||
static var sRGBColorSpace: CGColorSpace? { CGColorSpace(name: CGColorSpace.sRGB) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just call this sRGB
(and the others accordingly)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there already exist static properties with those names, but they only refer to the name of the color space, not the color spaces themself (like you see here in the implementation of the property).
|
||
/// The recommendation of the International Telecommunication Union (ITU) Radiocommunication sector for the BT.2100 color space, | ||
/// with the HLG transfer function. | ||
@available(iOS 12.6, macCatalyst 13.1, macOS 10.15.6, tvOS 12.0, watchOS 5.0, *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mention the fallback to 2020 in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually not a fallback but a misnaming from Apple that they fixed in later OS versions.
You see, BT.2100 HLG is actually just BT.2020 with the HLG transfer function (instead of the sRGB transfer function). That's probably why they had it named itur_2020_HLG
before, but itur_2100_HLG
is "more" correct and should cause for less confusion.
Sources/CIImage+Transformation.swift
Outdated
|
||
/// Returns a new image that represents the original image after scaling it by the given factors in x- and y-direction. | ||
/// | ||
/// The interpolation used for the sampling technique used by the image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What?
/// Returns a new image that represents the original image after moving its origin within the working space to the given point. | ||
/// - Parameter origin: The new origin point of the image. | ||
/// - Returns: A moved image with the new origin. | ||
func moved(to origin: CGPoint) -> CIImage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
withOrigin(at: )
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is moved(to:)
too confusing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I think about it, the by
and to
help
Added a lot more useful extensions that I came up with while working on my EDR test pattern (coming in the next PR).