-
Notifications
You must be signed in to change notification settings - Fork 51
Update _in for Cone and Torus
#575
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
Conversation
fhagemann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I mostly have code style changes (which might also be a bit unfair to put on you, because I can also see that they were there already before this PR - feel free to ignore to avoid wasting time)
|
No problem at all. I’m happy to make any code style changes so feel free to leave your comments and I’ll review them tomorrow |
fhagemann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Just one minor comment:
|
I added the change and tests to fix |
|
Yeah, I looked at this yesterday, but it was a bit more complicated than I thought. |
|
Yes, that's in now... I don't understand why some checks were failing though |
|
Small update: I think I have code now that should (in principle) give us what we want (adding or subtracting a fraction of ta volume using Closed primitive adding
|
|
Thanks, Felix! This looks great. Regarding the third case, is the larger tolerance here intentional, or should it be the same across all three cases? |
|
Yes, I increased |
|
I'm gonna add plots as comments into the code to keep this tidy. |
fhagemann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the full code I used to create the plots shown in this review (replacing the first few lines as indicated):
# Cylinder
cc = CSG.Cone{Float64}(r = 0.5, hZ = 0.5)
@assert cc isa CSG.Cylinder{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
Δx = 0.005
rs = -0.7:Δx:0.7
zs = -0.7:Δx:0.7
rzc = Matrix{Symbol}(undef, length(rs), length(zs))
rzo = Matrix{Symbol}(undef, length(rs), length(zs))
for (ir,r) in enumerate(rs), (iz,z) in enumerate(zs)
pt = CartesianPoint{Float64}(0,r,z)
rzc[ir, iz] = if in(pt, cc, 0.0); :green; elseif in(pt, cc, csgtol); :orange; else; :red; end
rzo[ir, iz] = if in(pt, co, csgtol); :green; elseif in(pt, co, 0.0); :yellow; else; :red; end
end
ys = -0.7:Δx:0.7
xyc = Matrix{Symbol}(undef, length(ys), length(ys))
xyo = Matrix{Symbol}(undef, length(ys), length(ys))
for (ix,x) in enumerate(ys), (iy,y) in enumerate(ys)
pt = CartesianPoint{Float64}(x,y,0)
xyc[ix, iy] = if in(pt, cc, 0.0); :green; elseif in(pt, cc, csgtol); :orange; else; :red; end
xyo[ix, iy] = if in(pt, co, csgtol); :green; elseif in(pt, co, 0.0); :yellow; else; :red; end
end
plot(
scatter(vcat(fill(rs,length(zs))), vcat(fill.(zs, length(rs))), color = rzc, ratio = 1,
xlabel = "r / m", ylabel = "z / m", label = false, msw = 0, xlims = extrema(rs), ylims = extrema(zs)),
scatter(vcat(fill(rs,length(zs))), vcat(fill.(zs, length(rs))), color = rzo, ratio = 1,
xlabel = "r / m", ylabel = "z / m", label = false, msw = 0, xlims = extrema(rs), ylims = extrema(zs)),
scatter(vcat(fill(ys,length(ys))), vcat(fill.(ys, length(ys))), color = xyc, ratio = 1,
xlabel = "x / m", ylabel = "y / m", label = false, msw = 0, xlims = extrema(ys), ylims = extrema(ys)),
scatter(vcat(fill(ys,length(ys))), vcat(fill.(ys, length(ys))), color = xyo, ratio = 1,
xlabel = "x / m", ylabel = "y / m", label = false, msw = 0, xlims = extrema(ys), ylims = extrema(ys)),
layout = (2,2), size = (600,600), fmt = :png,
)| function _in(pt::CartesianPoint, c::Cylinder{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| az = abs(pt.z) | ||
| az <= c.hZ + csgtol && begin | ||
| r = hypot(pt.x, pt.y) | ||
| r <= c.r + csgtol | ||
| end | ||
| hypot(max(zero(T), hypot(pt.x, pt.y) - c.r), max(zero(T), pt.z - c.hZ, - pt.z - c.hZ)) <= csgtol | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint, c::Cylinder{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| abs(pt.z) < c.hZ - csgtol && | ||
| hypot(pt.x, pt.y) < c.r - csgtol | ||
| abs(pt.z) < c.hZ - csgtol && hypot(pt.x, pt.y) < c.r - csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### PartialCylinder | ||
|
|
||
| function _in(pt::CartesianPoint, c::PartialCylinder{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| az = abs(pt.z) | ||
| az <= c.hZ + csgtol && begin | ||
| r = hypot(pt.x, pt.y) | ||
| r <= c.r + csgtol && | ||
| _in_angular_interval_closed(atan(pt.y, pt.x), c.φ, csgtol = csgtol / r) | ||
| function _in(pt::CartesianPoint, c::PartialCylinder{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| z::T = pt.z | ||
| abs(z) <= c.hZ + csgtol && begin | ||
| det::T = csgtol^2 - max(zero(T), z - c.hZ, - z - c.hZ)^2 | ||
| det >= 0 && (_in_angular_interval_closed(φ, c.φ, csgtol = zero(T)) && r <= c.r + sqrt(det) || let Δφ = min(T(2π)-φ, φ-c.φ) | ||
| if Δφ <= atan(sqrt(det), c.r) | ||
| det >= c.r^2 * sin(Δφ)^2 && r <= c.r * cos(Δφ) + sqrt(det - c.r^2 * sin(Δφ)^2) | ||
| else | ||
| r <= sqrt(det)/sin(min(Δφ, T(π/2))) | ||
| end | ||
| end) | ||
| end | ||
| end | ||
| function _in(pt::CartesianPoint, c::PartialCylinder{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| abs(pt.z) + csgtol < c.hZ && begin | ||
| r = hypot(pt.x, pt.y) | ||
| csgtol + r < c.r && | ||
| _in_angular_interval_open(atan(pt.y, pt.x), c.φ, csgtol = csgtol / r) | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint, c::PartialCylinder{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| abs(pt.z) + csgtol < c.hZ && r < c.r - csgtol && _in_angular_interval_closed(φ, c.φ, csgtol = zero(T)) && r * sin(min(φ, c.φ - φ, T(π/2))) > csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# PartialCylinder
cc = CSG.Cone{Float64}(r = 0.5, φ = 2, hZ = 0.5)
@assert cc isa CSG.PartialCylinder{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.1
# PartialCylinder
cc = CSG.Cone{Float64}(r = 0.5, φ = 7π/4, hZ = 0.5)
@assert cc isa CSG.PartialCylinder{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.1
| ### VaryingCylinder | ||
| function _in(pt::CartesianPoint, c::VaryingCylinder{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| az = abs(pt.z) | ||
| az <= c.hZ + csgtol && begin | ||
| r = hypot(pt.x, pt.y) | ||
| rz = radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) | ||
| r <= rz + csgtol | ||
| end | ||
| r::T = hypot(pt.x, pt.y) | ||
| z::T = pt.z | ||
| rz::T = radius_at_z(c.hZ, c.r[1][1], c.r[2][1], z) | ||
| Δr::T = c.r[2][1] - c.r[1][1] | ||
| Δz::T = iszero(c.hZ) ? zero(T) : -csgtol * Δr / hypot(2*c.hZ, Δr) | ||
| return (abs(z - Δz) < c.hZ && r <= rz + csgtol * hypot(2*c.hZ, Δr) / (2*c.hZ)) || | ||
| ((z + c.hZ)^2 <= csgtol^2 && r <= c.r[1][1] + sqrt(csgtol^2 - (z + c.hZ)^2)) || | ||
| ((z - c.hZ)^2 <= csgtol^2 && r <= c.r[2][1] + sqrt(csgtol^2 - (z - c.hZ)^2)) | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint, c::VaryingCylinder{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| abs(pt.z) + csgtol < c.hZ && | ||
| csgtol + hypot(pt.x, pt.y) < radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) | ||
| abs(pt.z) < c.hZ - csgtol && | ||
| hypot(pt.x, pt.y) < radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) - csgtol * hypot(2*c.hZ, c.r[2][1] - c.r[1][1]) / (2*c.hZ) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### PartialVaryingCylinder | ||
| function _in(pt::CartesianPoint, c::PartialVaryingCylinder{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| az = abs(pt.z) | ||
| az <= c.hZ + csgtol && begin | ||
| r = hypot(pt.x, pt.y) | ||
| rz = radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) | ||
| r <= rz + csgtol && | ||
| _in_angular_interval_closed(atan(pt.y, pt.x), c.φ, csgtol = csgtol / r) | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| z::T = pt.z | ||
| rz::T = radius_at_z(c.hZ, c.r[1][1], c.r[2][1], z) | ||
| Δr::T = c.r[2][1] - c.r[1][1] | ||
| abs(z) <= c.hZ + csgtol && if _in_angular_interval_closed(φ, c.φ, csgtol = zero(T)) | ||
| _in(pt, Cone{T,ClosedPrimitive}(c.r, nothing, c.hZ, c.origin, c.rotation); csgtol) | ||
| else | ||
| t::T = ((r - c.r[1][1]) * Δr + (z + c.hZ) * 2c.hZ) / (Δr^2 + 4c.hZ^2) | ||
| s::T = ((r - c.r[1][1]) * 2c.hZ - (z + c.hZ) * Δr) / (Δr^2 + 4c.hZ^2) | ||
| d::T = if r <= rz && abs(z) <= c.hZ | ||
| zero(T) | ||
| elseif 0 <= t <= 1 && s >= 0 | ||
| s * hypot(Δr, 2c.hZ) | ||
| elseif t > 1 || (t >= 0 && s <= 0 && Δr > 0) | ||
| hypot(abs(z - c.hZ), max(zero(T), r - c.r[2][1])) | ||
| elseif t < 0 || (t <= 1 && s <= 0 && Δr < 0) | ||
| hypot(abs(-z - c.hZ), max(zero(T), r - c.r[1][1])) | ||
| else | ||
| hypot(r, max(zero(T), pt.z - c.hZ, - pt.z - c.hZ)) | ||
| end | ||
| (r * sin(min(T(2π)-φ, φ-c.φ, T(π/2))))^2 + d^2 <= csgtol^2 | ||
| end | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint, c::PartialVaryingCylinder{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| abs(pt.z) + csgtol < c.hZ && begin | ||
| r = hypot(pt.x, pt.y) | ||
| csgtol + r < radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) && | ||
| _in_angular_interval_open(atan(pt.y, pt.x), c.φ, csgtol = csgtol / r) | ||
| end | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| Δr::T = c.r[2][1] - c.r[1][1] | ||
| abs(pt.z) + csgtol < c.hZ && | ||
| r < radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) - csgtol * hypot(2c.hZ, Δr) / 2c.hZ && | ||
| _in_angular_interval_open(atan(pt.y, pt.x), c.φ, csgtol = zero(T)) && r * sin(min(φ, c.φ - φ, T(π/2))) > csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# PartialVaryingCylinder
cc = CSG.Cone{Float64}(r = ((0.2,0.5), (0.1,0.3)), φ = 2, hZ = 0.1)
@assert cc isa CSG.PartialVaryingTube{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
# PartialVaryingCylinder
cc = CSG.Cone{Float64}(r = ((0.2,0.5), (0.1,0.3)), φ = 7π/4, hZ = 0.1)
@assert cc isa CSG.PartialVaryingTube{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
| ### VaryingTube | ||
| function _in(pt::CartesianPoint, c::VaryingTube{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| r::T = hypot(pt.x, pt.y) | ||
| z::T = pt.z | ||
| r_in::T = radius_at_z(c.hZ, c.r[1][1], c.r[2][1], z) | ||
| r_out::T = radius_at_z(c.hZ, c.r[1][2], c.r[2][2], z) | ||
| Δr_in::T = c.r[2][1] - c.r[1][1] | ||
| Δr_out::T = c.r[2][2] - c.r[1][2] | ||
| Δz_in::T = iszero(c.hZ) ? zero(T) : csgtol * Δr_in / hypot(2*c.hZ, Δr_in) | ||
| Δz_out::T = iszero(c.hZ) ? zero(T) : csgtol * Δr_out / hypot(2*c.hZ, Δr_out) | ||
| return (abs(z - Δz_in) < c.hZ && r >= r_in - csgtol * hypot(2*c.hZ, Δr_in) / (2*c.hZ) || | ||
| ((z + c.hZ)^2 <= csgtol^2 && r >= c.r[1][1] - sqrt(csgtol^2 - (z + c.hZ)^2)) || | ||
| ((z - c.hZ)^2 <= csgtol^2 && r >= c.r[2][1] - sqrt(csgtol^2 - (z - c.hZ)^2))) && | ||
| (abs(z + Δz_out) < c.hZ && r <= r_out + csgtol * hypot(2*c.hZ, Δr_out) / (2*c.hZ) || | ||
| ((z + c.hZ)^2 <= csgtol^2 && r <= c.r[1][2] + sqrt(csgtol^2 - (z + c.hZ)^2)) || | ||
| ((z - c.hZ)^2 <= csgtol^2 && r <= c.r[2][2] + sqrt(csgtol^2 - (z - c.hZ)^2))) | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint, c::VaryingTube{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| r::T = hypot(pt.x, pt.y) | ||
| return abs(pt.z) + csgtol < c.hZ && | ||
| r > radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) + csgtol * hypot(2*c.hZ, c.r[2][1] - c.r[1][1]) / (2*c.hZ) && | ||
| r < radius_at_z(c.hZ, c.r[1][2], c.r[2][2], pt.z) - csgtol * hypot(2*c.hZ, c.r[2][2] - c.r[1][2]) / (2*c.hZ) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### PartialVaryingTube | ||
| function _in(pt::CartesianPoint, c::PartialVaryingTube{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| az = abs(pt.z) | ||
| az <= c.hZ + csgtol && begin | ||
| r = hypot(pt.x, pt.y) | ||
| r_in = radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) | ||
| r_out = radius_at_z(c.hZ, c.r[1][2], c.r[2][2], pt.z) | ||
| r_in - csgtol <= r && | ||
| r <= r_out + csgtol && | ||
| _in_angular_interval_closed(atan(pt.y, pt.x), c.φ, csgtol = csgtol / r) | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| z::T = pt.z | ||
| abs(z) <= c.hZ + csgtol && if _in_angular_interval_closed(atan(pt.y, pt.x), c.φ, csgtol = zero(T)) | ||
| _in(pt, Cone{T,ClosedPrimitive}(c.r, nothing, c.hZ, c.origin, c.rotation); csgtol) | ||
| else | ||
| r_in = clamp(radius_at_z(c.hZ, c.r[1][1], c.r[2][1], z), extrema((c.r[1][1], c.r[2][1]))...) | ||
| r_out = clamp(radius_at_z(c.hZ, c.r[1][2], c.r[2][2], z), extrema((c.r[1][2], c.r[2][2]))...) | ||
| Δr_in = c.r[2][1] - c.r[1][1] | ||
| Δr_out = c.r[2][2] - c.r[1][2] | ||
| t_in = ((r - c.r[1][1]) * Δr_in + (z + c.hZ) * 2c.hZ) / (Δr_in^2 + 4c.hZ^2) | ||
| s_in = -((r - c.r[1][1]) * 2c.hZ - (z + c.hZ) * Δr_in) / (Δr_in^2 + 4c.hZ^2) | ||
| t_out = ((r - c.r[1][2]) * Δr_out + (z + c.hZ) * 2c.hZ) / (Δr_out^2 + 4c.hZ^2) | ||
| s_out = ((r - c.r[1][2]) * 2c.hZ - (z + c.hZ) * Δr_out) / (Δr_out^2 + 4c.hZ^2) | ||
| d::T = if r_in <= r <= r_out && abs(z) <= c.hZ | ||
| zero(T) | ||
| elseif 0 <= t_in <= 1 && s_in >= 0 | ||
| s_in * hypot(Δr_in, 2c.hZ) | ||
| elseif 0 <= t_out <= 1 && s_out >= 0 | ||
| s_out * hypot(Δr_out, 2c.hZ) | ||
| elseif t_out > 1 || t_in > 1 | ||
| hypot(abs(z - c.hZ), max(0, r - c.r[2][2], c.r[2][1] - r)) | ||
| else # t_out < 0 || t_in < 0 | ||
| hypot(abs(-z - c.hZ), max(0, r - c.r[1][2], c.r[1][1] - r)) | ||
| end | ||
| (r * sin(min(T(2π)-φ, φ-c.φ, T(π/2))))^2 + d^2 <= csgtol^2 | ||
| end | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint, c::PartialVaryingTube{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| abs(pt.z) + csgtol < c.hZ && begin | ||
| r = hypot(pt.x, pt.y) | ||
| csgtol + radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) < r < radius_at_z(c.hZ, c.r[1][2], c.r[2][2], pt.z) - csgtol && | ||
| _in_angular_interval_open(atan(pt.y, pt.x), c.φ, csgtol = csgtol / r) | ||
| end | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| abs(pt.z) + csgtol < c.hZ && | ||
| _in_angular_interval_open(atan(pt.y, pt.x), c.φ, csgtol = zero(T)) && r * sin(min(φ, c.φ - φ, T(π/2))) > csgtol && | ||
| r > radius_at_z(c.hZ, c.r[1][1], c.r[2][1], pt.z) + csgtol * hypot(2c.hZ, c.r[2][1] - c.r[1][1]) / 2c.hZ && | ||
| r < radius_at_z(c.hZ, c.r[1][2], c.r[2][2], pt.z) - csgtol * hypot(2c.hZ, c.r[2][2] - c.r[1][2]) / 2c.hZ | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# PartialVaryingTube
cc = CSG.Cone{Float64}(r = ((0.2,0.5), (0.3,0.4)), φ = 2, hZ = 0.5)
@assert cc isa CSG.PartialVaryingTube{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.1
# PartialVaryingTube
cc = CSG.Cone{Float64}(r = ((0.2,0.5), (0.3,0.4)), φ = 7π/4, hZ = 0.5)
@assert cc isa CSG.PartialVaryingTube{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.1
| function _in(pt::CartesianPoint, c::Cone{T,CO,Tuple{Tuple{Nothing,T},Nothing}}; csgtol::T = csg_default_tol(T)) where {T,CO} | ||
| _in(pt, Cone{T,CO}(((c.r[1][2],), (zero(T),)), c.φ, c.hZ, c.origin, c.rotation); csgtol) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function _in(pt::CartesianPoint, c::Cone{T,CO,Tuple{Nothing,Tuple{Nothing,T}}}; csgtol::T = csg_default_tol(T)) where {T,CO} | ||
| _in(pt, Cone{T,CO}(((zero(T),), (c.r[2][2],)), c.φ, c.hZ, c.origin, c.rotation); csgtol) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function _in(pt::CartesianPoint, c::Cone{T,CO,Tuple{Tuple{T,T},T}}; csgtol::T = csg_default_tol(T)) where {T,CO} | ||
| _in(pt, Cone{T,CO}((c.r[1], (c.r[2], c.r[2])), c.φ, c.hZ, c.origin, c.rotation); csgtol) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function _in(pt::CartesianPoint, c::Cone{T,CO,Tuple{T,Tuple{T,T}}}; csgtol::T = csg_default_tol(T)) where {T,CO} | ||
| _in(pt, Cone{T,CO}(((c.r[1], c.r[1]), c.r[2]), c.φ, c.hZ, c.origin, c.rotation); csgtol) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @test_broken CSG._in(CartesianPoint(0.0, 0.0, -1.0), p_tube_closed) | ||
| @test_broken CSG._in(CartesianPoint(0.8, 0.0, -1.0), p_tube_closed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests were never broken.
| # PartialTopClosedTube (ClosedPrimitive) | ||
| p_tube_closed = CSG.PartialTopClosedTube{Float64, CSG.ClosedPrimitive}(r_cone, phi_partial, hZ, origin, rotation) | ||
| @test CSG._in(CartesianPoint(1.0, 0.0, 1.0), p_tube_closed) | ||
| @test_broken CSG._in(CartesianPoint(0.0, 0.0, 1.0), p_tube_closed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was never broken.
| # PartialTopClosedTube (ClosedPrimitive) | ||
| p_tube_closed = CSG.PartialTopClosedTube{Float64, CSG.ClosedPrimitive}(r_cone, phi_partial, hZ, origin, rotation) | ||
| @test CSG._in(CartesianPoint(1.0, 0.0, 1.0), p_tube_closed) | ||
| @test_broken CSG._in(CartesianPoint(0.0, 0.0, 1.0), p_tube_closed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was never broken.
| @test_broken CSG._in(CartesianPoint(0.0, 0.0, -1.0), p_tube_closed) | ||
| @test_broken CSG._in(CartesianPoint(0.8, 0.0, -1.0), p_tube_closed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests were never broken.
| _parse_value(T, dict["h"], length_unit) / 2 | ||
| end | ||
|
|
||
| # TODO: throw error if hZ == 0 and different radii at top and bottom ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was getting weird results when defining a tube with height 0, so we might wanna think about throwing an error if this happens. But I wrote the code such that this should not be an issue anymore.
|
@claudiaalvgar This should be everything needed for |
fhagemann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, let's implement this also in this PR.
Not sure if this requires some additional tests to be written to cover all methods for _in with Torus.
| # FullTorus | ||
| function _in(pt::CartesianPoint{T}, t::FullTorus{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) <= t.r_tube + csgtol | ||
| end | ||
| function _in(pt::CartesianPoint{T}, t::Torus{T,OpenPrimitive,Tuple{T,T}}; csgtol::T = csg_default_tol(T)) where {T} | ||
| _r = hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) | ||
| rmin::T, rmax::T = _radial_endpoints(t.r_tube) | ||
| return rmin + csgtol < _r < rmax - csgtol && | ||
| (isnothing(t.φ) || _in_angular_interval_open(atan(pt.y, pt.x), t.φ, csgtol = csgtol)) && | ||
| (isnothing(t.θ) || _in_angular_interval_open(atan(pt.z, hypot(pt.x, pt.y) - t.r_torus), t.θ, csgtol = csgtol)) | ||
|
|
||
| function _in(pt::CartesianPoint{T}, t::FullTorus{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) < t.r_tube - csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function _in(pt::CartesianPoint{T}, t::FullThetaTorus{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| Δφ::T = max(min(T(2π)-φ, φ-t.φ), zero(T)) | ||
| _y::T, _x::T = hypot(pt.x, pt.y) .* sincos(Δφ) | ||
| _r::T = hypot(_x - t.r_torus, pt.z) | ||
| hypot(max(_r - t.r_tube, zero(T)), _y) <= csgtol | ||
| end | ||
|
|
||
| # Torus(r_torus, r_tubeMin, r_tubeMax, φMin, φMax, θMin, θMax, z) = Torus(;r_torus = r_torus, r_tubeMin = r_tubeMin, r_tubeMax = r_tubeMax, φMin = φMin, φMax = φMax, θMin = θMin, θMax = θMax, z = z) | ||
| function _in(pt::CartesianPoint{T}, t::FullThetaTorus{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| r::T = hypot(pt.x, pt.y) | ||
| φ::T = mod(atan(pt.y, pt.x), T(2π)) | ||
| hypot(r - t.r_torus, pt.z) < t.r_tube - csgtol && | ||
| _in_angular_interval_closed(φ, t.φ, csgtol = zero(T)) && | ||
| r * sin(min(φ, t.φ - φ, T(π/2))) > csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # FullPhiTorus | ||
| function _in(pt::CartesianPoint{T}, t::FullPhiTorus{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| _r::T = hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) | ||
| θ::T = mod(atan(pt.z, hypot(pt.x, pt.y) - t.r_torus) - t.θ[1], T(2π)) | ||
| Δθ::T = max(min(T(2π) - θ, θ - t.θ[2] + t.θ[1]), zero(T)) | ||
| Δθ <= atan(csgtol, t.r_tube) ? | ||
| csgtol^2 - t.r_tube^2 * sin(Δθ)^2 >= 0 && _r <= t.r_tube * cos(Δθ) + sqrt(csgtol^2 - t.r_tube^2 * sin(Δθ)^2) : | ||
| _r <= csgtol / sin(min(Δθ, T(π/2))) | ||
| end | ||
|
|
||
| # function RoundChamfer(r_torus::R1, r_tube::R2, z::TZ) where {R1<:Real, R2<:Real, TZ<:Real} | ||
| # T = float(promote_type(R1, R2, TZ)) | ||
| # Torus( T, T(r_torus), T(r_tube), nothing, T(0)..T(π/2), T(z)) | ||
| # end | ||
| function _in(pt::CartesianPoint{T}, t::FullPhiTorus{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| _r::T = hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) | ||
| θ::T = mod(atan(pt.z, hypot(pt.x, pt.y) - t.r_torus) - t.θ[1], T(2π)) | ||
| Δθ::T = max(min(T(2π) - θ, θ - t.θ[2] + t.θ[1]), zero(T)) | ||
| _r < t.r_tube - csgtol && _in_angular_interval_closed(θ, t.θ[2] - t.θ[1], csgtol = zero(T)) && _r * sin(min(θ, t.θ[2] - t.θ[1] - θ, T(π/2))) > csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# FullPhiTorus
cc = CSG.Torus{Float64}(r_tube = 0.2, r_torus = 0.3, θ = (0,2π/3))
@assert cc isa CSG.FullPhiTorus{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
# FullPhiTorus
cc = CSG.Torus{Float64}(r_tube = 0.2, r_torus = 0.3, θ = (0,4π/3))
@assert cc isa CSG.FullPhiTorus{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
| # HollowTorus | ||
| function _in(pt::CartesianPoint{T}, t::HollowTorus{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| t.r_tube[1] - csgtol <= hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) <= t.r_tube[2] + csgtol | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint{T}, t::HollowTorus{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| t.r_tube[1] + csgtol < hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) < t.r_tube[2] - csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # HollowPhiTorus | ||
| function _in(pt::CartesianPoint{T}, t::HollowPhiTorus{T,ClosedPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| _r::T = hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) | ||
| θ::T = mod(atan(pt.z, hypot(pt.x, pt.y) - t.r_torus) - t.θ[1], T(2π)) | ||
| Δθ::T = max(min(T(2π) - θ, θ - t.θ[2] + t.θ[1]), zero(T)) | ||
| return csgtol^2 - t.r_tube[1]^2 * sin(Δθ)^2 >=0 && | ||
| _r >= t.r_tube[1] * cos(Δθ) - sqrt(csgtol^2 - t.r_tube[1]^2 * sin(Δθ)^2) && | ||
| if Δθ <= atan(csgtol, t.r_tube[2]) | ||
| csgtol^2 - t.r_tube[2]^2 * sin(Δθ)^2 >= 0 && | ||
| _r <= t.r_tube[2] * cos(Δθ) + sqrt(csgtol^2 - t.r_tube[2]^2 * sin(Δθ)^2) | ||
| else | ||
| _r <= max(csgtol / sin(min(Δθ, T(π/2))), t.r_tube[1] * cos(Δθ) + sqrt(csgtol^2 - t.r_tube[1]^2 * sin(Δθ)^2)) | ||
| end | ||
| end | ||
|
|
||
| function _in(pt::CartesianPoint{T}, t::HollowPhiTorus{T,OpenPrimitive}; csgtol::T = csg_default_tol(T)) where {T} | ||
| _r::T = hypot(hypot(pt.x, pt.y) - t.r_torus, pt.z) | ||
| θ::T = mod(atan(pt.z, hypot(pt.x, pt.y) - t.r_torus) - t.θ[1], T(2π)) | ||
| Δθ::T = max(min(T(2π) - θ, θ - t.θ[2] + t.θ[1]), zero(T)) | ||
| return t.r_tube[1] + csgtol < _r < t.r_tube[2] - csgtol && | ||
| _in_angular_interval_closed(θ, t.θ[2] - t.θ[1], csgtol = zero(T)) && | ||
| _r * sin(min(θ, t.θ[2] - t.θ[1] - θ, T(π/2))) > csgtol | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# HollowPhiTorus
cc = CSG.Torus{Float64}(r_tube = (0.1,0.25), r_torus = 0.3, θ = (0,2π/3))
@assert cc isa CSG.HollowPhiTorus{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
# HollowPhiTorus
cc = CSG.Torus{Float64}(r_tube = (0.1,0.25), r_torus = 0.3, θ = (0,4π/3))
@assert cc isa CSG.HollowPhiTorus{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected that on the upper panel we don't see anything in the bottom right plot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, my code is plotting the cross section at z=0, and because it’s an open primitive, there are no points at z=0 that are in. Probably not the best plot to show that it’s working ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here a more reasonable plot starting theta at negative values:
# FullPhiTorus
cc = CSG.Torus{Float64}(r_tube = 0.2, r_torus = 0.3, θ = (-π/3,π/3))
@assert cc isa CSG.FullPhiTorus{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
# FullPhiTorus
cc = CSG.Torus{Float64}(r_tube = 0.2, r_torus = 0.3, θ = (-π/3,4π/3))
@assert cc isa CSG.FullPhiTorus{Float64, CSG.ClosedPrimitive}
co = CSG.switchClosedOpen(cc)
csgtol = 0.05
| # Generic Torus (combination of HollowPhiTorus and FullThetaTorus) | ||
| function _in(pt::CartesianPoint{T}, t::Torus{T,CO}; csgtol::T = csg_default_tol(T)) where {T,CO} | ||
| rmax::T = _radial_endpoints(t.r_tube)[2] | ||
| _in(pt, Torus{T,CO}(t.r_torus, t.r_tube, nothing, t.θ, t.origin, t.rotation); csgtol) && | ||
| (isnothing(t.φ) || _in(pt, Torus{T,CO}(t.r_torus, rmax, t.φ, nothing, t.origin, t.rotation); csgtol)) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_in for Cone and Torus
|
I renamed the PR and will merge this by squashing all commits to keep this tidy. |






















This Pull Request addresses the third part of the list in #552 by adding tests to increase CartesianPoint and CartesianVector coverage and it fixes a previous typo in a function call.