Skip to content

Commit ad34961

Browse files
Remove deprecated apply_eclipse_shadowing! API (#51)
* refactor\!: Remove deprecated apply_eclipse_shadowing\! signature BREAKING CHANGE: Remove function signature that uses t₁₂ parameter The old signature with parameter order (shape1, r☉₁, R₁₂, t₁₂, shape2) has been removed. Use the new signature (shape1, shape2, r☉₁, r₁₂, R₁₂) which directly accepts shape2's position for better SPICE integration. * test: Update tests to use new apply_eclipse_shadowing! API - Update test_eclipse_shadowing.jl to use new API signature - Remove obsolete performance comparison test - Update test_visibility_extended.jl to use new API - Remove consistency test between old and new APIs * docs: Update documentation for API removal - Update migration guide to reflect API change - Mark task as completed in ROADMAP.md * docs: Add breaking change to CHANGELOG Document the removal of deprecated apply_eclipse_shadowing! signature in the Unreleased section of CHANGELOG.md * docs: Update migration guide for v0.5.0 - Reorganize sections with Getting Help and Future Deprecations at the top - Add comprehensive migration instructions for removed apply_eclipse_shadowing! signature - Include detailed code examples showing migration from t₁₂ to r₁₂ parameter - Remove outdated references to deprecated API in future deprecations section
1 parent 2c21f2e commit ad34961

File tree

6 files changed

+91
-399
lines changed

6 files changed

+91
-399
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Breaking Changes
11+
- **Removed deprecated `apply_eclipse_shadowing!` signature** (#51)
12+
- Removed the old function signature that used `t₁₂` parameter: `apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)`
13+
- Only the new signature remains: `apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)`
14+
- This change was planned in v0.4.1 and scheduled for v0.5.0
15+
1016
## [0.4.2] - 2025-01-21
1117

1218
### Added

ROADMAP.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ Please check our [GitHub Issues](https://github.com/Astroshaper/AsteroidShapeMod
7676
- [ ] Add comprehensive test coverage for roughness features
7777

7878
### API Improvements
79-
- [ ] Remove deprecated `apply_eclipse_shadowing!` signature that uses `t₁₂` parameter
79+
- [x] Remove deprecated `apply_eclipse_shadowing!` signature that uses `t₁₂` parameter
8080
- Old signature: `apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)`
8181
- Keep only the new signature with `r₁₂` parameter introduced in v0.4.1
8282
- [ ] Remove `use_elevation_optimization` parameter from illumination APIs

docs/src/guides/migration.md

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,56 @@
22

33
This guide helps you migrate your code when upgrading between major versions of `AsteroidShapeModels.jl`.
44

5+
## Getting Help
6+
7+
If you encounter issues during migration:
8+
9+
1. Check the [CHANGELOG](https://github.com/Astroshaper/AsteroidShapeModels.jl/blob/main/CHANGELOG.md) for detailed changes
10+
2. Review the [API documentation](https://astroshaper.github.io/AsteroidShapeModels.jl/stable)
11+
3. Open an [issue](https://github.com/Astroshaper/AsteroidShapeModels.jl/issues) on GitHub
12+
13+
## Future Deprecations
14+
15+
1. **`use_elevation_optimization` parameter**
16+
- Will be removed in a future version
17+
- Optimization will become the default behavior
18+
- Start removing explicit `use_elevation_optimization=true` from your code
19+
20+
## Migrating to v0.5.0 (Unreleased)
21+
22+
### Breaking Changes
23+
24+
#### Removed `apply_eclipse_shadowing!` deprecated signature
25+
26+
The old function signature that used the `t₁₂` parameter has been removed:
27+
28+
```julia
29+
# Old signature (removed)
30+
apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)
31+
32+
# New signature (use this)
33+
apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)
34+
```
35+
36+
**Migration steps:**
37+
38+
1. Replace the `t₁₂` parameter with `r₁₂` (shape2's position in shape1's frame)
39+
- The position corresponding to `r₁₂` can be retrieved from SPICE kernels.
40+
- If you have `t₁₂`, compute `r₁₂` using: `r₁₂ = -R₁₂' * t₁₂`
41+
2. Update the parameter order to group shapes together
42+
43+
**Example migration:**
44+
45+
```julia
46+
# Before (v0.4.x)
47+
t₁₂ = -R₁₂ * r₁₂ # You might have computed t₁₂ like this
48+
apply_eclipse_shadowing!(illuminated, shape1, sun_pos, R₁₂, t₁₂, shape2)
49+
50+
# After (v0.5.0)
51+
# Use r₁₂ directly (shape2's position)
52+
apply_eclipse_shadowing!(illuminated, shape1, shape2, sun_pos, r₁₂, R₁₂)
53+
```
54+
555
## Migrating to v0.4.2
656

757
### New Performance Features
@@ -34,26 +84,14 @@ illuminated = isilluminated(
3484
The parameter order has been changed for better SPICE integration:
3585

3686
```julia
37-
# Old API (deprecated)
38-
apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)
39-
40-
# New API (recommended)
87+
# New API
4188
apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)
4289
```
4390

44-
Key differences:
91+
Key differences from v0.4.0:
4592
- `shape1` and `shape2` are now grouped together
46-
- `r₁₂` (shape2's position in shape1's frame) replaces `t₁₂`
47-
- More intuitive parameter ordering
48-
49-
Migration example:
50-
```julia
51-
# If you have t₁₂, convert it to r₁₂:
52-
r₁₂ = -R₁₂' * t₁₂
53-
54-
# Then use the new API:
55-
apply_eclipse_shadowing!(illuminated, shape1, shape2, sun_position, r₁₂, R₁₂)
56-
```
93+
- `r₁₂` (shape2's position in shape1's frame) is used directly
94+
- More intuitive parameter ordering for SPICE integration
5795

5896
## Migrating to v0.4.0
5997

@@ -80,24 +118,3 @@ New batch processing functions for better performance:
80118
illuminated = Vector{Bool}(undef, length(shape.faces))
81119
update_illumination!(illuminated, shape, sun_position; with_self_shadowing=true)
82120
```
83-
84-
## Future Deprecations (v0.5.0)
85-
86-
### Planned Removals
87-
88-
1. **`use_elevation_optimization` parameter**
89-
- Will be removed in v0.5.0
90-
- Optimization will become the default behavior
91-
- Start removing explicit `use_elevation_optimization=true` from your code
92-
93-
2. **Old `apply_eclipse_shadowing!` signature**
94-
- The deprecated signature with `t₁₂` will be removed
95-
- Migrate to the new API with `r₁₂` parameter
96-
97-
## Getting Help
98-
99-
If you encounter issues during migration:
100-
101-
1. Check the [CHANGELOG](https://github.com/Astroshaper/AsteroidShapeModels.jl/blob/main/CHANGELOG.md) for detailed changes
102-
2. Review the [API documentation](https://astroshaper.github.io/AsteroidShapeModels.jl/stable)
103-
3. Open an [issue](https://github.com/Astroshaper/AsteroidShapeModels.jl/issues) on GitHub

src/eclipse_shadowing.jl

Lines changed: 0 additions & 214 deletions
Original file line numberDiff line numberDiff line change
@@ -36,220 +36,6 @@ end
3636
# ║ Eclipse Shadowing Functions ║
3737
# ╚═══════════════════════════════════════════════════════════════════╝
3838

39-
"""
40-
apply_eclipse_shadowing!(
41-
illuminated_faces::AbstractVector{Bool}, shape1::ShapeModel, r☉₁::StaticVector{3},
42-
R₁₂::StaticMatrix{3,3}, t₁₂::StaticVector{3}, shape2::ShapeModel
43-
) -> EclipseStatus
44-
45-
Apply eclipse shadowing effects from another shape onto already illuminated faces.
46-
47-
!!! warning "Deprecated"
48-
This function signature will be removed in v0.5.0. Please use the new signature:
49-
`apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)`
50-
which directly accepts shape2's position instead of the transformation parameter.
51-
52-
!!! note
53-
As of v0.4.0, `shape2` must have BVH pre-built before calling this function.
54-
Use either `with_bvh=true` when loading or call `build_bvh!(shape2)` explicitly.
55-
56-
# Arguments
57-
- `illuminated_faces` : Boolean vector with current illumination state (will be modified)
58-
- `shape1` : Target shape model being shadowed (the shape receiving shadows)
59-
- `r☉₁` : Sun's position in shape1's frame
60-
- `R₁₂` : 3×3 rotation matrix from `shape1` frame to `shape2` frame
61-
- `t₁₂` : 3D translation vector from `shape1` frame to `shape2` frame
62-
- `shape2` : Occluding shape model that may cast shadows on `shape1` (must have BVH built via `build_bvh!`)
63-
64-
# Returns
65-
- `NO_ECLIPSE`: No eclipse occurs (bodies are misaligned).
66-
- `PARTIAL_ECLIPSE`: Some faces that were illuminated are now in shadow by the occluding body.
67-
- `TOTAL_ECLIPSE`: All faces that were illuminated are now in shadow.
68-
69-
# Throws
70-
- `ArgumentError` if `shape2` does not have BVH built. Call `build_bvh!(shape2)` before using this function.
71-
72-
# Description
73-
This function ONLY checks for mutual shadowing (eclipse) effects. It assumes that
74-
the `illuminated_faces` vector already contains the result of face orientation and/or
75-
self-shadowing checks. Only faces marked as `true` in the input will be tested
76-
for occlusion by the other body.
77-
78-
This separation allows flexible control of shadowing effects in thermal modeling:
79-
- Call `update_illumination_*` first for self-shadowing (or face orientation only)
80-
- Then call this function to add mutual shadowing effects
81-
82-
# Performance Optimizations
83-
The function includes early-out checks at two levels:
84-
85-
## Body-level optimizations:
86-
1. **Behind Check**: If the occluding body is entirely behind the target relative to the sun,
87-
no eclipse can occur.
88-
89-
2. **Lateral Separation Check**: If bodies are too far apart laterally (perpendicular to
90-
sun direction), no eclipse can occur.
91-
92-
3. **Total Eclipse Check**: If the target is completely within the occluding body's shadow,
93-
all illuminated faces are set to false without individual ray checks.
94-
95-
## Face-level optimizations:
96-
4. **Ray-Sphere Intersection Check**: For each face, checks if the ray to the sun can possibly
97-
intersect the occluding body's bounding sphere. Skips ray-shape test if the ray clearly
98-
misses the sphere.
99-
100-
5. **Inscribed Sphere Check**: If the ray passes through the occluding body's inscribed sphere,
101-
the face is guaranteed to be shadowed, avoiding the expensive ray-shape intersection test.
102-
103-
These optimizations use `maximum_radius` and `minimum_radius` for accurate sphere calculations.
104-
105-
# Coordinate Systems
106-
The transformation from `shape1` frame to `shape2` frame is given by:
107-
`p_shape2 = R₁₂ * p_shape1 + t₁₂`
108-
109-
# Example
110-
```julia
111-
# Check self-shadowing first
112-
update_illumination!(illuminated_faces1, shape1, sun_position1; with_self_shadowing=true)
113-
update_illumination!(illuminated_faces2, shape2, sun_position2; with_self_shadowing=true)
114-
115-
# Or if you want to ignore self-shadowing:
116-
update_illumination!(illuminated_faces1, shape1, sun_position1; with_self_shadowing=false)
117-
update_illumination!(illuminated_faces2, shape2, sun_position2; with_self_shadowing=false)
118-
119-
# Then check eclipse shadowing
120-
# For checking mutual shadowing, apply to both shape1 and shape2:
121-
status1 = apply_eclipse_shadowing!(illuminated_faces1, shape1, sun_position1, R12, t12, shape2)
122-
status2 = apply_eclipse_shadowing!(illuminated_faces2, shape2, sun_position2, R21, t21, shape1)
123-
124-
# Handle eclipse status
125-
if status1 == NO_ECLIPSE
126-
println("Shape1 is not eclipsed by shape2.")
127-
elseif status1 == PARTIAL_ECLIPSE
128-
println("Shape1 is partially eclipsed by shape2.")
129-
elseif status1 == TOTAL_ECLIPSE
130-
println("Shape1 is totally eclipsed by shape2.")
131-
end
132-
```
133-
"""
134-
function apply_eclipse_shadowing!(
135-
illuminated_faces::AbstractVector{Bool}, shape1::ShapeModel, r☉₁::StaticVector{3},
136-
R₁₂::StaticMatrix{3,3}, t₁₂::StaticVector{3}, shape2::ShapeModel
137-
)::EclipseStatus
138-
@assert length(illuminated_faces) == length(shape1.faces) "illuminated_faces vector must have same length as number of faces."
139-
isnothing(shape2.bvh) && throw(ArgumentError("Occluding shape model (`shape2`) must have BVH built before checking eclipse shadowing. Call `build_bvh!(shape2)` first."))
140-
141-
# Recover shape2's position in shape1's frame
142-
# Since p_shape2 = R₁₂ * p_shape1 + t₁₂, and shape2's origin (0) is at r₁₂ in shape1's frame:
143-
# 0 = R₁₂ * r₁₂ + t₁₂, therefore: r₁₂ = -R₁₂' * t₁₂
144-
r₁₂ = -R₁₂' * t₁₂
145-
146-
r̂☉₁ = normalize(r☉₁) # Normalized sun direction in shape1's frame
147-
r̂☉₂ = normalize(R₁₂ * r☉₁ + t₁₂) # Normalized sun direction in shape2's frame
148-
149-
# Get bounding sphere radii for both shapes
150-
ρ₁ = maximum_radius(shape1)
151-
ρ₂ = maximum_radius(shape2)
152-
153-
# Get inscribed sphere radius for shape2 (for guaranteed shadow regions)
154-
ρ₂_inner = minimum_radius(shape2)
155-
156-
# ==== Early Out 1 (Behind Check) ====
157-
# If shape2 is entirely behind shape1 relative to sun, no eclipse occur.
158-
if dot(r₁₂, r̂☉₁) < -(ρ₁ + ρ₂)
159-
return NO_ECLIPSE
160-
end
161-
162-
# ==== Early Out 2 (Lateral Separation Check) ====
163-
# If bodies are too far apart laterally, no eclipse occur.
164-
r₁₂⊥ = r₁₂ - (dot(r₁₂, r̂☉₁) * r̂☉₁) # Component of r₁₂ perpendicular to sun direction
165-
d⊥ = norm(r₁₂⊥) # Lateral distance between bodies
166-
if d⊥ > ρ₁ + ρ₂
167-
return NO_ECLIPSE
168-
end
169-
170-
# ==== Early Out 3 (Total Eclipse Check) ====
171-
# If shape1 is completely within shape2's shadow, all faces are shadowed.
172-
# This happens when shape2 is between sun and shape1, and is larger than shape1.
173-
# Check if shape2 is in front of shape1 along sun direction,
174-
# and if the lateral distance is small enough.
175-
if dot(r₁₂, r̂☉₁) > 0 && d⊥ + ρ₁ < ρ₂_inner
176-
illuminated_faces .= false # All faces are shadowed.
177-
return TOTAL_ECLIPSE
178-
end
179-
180-
# Track whether any eclipse occurred
181-
eclipse_occurred = false
182-
183-
# Check occlusion by the other body for illuminated faces only
184-
@inbounds for i in eachindex(shape1.faces)
185-
if illuminated_faces[i] # Only check if not already in shadow
186-
# Ray from face center to sun in shape1's frame
187-
ray_origin1 = shape1.face_centers[i]
188-
189-
# Transform ray's origin to shape2's frame
190-
ray_origin2 = R₁₂ * ray_origin1 + t₁₂
191-
192-
# ==== Face-level Early Out ====
193-
# Check if the ray from this face to the sun can possibly intersect
194-
# shape2's bounding sphere.
195-
196-
# Calculate the parameter t where the ray is closest to shape2's center.
197-
# Ray: P(t) = ray_origin2 + t * r̂☉₂, where t > 0 toward sun
198-
# The closest point is where d/dt |P(t)|² = 0
199-
t_min = -dot(ray_origin2, r̂☉₂)
200-
201-
if t_min < 0
202-
# Shape2's center is in the opposite direction from the sun.
203-
# (i.e., the ray is moving away from shape2)
204-
# In this case, check if the face itself is outside bounding sphere.
205-
if norm(ray_origin2) > ρ₂
206-
continue
207-
end
208-
else
209-
# The ray approaches shape2's center.
210-
# Calculate the closest point on the ray to the center
211-
p_closest = ray_origin2 + t_min * r̂☉₂
212-
d_center = norm(p_closest)
213-
214-
# ==== Early Out 4 (Ray-Sphere Intersection Check) ====
215-
# If the ray passes within the bounding sphere,
216-
# ray misses the bounding sphere entirely.
217-
if d_center > ρ₂
218-
continue
219-
end
220-
221-
# ==== Early Out 5 (Inscribed Sphere Check) ====
222-
# If the ray passes through the inscribed sphere, it's guaranteed to hit shape2
223-
# (no need for detailed intersection test)
224-
if d_center < ρ₂_inner
225-
illuminated_faces[i] = false
226-
eclipse_occurred = true
227-
continue
228-
end
229-
end
230-
231-
# Create ray in shape2's frame
232-
# (Direction was already transformed at the beginning of the function)
233-
ray2 = Ray(ray_origin2, r̂☉₂)
234-
235-
# Check intersection with shape2
236-
if intersect_ray_shape(ray2, shape2).hit
237-
illuminated_faces[i] = false
238-
eclipse_occurred = true
239-
end
240-
end
241-
end
242-
243-
# Determine eclipse status based on results
244-
if !eclipse_occurred
245-
return NO_ECLIPSE
246-
elseif count(illuminated_faces) == 0 # if all faces are now in shadow
247-
return TOTAL_ECLIPSE
248-
else
249-
return PARTIAL_ECLIPSE
250-
end
251-
end
252-
25339
"""
25440
apply_eclipse_shadowing!(
25541
illuminated_faces::AbstractVector{Bool}, shape1::ShapeModel, shape2::ShapeModel,

0 commit comments

Comments
 (0)