JuliaGraphics / ColorTypes.jl

Basic color definitions and traits
Other
78 stars 36 forks source link

Fix regression of constant propagation in `color_type` #233

Closed kimikage closed 3 years ago

kimikage commented 3 years ago

The recursion mechanism using supertype could be broken when a subtype of TransparentColor was added. Therefore, the checking with nameof was used for safety in PR #211. However, it interfered with constant folding. This avoids the use of nameof for the checking.

Closes #208

codecov[bot] commented 3 years ago

Codecov Report

Merging #233 (84c920d) into master (3507106) will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   95.28%   95.30%   +0.01%     
==========================================
  Files           8        8              
  Lines         743      745       +2     
==========================================
+ Hits          708      710       +2     
  Misses         35       35              
Impacted Files Coverage Δ
src/traits.jl 98.89% <100.00%> (+<0.01%) :arrow_up:
src/conversions.jl 84.94% <0.00%> (+0.16%) :arrow_up:

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 3507106...84c920d. Read the comment docs.

kimikage commented 3 years ago
julia> @btime color_type(ARGB)(g) setup=(g=rand());
  28.083 ns (0 allocations: 0 bytes) # v0.10.8-v0.10.10
  1.099 ns (0 allocations: 0 bytes)  # this PR
kimikage commented 3 years ago

If we are going to merge PR #232 as it stands, or if we can make an improved version soon, I will release v0.10.11 with this and PR #232.

kimikage commented 3 years ago

Julia's compiler is too hard to understand. :confused:

kimikage commented 3 years ago

I think this is potentially fragile. However, since its negative effect is a failure of inference and constant propagation, I think solving the problem we know now is more important than the potential problems.