Skip to content
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

[RFC/WIP] Add abstract types for gamut definitions (#139) #140

Closed
wants to merge 2 commits into from

Conversation

kimikage
Copy link
Collaborator

@kimikage kimikage commented Dec 7, 2019

This adds abstract types for the gamut definitions (cf. #139, #125).

This also changes the gamutmin/gamutmax definitions. The new gamutmin/gamutmax mean the corners of the minimum bounding box of the (implicitly) specified gamut. Specifying the gamut (i.e. "sRGB") specifies the range somewhat formally.

Edit:
Of course, if we allow breaking changes, we also have the option of moving these to Colors.jl. I think rand() (with gamuts) is too complex for ColorTypes.jl, a minimal package.

@kimikage
Copy link
Collaborator Author

kimikage commented Dec 7, 2019

We can generate the random TransparentColors. (cf. #125 (comment))

julia> rand(LabA)
LabA{Float64}(22.480820110815625,13.641014129939421,-5.753612633740147,0.23049096545302006)

We can override the default range of color space (in a somewhat tricky manner).

julia> ColorTypes.gamutmax(::Type{<:Lab}) = (100,200,200)

julia> rand(Lab)
Lab{Float64}(76.07250064744058,192.4625550050896,5.393193026026793)

Comment on lines +81 to +82
gamutmax(::Type{<:YIQ}, ::Type{Gamut_sRGB}) = (1,0.5957,0.5226)
gamutmin(::Type{<:YIQ}, ::Type{Gamut_sRGB}) = (0,-0.5957,-0.5226)
Copy link
Collaborator Author

@kimikage kimikage Dec 8, 2019

Choose a reason for hiding this comment

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

The color primaries of NTSC and sRGB are different, and the NTSC color gamut is actually wider than sRGB's one. However, this difference is often ignored as "RGB" does not explicitly conform the sRGB.
Therefore, the following measures can be considered:

  • Set the default color gamut for YIQ to Gamut_NTSC (or Gamut_SMPTE_C)
  • Narrow the range to be correct as sRGB

In any case, the gamut conflicts with the conversion methods in Colors.jl.

Comment on lines +87 to +88
gamutmax(::Type{<:AbstractGray}, ::Type{Gamut_sRGB}) = (1,)
gamutmin(::Type{<:AbstractGray}, ::Type{Gamut_sRGB}) = (0,)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strictly speaking, sRGB should not specify the so-called gray range. The range of luminance Y is [0,1], but Y is different from the so-called gray.

This also changes the `gamutmin`/`gamutmax` definitions.
@codecov
Copy link

codecov bot commented Dec 8, 2019

Codecov Report

Merging #140 into master will decrease coverage by 2.7%.
The diff coverage is 12.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
- Coverage   86.53%   83.82%   -2.71%     
==========================================
  Files           6        8       +2     
  Lines         490      581      +91     
==========================================
+ Hits          424      487      +63     
- Misses         66       94      +28
Impacted Files Coverage Δ
src/operations.jl 96.22% <ø> (-0.96%) ⬇️
src/ColorTypes.jl 100% <ø> (ø) ⬆️
src/gamut.jl 12.5% <12.5%> (ø)
src/precompile.jl 100% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b3b5e9...3b5576f. Read the comment docs.

@kimikage
Copy link
Collaborator Author

kimikage commented Dec 9, 2019

I'm going to rename "gamut.jl" -> "gamuts.jl".

@johnnychen94
Copy link
Member

I like the simplicity of this design 👍

src/gamut.jl Outdated Show resolved Hide resolved
Co-Authored-By: Johnny Chen <johnnychen94@hotmail.com>
@kimikage
Copy link
Collaborator Author

kimikage commented Mar 1, 2020

I prioritize extending Colors.jl and close this PR.(cf. #139 (comment))

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.

None yet

2 participants