From 1d1b69696de2ae9fbb21e5e8b5b77bab4ea3486f Mon Sep 17 00:00:00 2001 From: Steve Kelly Date: Tue, 1 Dec 2015 21:35:48 -0500 Subject: [PATCH 1/4] make getindex(::Array, ::Face) return a tuple since it is of deterministic length. Also provide unsafe version for `inbounds`ing --- src/faces.jl | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/faces.jl b/src/faces.jl index 418c068..533409f 100644 --- a/src/faces.jl +++ b/src/faces.jl @@ -1,6 +1,25 @@ -function getindex{T,N,FD,FT,Offset}(a::Array{T,N}, i::Face{FD, FT, Offset}) - a[[(map(Int,i)-Offset)...]] +""" +Given an Array, `a`, and a face, `f`, return a tuple of numbers +interpreting the values and offset of the face as indices into `A`. +""" +@generated function getindex{T,N,FD,FT,Offset}(a::Array{T,N}, + f::Face{FD, FT, Offset}) + v = Expr(:tuple) + for i = 1:FD + push!(v.args, Expr(:ref, :a, :(f[$i]-Offset))) + end + Expr(:(::), v, :(NTuple{FD,T})) end + +@generated function Base.unsafe_getindex{T,N,FD,FT,Offset}(a::Array{T,N}, + f::Face{FD, FT, Offset}) + v = Expr(:tuple) + for i = 1:FD + push!(v.args, Expr(:call, Base.unsafe_getindex, :a, :(f[$i]-Offset))) + end + Expr(:(::), v, :(NTuple{FD,T})) +end + function setindex!{T,N,FD,FT,Offset}(a::Array{T,N}, b::Array{T,N}, i::Face{FD, FT, Offset}) a[[(map(Int,i)-Offset)...]] = b end From 36ae4a90ba23e3b565a2905d4fbfe8127918536a Mon Sep 17 00:00:00 2001 From: Steve Kelly Date: Fri, 4 Dec 2015 00:38:47 -0500 Subject: [PATCH 2/4] make unsafe getindex the default for face types and add tests --- src/faces.jl | 14 ++++---------- test/faces.jl | 6 ++++++ test/runtests.jl | 1 + 3 files changed, 11 insertions(+), 10 deletions(-) create mode 100644 test/faces.jl diff --git a/src/faces.jl b/src/faces.jl index 533409f..52e26c9 100644 --- a/src/faces.jl +++ b/src/faces.jl @@ -1,17 +1,11 @@ """ Given an Array, `a`, and a face, `f`, return a tuple of numbers interpreting the values and offset of the face as indices into `A`. -""" -@generated function getindex{T,N,FD,FT,Offset}(a::Array{T,N}, - f::Face{FD, FT, Offset}) - v = Expr(:tuple) - for i = 1:FD - push!(v.args, Expr(:ref, :a, :(f[$i]-Offset))) - end - Expr(:(::), v, :(NTuple{FD,T})) -end -@generated function Base.unsafe_getindex{T,N,FD,FT,Offset}(a::Array{T,N}, +Note: This is not bounds checked. It is recommended that you use +`checkbounds` to confirm the indices are safe for loops. +""" +@generated function Base.getindex{T,N,FD,FT,Offset}(a::Array{T,N}, f::Face{FD, FT, Offset}) v = Expr(:tuple) for i = 1:FD diff --git a/test/faces.jl b/test/faces.jl new file mode 100644 index 0000000..cba377e --- /dev/null +++ b/test/faces.jl @@ -0,0 +1,6 @@ +context("faces") do + a = [1,2,3,4] + @fact a[Face{3,Int,0}(1,2,3)] --> (1,2,3) + @fact a[Face{3,Int,-1}(0,1,2)] --> (1,2,3) + @fact a[Face{3,Int,1}(2,3,4)] --> (1,2,3) +end diff --git a/test/runtests.jl b/test/runtests.jl index 77a11e6..f96dd12 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -5,6 +5,7 @@ typealias Vec3f0 Vec{3, Float32} facts("GeometryTypes") do include("hyperrectangles.jl") + include("faces.jl") include("meshtypes.jl") include("distancefields.jl") include("primitives.jl") From b0078482245ebed9b2d670f6bb61149bf115241c Mon Sep 17 00:00:00 2001 From: Steve Kelly Date: Fri, 4 Dec 2015 18:20:41 -0500 Subject: [PATCH 3/4] add checkbounds for Mesh, cause ya' know, safety is important and stuff. --- src/GeometryTypes.jl | 1 + src/checkbounds.jl | 19 +++++++++++++++++++ src/faces.jl | 2 ++ test/meshtypes.jl | 19 +++++++++++++++++++ 4 files changed, 41 insertions(+) create mode 100644 src/checkbounds.jl diff --git a/src/GeometryTypes.jl b/src/GeometryTypes.jl index 1d8f459..cf24e2b 100644 --- a/src/GeometryTypes.jl +++ b/src/GeometryTypes.jl @@ -42,6 +42,7 @@ include("display.jl") include("slice.jl") include("decompose.jl") include("deprecated.jl") +include("checkbounds.jl") export AABB, AbsoluteRectangle, diff --git a/src/checkbounds.jl b/src/checkbounds.jl new file mode 100644 index 0000000..c328be3 --- /dev/null +++ b/src/checkbounds.jl @@ -0,0 +1,19 @@ +""" +Check the `Face` indices to ensure they are in the bounds of the vertex +array of the `AbstractMesh`. +""" +function Base.checkbounds{VT,FT,FD,FO}(m::AbstractMesh{VT,Face{FD,FT,FO}}) + isempty(faces(m)) && return true # nothing to worry about I guess + + # index max and min + const i = 1 + FO # normalize face offset + s = length(vertices(m)) + FO + + for face in faces(m) + # I hope this is unrolled + for elt in face + i <= elt && elt <= s || return false + end + end + return true +end diff --git a/src/faces.jl b/src/faces.jl index 52e26c9..6e90d12 100644 --- a/src/faces.jl +++ b/src/faces.jl @@ -4,6 +4,8 @@ interpreting the values and offset of the face as indices into `A`. Note: This is not bounds checked. It is recommended that you use `checkbounds` to confirm the indices are safe for loops. +Also be aware when writing generic code that faces may be of more than +3 vertices. """ @generated function Base.getindex{T,N,FD,FT,Offset}(a::Array{T,N}, f::Face{FD, FT, Offset}) diff --git a/test/meshtypes.jl b/test/meshtypes.jl index 2112cfe..6d9c7c7 100644 --- a/test/meshtypes.jl +++ b/test/meshtypes.jl @@ -85,4 +85,23 @@ context("Slice") do @fact length(s2) --> length(exp2) end +context("checkbounds") do + m1 = HomogenousMesh([Point{3,Float64}(0.0,0.0,10.0), + Point{3,Float64}(0.0,10.0,10.0), + Point{3,Float64}(0.0,0.0,0.0)], + [Face{3,Int,0}(1,2,3)]) + @fact checkbounds(m1) --> true + m2 = HomogenousMesh([Point{3,Float64}(0.0,0.0,10.0), + Point{3,Float64}(0.0,10.0,10.0), + Point{3,Float64}(0.0,0.0,0.0)], + [Face{3,Int,-1}(1,2,3)]) + @fact checkbounds(m2) --> false + # empty case + m3 = HomogenousMesh([Point{3,Float64}(0.0,0.0,10.0), + Point{3,Float64}(0.0,10.0,10.0), + Point{3,Float64}(0.0,0.0,0.0)], + Face{3,Int,-1}[]) + @fact checkbounds(m3) --> true +end + end From 3246b450fc1c9852c7a02d7d9ad260f1ac020f5d Mon Sep 17 00:00:00 2001 From: Steve Kelly Date: Fri, 4 Dec 2015 19:03:30 -0500 Subject: [PATCH 4/4] checkbounds: 1 -> one(T) --- src/checkbounds.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/checkbounds.jl b/src/checkbounds.jl index c328be3..3b1d20b 100644 --- a/src/checkbounds.jl +++ b/src/checkbounds.jl @@ -6,7 +6,7 @@ function Base.checkbounds{VT,FT,FD,FO}(m::AbstractMesh{VT,Face{FD,FT,FO}}) isempty(faces(m)) && return true # nothing to worry about I guess # index max and min - const i = 1 + FO # normalize face offset + const i = one(FO) + FO # normalize face offset s = length(vertices(m)) + FO for face in faces(m)