Skip to content

Fix landscape composite moon rendering alignment in photography guide#19

Merged
c0d3rb4b4 merged 3 commits into
mainfrom
codex/fix-landscape-composite-render-issue
Feb 26, 2026
Merged

Fix landscape composite moon rendering alignment in photography guide#19
c0d3rb4b4 merged 3 commits into
mainfrom
codex/fix-landscape-composite-render-issue

Conversation

@c0d3rb4b4
Copy link
Copy Markdown
Owner

Motivation

  • The landscape composite modal used separate azimuth/altitude math for moon placement which drifted relative to the shot schedule previews, causing mismatched renders.
  • The goal is to make composite visuals match the preview thumbnails while keeping observer-based sun placement and horizon behavior intact.

Description

  • Replaced the ad-hoc azimuth/altitude + clamp-delta moon placement in buildLandscapeCompositeLayout with the same calculatePreviewMoonGeometry model used by thumbnails so moon offsets are computed from the phase geometry and travel vector. (apps/mobile/src/utils/photographyGuide.ts)
  • Normalized and reused the travelVector in the observer-based composite path so moon offsets are stable per layout render. (apps/mobile/src/utils/photographyGuide.ts)
  • Removed the previous clamped-delta math that could introduce drift and incorrect occlusion; preserved observer-derived sun positioning and horizon calculation. (apps/mobile/src/utils/photographyGuide.ts)
  • Strengthened the Gibraltar 2027 regression to assert moon visibility and moon-to-sun proximity for each composite placement. (apps/mobile/tests/photography-guide.test.ts)

Testing

  • Ran unit tests with pnpm --filter @eclipse-timer/mobile test -- photography-guide.test.ts and all tests passed (1 file, 8 tests).
  • Attempted a Playwright-based screenshot run to validate the rendered modal, but headless Chromium crashed in this environment (SIGSEGV), so only automated unit tests were used for verification.

Codex Task

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns the Photography Guide landscape composite modal’s moon placement with the same preview-geometry model used by schedule thumbnails, reducing drift/mismatch between composite renders and previews.

Changes:

  • Replaced the landscape composite moon placement math with calculatePreviewMoonGeometry-based offsets.
  • Normalized/reused travelVector in the observer-driven composite layout path for stable moon offset computation.
  • Strengthened the Gibraltar 2027 regression test with assertions around moon visibility and proximity to the sun per placement.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
documents/reference/CHANGELOG.md Documents the moon alignment fix and version bump.
apps/mobile/src/utils/photographyGuide.ts Switches composite moon positioning to preview geometry and normalizes travel vector.
apps/mobile/tests/photography-guide.test.ts Adds regression assertions ensuring moon geometry is present and near the sun.
apps/mobile/package.json Bumps mobile package version to 1.1.36.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/mobile/src/utils/photographyGuide.ts
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@c0d3rb4b4 c0d3rb4b4 merged commit c200b4f into main Feb 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants