Skip to content

Commit 32373c7

Browse files
authored
Fix overflow errors in division by Normed/Fixed (#168)
This also removes some of the optimization using the reciprocals.
1 parent efa9959 commit 32373c7

File tree

2 files changed

+41
-15
lines changed

2 files changed

+41
-15
lines changed

src/ColorVectorSpace.jl

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ end
9090
multype(::Type{A}, ::Type{B}) where {A,B} = coltype(typeof(zero(A)*zero(B)))
9191
sumtype(::Type{A}, ::Type{B}) where {A,B} = coltype(typeof(zero(A)+zero(B)))
9292
divtype(::Type{A}, ::Type{B}) where {A,B} = coltype(typeof(zero(A)/oneunit(B)))
93+
divtype(::Type{A}, ::Type{B}) where {A,B<:FixedPoint} = coltype(typeof(zero(A)/typemax(B)))
9394
powtype(::Type{A}, ::Type{B}) where {A,B} = coltype(typeof(zero(A)^zero(B)))
9495
sumtype(a::Colorant, b::Colorant) = coltype(sumtype(eltype(a),eltype(b)))
9596

@@ -131,6 +132,7 @@ rettype(::typeof(-), a, b) = parametric(color_rettype(a, b), sumtype(a, b))
131132
rettype(::typeof(*), a, b) = parametric(color_rettype(a, b), multype(eltype(a), eltype(b))) # gray * gray
132133
rettype(::typeof(*), a::Real, b) = arith_colorant_type(b){multype(typeof(a), eltype(b))}
133134
rettype(::typeof(/), a, b::Real) = arith_colorant_type(a){divtype(eltype(a), typeof(b))}
135+
rettype(::typeof(/), a, b) = arith_colorant_type(a){divtype(eltype(a), eltype(b))}
134136
rettype(::typeof(^), a, b) = arith_colorant_type(a){powtype(eltype(a), typeof(b))}
135137
rettype(::typeof(^), a, b::Integer) = arith_colorant_type(a){powtype(eltype(a), Int)}
136138

@@ -155,8 +157,21 @@ end
155157
_mul(x::T, y::T) where {T} = x * y
156158
_mul(x, y) = (T = multype(typeof(x), typeof(y)); _mul(T(x), T(y)))
157159

158-
_mapc(::Type{C}, f, c) where {C<:MathTypes} = C(f.(channels(c))...)
159-
_mapc(::Type{C}, f, a, b) where {C<:MathTypes} = C(f.(channels(a), channels(b))...)
160+
function _div(x::T, y::T) where {T <: FixedPoint}
161+
F = floattype(T)
162+
# range check should be done in the color constructor
163+
F(reinterpret(x)) / F(reinterpret(y))
164+
end
165+
function _div(x::AbstractFloat, y::Normed)
166+
F = divtype(typeof(x), typeof(y))
167+
F(x) * F(reinterpret(oneunit(y))) / F(reinterpret(y))
168+
end
169+
_div(x::AbstractFloat, y::Integer) = _mul(x, oneunit(x) / y)
170+
_div(x::T, y::T) where {T} = x / y
171+
_div(x, y) = (T = divtype(typeof(x), typeof(y)); _div(T(x), T(y)))
172+
173+
@inline _mapc(::Type{C}, f, c) where {C<:MathTypes} = C(f.(channels(c))...)
174+
@inline _mapc(::Type{C}, f, a, b) where {C<:MathTypes} = C(f.(channels(a), channels(b))...)
160175

161176
## Generic algorithms
162177
Base.add_sum(c1::MathTypes,c2::MathTypes) = mapc(Base.add_sum, c1, c2)
@@ -202,12 +217,11 @@ complement(x::TransparentColor) = typeof(x)(complement(color(x)), alpha(x))
202217
copy(c::MathTypes) = c
203218
(*)(f::Real, c::MathTypes) = _mapc(rettype(*, f, c), v -> _mul(f, v), c)
204219
(*)(c::MathTypes, f::Real) = (*)(f, c)
220+
(/)(c::MathTypes, f::Real) = _mapc(rettype(/, c, f), v -> _div(v, f), c)
205221
(+)(c::MathTypes) = mapc(+, c)
206222
(+)(c::MathTypes{Bool}) = c
207223
(-)(c::MathTypes) = mapc(-, c)
208224
(-)(c::MathTypes{Bool}) = c
209-
(/)(c::MathTypes, f::Real) = (one(f)/f)*c
210-
(/)(c::MathTypes, f::Integer) = (one(eltype(c))/f)*c
211225
abs(c::MathTypes) = mapc(abs, c)
212226
norm(c::MathTypes, p::Real=2) = (cc = channels(c); norm(cc, p)/(p == 0 ? length(cc) : length(cc)^(1/p)))
213227
()(a::C, b::C) where {C<:MathTypes} = _mapc(rettype(*, a, b), _mul, a, b)
@@ -223,13 +237,9 @@ norm(c::MathTypes, p::Real=2) = (cc = channels(c); norm(cc, p)/(p == 0 ? length(
223237

224238

225239
# Scalar RGB
226-
function (/)(c::AbstractRGB{T}, f::Real) where T<:Normed
227-
fs = (one(f)/reinterpret(oneunit(T)))/f
228-
rettype(/, c, f)(fs*reinterpret(red(c)), fs*reinterpret(green(c)), fs*reinterpret(blue(c)))
229-
end
230-
function (/)(c::AbstractRGB{T}, f::Integer) where T<:Normed
231-
fs = (1/reinterpret(oneunit(T)))/f
232-
rettype(/, c, f)(fs*reinterpret(red(c)), fs*reinterpret(green(c)), fs*reinterpret(blue(c)))
240+
function (/)(c::C, f::AbstractFloat) where {C<:Union{AbstractRGB, TransparentRGB}}
241+
r = oneunit(divtype(eltype(c), typeof(f))) / f
242+
_mapc(rettype(/, c, f), v -> v * r, c)
233243
end
234244
(+)(a::AbstractRGB, b::AbstractRGB) = rettype(+, a, b)(red(a)+red(b), green(a)+green(b), blue(a)+blue(b))
235245
(-)(a::AbstractRGB, b::AbstractRGB) = rettype(-, a, b)(red(a)-red(b), green(a)-green(b), blue(a)-blue(b))
@@ -279,16 +289,15 @@ end
279289
middle(c::AbstractGray) = arith_colorant_type(c)(middle(gray(c)))
280290
middle(x::C, y::C) where {C<:AbstractGray} = arith_colorant_type(C)(middle(gray(x), gray(y)))
281291

282-
(/)(n::Number, c::AbstractGray) = base_color_type(c)(n/gray(c))
292+
(/)(n::Number, c::AbstractGray) = base_color_type(c)(_div(real(n), gray(c)))
283293
(+)(a::AbstractGray, b::AbstractGray) = rettype(+, a, b)(gray(a)+gray(b))
284294
(+)(a::TransparentGray, b::TransparentGray) = rettype(+, a, b)(gray(a)+gray(b), alpha(a)+alpha(b))
285295
(-)(a::AbstractGray, b::AbstractGray) = rettype(-, a, b)(gray(a)-gray(b))
286296
(-)(a::TransparentGray, b::TransparentGray) = rettype(-, a, b)(gray(a)-gray(b), alpha(a)-alpha(b))
287297
(*)(a::AbstractGray, b::AbstractGray) = a b
288298
(^)(a::AbstractGray, b::Integer) = rettype(^, a, b)(gray(a)^convert(Int,b))
289299
(^)(a::AbstractGray, b::Real) = rettype(^, a, b)(gray(a)^b)
290-
(/)(a::C, b::C) where C<:AbstractGray = base_color_type(C)(gray(a)/gray(b))
291-
(/)(a::AbstractGray, b::AbstractGray) = /(promote(a, b)...)
300+
(/)(a::AbstractGray, b::AbstractGray) = rettype(/, a, b)(_div(gray(a), gray(b)))
292301
(+)(a::AbstractGray, b::Number) = base_color_type(a)(gray(a)+b)
293302
(+)(a::Number, b::AbstractGray) = b+a
294303
(-)(a::AbstractGray, b::Number) = base_color_type(a)(gray(a)-b)

test/runtests.jl

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ ColorTypes.comp2(c::RGBA32) = alpha(c)
129129
@test @inferred(cf/2.0f0) === Gray{Float32}(0.05)
130130
@test @inferred(cu/2) === Gray(cu.val/2)
131131
@test @inferred(cu/0.5f0) === Gray(cu.val/0.5f0)
132+
@test @inferred(cu/0.1N0f8) === Gray{N0f8}(1) # issue #154
133+
@test_colortype_approx_eq @inferred(cf/0.6N0f8) Gray{Float32}(1/6) # issue #154
134+
@test @inferred(Gray{Q0f7}(0.25) / 0.5Q0f7) === Gray{Q0f7}(0.5) # issue #154
135+
@test @inferred(1+0im / Gray(1)) === @inferred(1 / Gray(1)) === Gray{Float32}(1)
132136
@test @inferred(cf+cf) === ccmp
133137
@test isfinite(cf)
134138
@test isfinite(Gray(true))
@@ -300,6 +304,12 @@ ColorTypes.comp2(c::RGBA32) = alpha(c)
300304
@test_colortype_approx_eq ([p1]/2)[1] GrayA{Float32}(Gray(0.4),0.1)
301305
@test_colortype_approx_eq (0.4f0*[p1]+0.6f0*[p2])[1] GrayA{Float32}(Gray(0.68),0.26)
302306

307+
cf = AGray{Float32}(0.8, 0.2)
308+
cu = AGray{N0f8}(0.8, 0.2)
309+
@test @inferred(cu / 0.8N0f8) === AGray{N0f8}(1, 0.25) # issue #154
310+
@test_colortype_approx_eq @inferred(cf / 0.8N0f8) AGray{Float32}(1, 0.25) # issue #154
311+
@test @inferred(AGray{Q0f7}(0.25, 0.125) / 0.5Q0f7) === AGray{Q0f7}(0.5, 0.25) # issue #154
312+
303313
a = GrayA{N0f8}[GrayA(0.8,0.7), GrayA(0.5,0.2)]
304314
@test sum(a) == GrayA(n8sum(0.8,0.5), n8sum(0.7,0.2))
305315
@test isapprox(a, a)
@@ -328,7 +338,6 @@ ColorTypes.comp2(c::RGBA32) = alpha(c)
328338
@test @inferred(GrayA32(1, 0.4) - GrayA32(0.2, 0.2)) === GrayA32(0.8, 0.2)
329339

330340
# Multiplication
331-
cf = AGray{Float32}(0.8, 0.2)
332341
@test_throws MethodError cf * cf
333342
@test_throws MethodError cf cf
334343
@test_throws MethodError cf cf
@@ -357,6 +366,10 @@ ColorTypes.comp2(c::RGBA32) = alpha(c)
357366
@test cf/2.0f0 == RGB{Float32}(0.05,0.1,0.15)
358367
@test cu/2 RGB(cu.r/2,cu.g/2,cu.b/2)
359368
@test cu/0.5f0 RGB(cu.r/0.5f0, cu.g/0.5f0, cu.b/0.5f0)
369+
@test @inferred(cu/0.4N0f8) === RGB{N0f8}(26/102, 51/102, 76/102)
370+
@test_colortype_approx_eq @inferred(cf / 0.4N0f8) RGB{Float32}(0.25, 0.5, 0.75)
371+
cq0f7 = RGB{Q0f7}(0.125, 0.25, 0.375)
372+
@test @inferred(cq0f7 / 0.5Q0f7) === RGB{Q0f7}(0.25, 0.5, 0.75) # issue #154
360373
@test cf+cf == ccmp
361374
@test cu * 1//2 == mapc(x->Float64(Rational(x)/2), cu)
362375
@test_colortype_approx_eq (cf.*[0.8f0])[1] RGB{Float32}(0.8*0.1,0.8*0.2,0.8*0.3)
@@ -472,6 +485,10 @@ ColorTypes.comp2(c::RGBA32) = alpha(c)
472485
@test cf/2.0f0 == RGBA{Float32}(0.05,0.1,0.15,0.2)
473486
@test cu/2 == RGBA(cu.r/2,cu.g/2,cu.b/2,cu.alpha/2)
474487
@test cu/0.5f0 == RGBA(cu.r/0.5f0, cu.g/0.5f0, cu.b/0.5f0, cu.alpha/0.5f0)
488+
@test @inferred(cu/0.4N0f8) === RGBA{N0f8}(26/102, 51/102, 76/102, 102/102)
489+
@test_colortype_approx_eq @inferred(cf / 0.4N0f8) RGBA{Float32}(0.25, 0.5, 0.75, 1)
490+
cq0f7 = RGBA{Q0f7}(0.125, 0.25, 0.375, 0.4375)
491+
@test @inferred(cq0f7 / 0.5Q0f7) === RGBA{Q0f7}(0.25, 0.5, 0.75, 0.875) # issue #154
475492
@test cf+cf == ccmp
476493
@test_colortype_approx_eq (cf.*[0.8f0])[1] RGBA{Float32}(0.8*0.1,0.8*0.2,0.8*0.3,0.8*0.4)
477494
@test_colortype_approx_eq ([0.8f0].*cf)[1] RGBA{Float32}(0.8*0.1,0.8*0.2,0.8*0.3,0.8*0.4)

0 commit comments

Comments
 (0)