Skip to content

Remove deprecated apply_eclipse_shadowing! API #51

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.

Sign up for GitHub

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

Merged
MasanoriKanamaru merged 5 commits into from
Jul 22, 2025
Merged

Remove deprecated apply_eclipse_shadowing! API #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43af039
refactor\!: Remove deprecated apply_eclipse_shadowing\! signature
MasanoriKanamaru Jul 22, 2025
7b194c7
test: Update tests to use new apply_eclipse_shadowing! API
MasanoriKanamaru Jul 22, 2025
17c03c1
docs: Update documentation for API removal
MasanoriKanamaru Jul 22, 2025
9749563
docs: Add breaking change to CHANGELOG
MasanoriKanamaru Jul 22, 2025
1669852
docs: Update migration guide for v0.5.0
MasanoriKanamaru Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Breaking Changes
- **Removed deprecated `apply_eclipse_shadowing!` signature** (#51)
- Removed the old function signature that used `t₁₂` parameter: `apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)`
- Only the new signature remains: `apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)`
- This change was planned in v0.4.1 and scheduled for v0.5.0

## [0.4.2] - 2025-01-21

### Added
Expand Down
2 changes: 1 addition & 1 deletion ROADMAP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Please check our [GitHub Issues](https://github.com/Astroshaper/AsteroidShapeMod
- [ ] Add comprehensive test coverage for roughness features

### API Improvements
- [ ] Remove deprecated `apply_eclipse_shadowing!` signature that uses `t₁₂` parameter
- [x] Remove deprecated `apply_eclipse_shadowing!` signature that uses `t₁₂` parameter
- Old signature: `apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)`
- Keep only the new signature with `r₁₂` parameter introduced in v0.4.1
- [ ] Remove `use_elevation_optimization` parameter from illumination APIs
Expand Down
91 changes: 54 additions & 37 deletions docs/src/guides/migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,56 @@

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

## Getting Help

If you encounter issues during migration:

1. Check the [CHANGELOG](https://github.com/Astroshaper/AsteroidShapeModels.jl/blob/main/CHANGELOG.md) for detailed changes
2. Review the [API documentation](https://astroshaper.github.io/AsteroidShapeModels.jl/stable)
3. Open an [issue](https://github.com/Astroshaper/AsteroidShapeModels.jl/issues) on GitHub

## Future Deprecations

1. **`use_elevation_optimization` parameter**
- Will be removed in a future version
- Optimization will become the default behavior
- Start removing explicit `use_elevation_optimization=true` from your code

## Migrating to v0.5.0 (Unreleased)

### Breaking Changes

#### Removed `apply_eclipse_shadowing!` deprecated signature

The old function signature that used the `t₁₂` parameter has been removed:

```julia
# Old signature (removed)
apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)

# New signature (use this)
apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)
```

**Migration steps:**

1. Replace the `t₁₂` parameter with `r₁₂` (shape2's position in shape1's frame)
- The position corresponding to `r₁₂` can be retrieved from SPICE kernels.
- If you have `t₁₂`, compute `r₁₂` using: `r₁₂ = -R₁₂' * t₁₂`
2. Update the parameter order to group shapes together

**Example migration:**

```julia
# Before (v0.4.x)
t₁₂ = -R₁₂ * r₁₂ # You might have computed t₁₂ like this
apply_eclipse_shadowing!(illuminated, shape1, sun_pos, R₁₂, t₁₂, shape2)

# After (v0.5.0)
# Use r₁₂ directly (shape2's position)
apply_eclipse_shadowing!(illuminated, shape1, shape2, sun_pos, r₁₂, R₁₂)
```

## Migrating to v0.4.2

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

```julia
# Old API (deprecated)
apply_eclipse_shadowing!(illuminated_faces, shape1, r☉₁, R₁₂, t₁₂, shape2)

# New API (recommended)
# New API
apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)
```

Key differences:
Key differences from v0.4.0:
- `shape1` and `shape2` are now grouped together
- `r₁₂` (shape2's position in shape1's frame) replaces `t₁₂`
- More intuitive parameter ordering

Migration example:
```julia
# If you have t₁₂, convert it to r₁₂:
r₁₂ = -R₁₂' * t₁₂

# Then use the new API:
apply_eclipse_shadowing!(illuminated, shape1, shape2, sun_position, r₁₂, R₁₂)
```
- `r₁₂` (shape2's position in shape1's frame) is used directly
- More intuitive parameter ordering for SPICE integration

## Migrating to v0.4.0

Expand All @@ -80,24 +118,3 @@ New batch processing functions for better performance:
illuminated = Vector{Bool}(undef, length(shape.faces))
update_illumination!(illuminated, shape, sun_position; with_self_shadowing=true)
```

## Future Deprecations (v0.5.0)

### Planned Removals

1. **`use_elevation_optimization` parameter**
- Will be removed in v0.5.0
- Optimization will become the default behavior
- Start removing explicit `use_elevation_optimization=true` from your code

2. **Old `apply_eclipse_shadowing!` signature**
- The deprecated signature with `t₁₂` will be removed
- Migrate to the new API with `r₁₂` parameter

## Getting Help

If you encounter issues during migration:

1. Check the [CHANGELOG](https://github.com/Astroshaper/AsteroidShapeModels.jl/blob/main/CHANGELOG.md) for detailed changes
2. Review the [API documentation](https://astroshaper.github.io/AsteroidShapeModels.jl/stable)
3. Open an [issue](https://github.com/Astroshaper/AsteroidShapeModels.jl/issues) on GitHub
214 changes: 0 additions & 214 deletions src/eclipse_shadowing.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,220 +36,6 @@ end
# ║ Eclipse Shadowing Functions ║
# ╚═══════════════════════════════════════════════════════════════════╝

"""
apply_eclipse_shadowing!(
illuminated_faces::AbstractVector{Bool}, shape1::ShapeModel, r☉₁::StaticVector{3},
R₁₂::StaticMatrix{3,3}, t₁₂::StaticVector{3}, shape2::ShapeModel
) -> EclipseStatus

Apply eclipse shadowing effects from another shape onto already illuminated faces.

!!! warning "Deprecated"
This function signature will be removed in v0.5.0. Please use the new signature:
`apply_eclipse_shadowing!(illuminated_faces, shape1, shape2, r☉₁, r₁₂, R₁₂)`
which directly accepts shape2's position instead of the transformation parameter.

!!! note
As of v0.4.0, `shape2` must have BVH pre-built before calling this function.
Use either `with_bvh=true` when loading or call `build_bvh!(shape2)` explicitly.

# Arguments
- `illuminated_faces` : Boolean vector with current illumination state (will be modified)
- `shape1` : Target shape model being shadowed (the shape receiving shadows)
- `r☉₁` : Sun's position in shape1's frame
- `R₁₂` : 3×3 rotation matrix from `shape1` frame to `shape2` frame
- `t₁₂` : 3D translation vector from `shape1` frame to `shape2` frame
- `shape2` : Occluding shape model that may cast shadows on `shape1` (must have BVH built via `build_bvh!`)

# Returns
- `NO_ECLIPSE`: No eclipse occurs (bodies are misaligned).
- `PARTIAL_ECLIPSE`: Some faces that were illuminated are now in shadow by the occluding body.
- `TOTAL_ECLIPSE`: All faces that were illuminated are now in shadow.

# Throws
- `ArgumentError` if `shape2` does not have BVH built. Call `build_bvh!(shape2)` before using this function.

# Description
This function ONLY checks for mutual shadowing (eclipse) effects. It assumes that
the `illuminated_faces` vector already contains the result of face orientation and/or
self-shadowing checks. Only faces marked as `true` in the input will be tested
for occlusion by the other body.

This separation allows flexible control of shadowing effects in thermal modeling:
- Call `update_illumination_*` first for self-shadowing (or face orientation only)
- Then call this function to add mutual shadowing effects

# Performance Optimizations
The function includes early-out checks at two levels:

## Body-level optimizations:
1. **Behind Check**: If the occluding body is entirely behind the target relative to the sun,
no eclipse can occur.

2. **Lateral Separation Check**: If bodies are too far apart laterally (perpendicular to
sun direction), no eclipse can occur.

3. **Total Eclipse Check**: If the target is completely within the occluding body's shadow,
all illuminated faces are set to false without individual ray checks.

## Face-level optimizations:
4. **Ray-Sphere Intersection Check**: For each face, checks if the ray to the sun can possibly
intersect the occluding body's bounding sphere. Skips ray-shape test if the ray clearly
misses the sphere.

5. **Inscribed Sphere Check**: If the ray passes through the occluding body's inscribed sphere,
the face is guaranteed to be shadowed, avoiding the expensive ray-shape intersection test.

These optimizations use `maximum_radius` and `minimum_radius` for accurate sphere calculations.

# Coordinate Systems
The transformation from `shape1` frame to `shape2` frame is given by:
`p_shape2 = R₁₂ * p_shape1 + t₁₂`

# Example
```julia
# Check self-shadowing first
update_illumination!(illuminated_faces1, shape1, sun_position1; with_self_shadowing=true)
update_illumination!(illuminated_faces2, shape2, sun_position2; with_self_shadowing=true)

# Or if you want to ignore self-shadowing:
update_illumination!(illuminated_faces1, shape1, sun_position1; with_self_shadowing=false)
update_illumination!(illuminated_faces2, shape2, sun_position2; with_self_shadowing=false)

# Then check eclipse shadowing
# For checking mutual shadowing, apply to both shape1 and shape2:
status1 = apply_eclipse_shadowing!(illuminated_faces1, shape1, sun_position1, R12, t12, shape2)
status2 = apply_eclipse_shadowing!(illuminated_faces2, shape2, sun_position2, R21, t21, shape1)

# Handle eclipse status
if status1 == NO_ECLIPSE
println("Shape1 is not eclipsed by shape2.")
elseif status1 == PARTIAL_ECLIPSE
println("Shape1 is partially eclipsed by shape2.")
elseif status1 == TOTAL_ECLIPSE
println("Shape1 is totally eclipsed by shape2.")
end
```
"""
function apply_eclipse_shadowing!(
illuminated_faces::AbstractVector{Bool}, shape1::ShapeModel, r☉₁::StaticVector{3},
R₁₂::StaticMatrix{3,3}, t₁₂::StaticVector{3}, shape2::ShapeModel
)::EclipseStatus
@assert length(illuminated_faces) == length(shape1.faces) "illuminated_faces vector must have same length as number of faces."
isnothing(shape2.bvh) && throw(ArgumentError("Occluding shape model (`shape2`) must have BVH built before checking eclipse shadowing. Call `build_bvh!(shape2)` first."))

# Recover shape2's position in shape1's frame
# Since p_shape2 = R₁₂ * p_shape1 + t₁₂, and shape2's origin (0) is at r₁₂ in shape1's frame:
# 0 = R₁₂ * r₁₂ + t₁₂, therefore: r₁₂ = -R₁₂' * t₁₂
r₁₂ = -R₁₂' * t₁₂

r̂☉₁ = normalize(r☉₁) # Normalized sun direction in shape1's frame
r̂☉₂ = normalize(R₁₂ * r☉₁ + t₁₂) # Normalized sun direction in shape2's frame

# Get bounding sphere radii for both shapes
ρ₁ = maximum_radius(shape1)
ρ₂ = maximum_radius(shape2)

# Get inscribed sphere radius for shape2 (for guaranteed shadow regions)
ρ₂_inner = minimum_radius(shape2)

# ==== Early Out 1 (Behind Check) ====
# If shape2 is entirely behind shape1 relative to sun, no eclipse occur.
if dot(r₁₂, r̂☉₁) < -(ρ₁ + ρ₂)
return NO_ECLIPSE
end

# ==== Early Out 2 (Lateral Separation Check) ====
# If bodies are too far apart laterally, no eclipse occur.
r₁₂⊥ = r₁₂ - (dot(r₁₂, r̂☉₁) * r̂☉₁) # Component of r₁₂ perpendicular to sun direction
d⊥ = norm(r₁₂⊥) # Lateral distance between bodies
if d⊥ > ρ₁ + ρ₂
return NO_ECLIPSE
end

# ==== Early Out 3 (Total Eclipse Check) ====
# If shape1 is completely within shape2's shadow, all faces are shadowed.
# This happens when shape2 is between sun and shape1, and is larger than shape1.
# Check if shape2 is in front of shape1 along sun direction,
# and if the lateral distance is small enough.
if dot(r₁₂, r̂☉₁) > 0 && d⊥ + ρ₁ < ρ₂_inner
illuminated_faces .= false # All faces are shadowed.
return TOTAL_ECLIPSE
end

# Track whether any eclipse occurred
eclipse_occurred = false

# Check occlusion by the other body for illuminated faces only
@inbounds for i in eachindex(shape1.faces)
if illuminated_faces[i] # Only check if not already in shadow
# Ray from face center to sun in shape1's frame
ray_origin1 = shape1.face_centers[i]

# Transform ray's origin to shape2's frame
ray_origin2 = R₁₂ * ray_origin1 + t₁₂

# ==== Face-level Early Out ====
# Check if the ray from this face to the sun can possibly intersect
# shape2's bounding sphere.

# Calculate the parameter t where the ray is closest to shape2's center.
# Ray: P(t) = ray_origin2 + t * r̂☉₂, where t > 0 toward sun
# The closest point is where d/dt |P(t)|² = 0
t_min = -dot(ray_origin2, r̂☉₂)

if t_min < 0
# Shape2's center is in the opposite direction from the sun.
# (i.e., the ray is moving away from shape2)
# In this case, check if the face itself is outside bounding sphere.
if norm(ray_origin2) > ρ₂
continue
end
else
# The ray approaches shape2's center.
# Calculate the closest point on the ray to the center
p_closest = ray_origin2 + t_min * r̂☉₂
d_center = norm(p_closest)

# ==== Early Out 4 (Ray-Sphere Intersection Check) ====
# If the ray passes within the bounding sphere,
# ray misses the bounding sphere entirely.
if d_center > ρ₂
continue
end

# ==== Early Out 5 (Inscribed Sphere Check) ====
# If the ray passes through the inscribed sphere, it's guaranteed to hit shape2
# (no need for detailed intersection test)
if d_center < ρ₂_inner
illuminated_faces[i] = false
eclipse_occurred = true
continue
end
end

# Create ray in shape2's frame
# (Direction was already transformed at the beginning of the function)
ray2 = Ray(ray_origin2, r̂☉₂)

# Check intersection with shape2
if intersect_ray_shape(ray2, shape2).hit
illuminated_faces[i] = false
eclipse_occurred = true
end
end
end

# Determine eclipse status based on results
if !eclipse_occurred
return NO_ECLIPSE
elseif count(illuminated_faces) == 0 # if all faces are now in shadow
return TOTAL_ECLIPSE
else
return PARTIAL_ECLIPSE
end
end

"""
apply_eclipse_shadowing!(
illuminated_faces::AbstractVector{Bool}, shape1::ShapeModel, shape2::ShapeModel,
Expand Down
Loading
Loading