Skip to content

Test pattern - #9

Open
frankschlegel wants to merge 31 commits into
mainfrom
test-pattern
Open

Test pattern#9
frankschlegel wants to merge 31 commits into
mainfrom
test-pattern

Conversation

@frankschlegel

Copy link
Copy Markdown
Member

Created an EDR brightness and wide gamut color test pattern image that can be used, for instance, to verify colors are displayed correctly, filters conserve those color properties correctly, etc.

…ions into test-pattern

# Conflicts:
#	README.md
#	Sources/CIColor+Extensions.swift
#	Sources/CIImage+Blending.swift
#	Sources/CIImage+Text.swift
#	Sources/CIImage+Transformation.swift
#	Sources/CIKernel+MetalSource.swift
#	Tests/ColorExtensionsTests.swift
#	Tests/ImageTransformationTests.swift
@frankschlegel
frankschlegel requested a review from maikz August 9, 2022 10:53
@frankschlegel
frankschlegel requested a review from phjs August 9, 2022 10:53

@maikz maikz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed parts of it. The rest will follow soon

Comment thread Sources/CIImage+TestPattern.swift Outdated
// Note: We reverse here so the first color specified in the array is displayed on the top.
for (row, color) in rowColors.reversed().enumerated() {
var swatch = CIImage.colorSwatch(for: color)
swatch = swatch.moved(to: CGPoint(x: CGFloat(column) * (swatch.extent.width + self.margin), y: CGFloat(row) * (swatch.extent.height + self.margin)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let swatch = CIImage.colorSwatch()
    .moved(to:)

or even

pattern = CIImage.colorSwatch()
    .moved(to:)
   .composited(over: pattern)

?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same below

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here (and with the label) is that I need the extent of the swatch in the moved(to:), so I can't really chain it.

/// Colors for which swatches should be rendered, ordered by columns (outer) and rows (inner arrays).
private static var swatchColors: [[CIColor]] { [[.red, .green, .blue], [.cyan, .magenta, .yellow]] }
/// The different color spaces in which colors should be rendered in the color swatches.
private static var swatchColorSpaces: [(label: String, colorSpace: CGColorSpace)] = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have some kind of Swatch type with label, color space and color?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dict here is only needed to assign labels to the different color spaces. What would the Swatch type replace then?

// Put a label in the bottom left corner over the tile that displays the brightness value (also in nits).
let labelText = String(format: "%.2f (%d nits)", brightness, Int(brightness * 500))
let labelColor = CIColor(extendedWhite: brightness)!.contrastOverlayColor
let label = CIImage.text(labelText, fontName: self.labelFont, fontSize: size.height / 8.0, color: labelColor)!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to be very safe we can add a fontScaleFactor since we divide by 8 multiple times.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And one time / 7.0. 😅
I didn't want to over-engineer this more than I already did. Otherwise, I'll end up writing a layout system for Core Image. 😬

Comment thread Tests/TestPatternTests.swift Outdated
@frankschlegel
frankschlegel requested a review from maikz August 18, 2022 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants