Skip to content

feat: Add 3D rotation effect option for snowflakes#90

Merged
cahilfoley merged 1 commit into
cahilfoley:mainfrom
ashtonisher:feature
Dec 1, 2025
Merged

feat: Add 3D rotation effect option for snowflakes#90
cahilfoley merged 1 commit into
cahilfoley:mainfrom
ashtonisher:feature

Conversation

@ashtonisher

@ashtonisher ashtonisher commented Nov 17, 2025

Copy link
Copy Markdown
Contributor
  • Add enable3DRotation prop to enable 3D tumbling effect
  • Add UI toggle in demo settings
  • No perspective scale rotation
  • Update README.md

- Add enable3DRotation prop to enable 3D tumbling effect
- Add UI toggle in demo settings
- Update README.md
@cahilfoley cahilfoley requested a review from Copilot December 1, 2025 09:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new 3D rotation effect feature for snowflakes, creating a more realistic tumbling animation similar to falling leaves. The feature can be toggled via the enable3DRotation prop and includes an interactive UI control in the demo application.

Key Changes:

  • Added enable3DRotation configuration option with 3D rotation math for X, Y, and Z axes
  • Implemented separate rendering path for 3D-rotated circles to apply transforms individually
  • Updated demo UI with a checkbox toggle for the new feature

Reviewed changes

Copilot reviewed 10 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/react-snowfall/src/Snowflake.ts Added 3D rotation parameters, transform calculation logic, and new drawCircle3D method
packages/react-snowfall/src/SnowfallCanvas.ts Added conditional rendering path for 3D circles using individual transforms
packages/react-snowfall/src/Snowfall.tsx Added enable3DRotation prop with default value
packages/react-snowfall/lib/Snowflake.js Transpiled version with perspective scaling added to 3D transform
packages/react-snowfall/lib/SnowfallCanvas.js Transpiled version of canvas rendering updates
packages/react-snowfall/lib/Snowfall.js Transpiled version with new prop
packages/demo/src/settings.tsx Added enable3DRotation to settings store
packages/demo/src/components/Settings/Settings.tsx Added UI checkbox for 3D rotation toggle
packages/demo/src/App.tsx Passed enable3DRotation prop to Snowfall component
README.md Documented new enable3DRotation property

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

Comment on lines +312 to +315
// Apply the transform
// The arguments for setTransform are: a, b, c, d, e, f
// a (scaleX), b (skewY), c (skewX), d (scaleY), e (translateX), f (translateY)
ctx.setTransform(a, b, c, d, x, y)

Copilot AI Dec 1, 2025

Copy link

Choose a reason for hiding this comment

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

The 3D transform matrix calculation is incomplete. Variables a, b, c, and d represent only part of the 3x3 rotation matrix. For a proper 3D effect on a 2D canvas, you need to project the 3D coordinates to 2D. The current implementation missing the e and f calculations which should incorporate sinY*sinZ terms. Compare this with the compiled JavaScript version at line 185 which includes perspective scaling - the TypeScript source should match its compiled output.

Suggested change
// Apply the transform
// The arguments for setTransform are: a, b, c, d, e, f
// a (scaleX), b (skewY), c (skewX), d (scaleY), e (translateX), f (translateY)
ctx.setTransform(a, b, c, d, x, y)
// Perspective projection parameters
// Simulate a simple perspective effect
const perspective = 400 // You can tweak this value for more/less perspective
// Calculate the Z position after rotation
const z = -sinY * cosX // This is a simplification for a unit vector
// Calculate perspective scale
const scale = perspective / (perspective - z * 100) // 100 is arbitrary depth
// Projected 2D position
const e = x * scale
const f = y * scale
// Apply the transform
// The arguments for setTransform are: a, b, c, d, e, f
// a (scaleX), b (skewY), c (skewX), d (scaleY), e (translateX), f (translateY)
ctx.setTransform(a * scale, b * scale, c * scale, d * scale, e, f)

Copilot uses AI. Check for mistakes.
Comment on lines +178 to +185
// Apply perspective scaling based on rotation (simulates depth)
const perspectiveScale = 0.5 + 0.5 * cosX * cosY;
const scaleX = perspectiveScale;
const scaleY = perspectiveScale;
// Apply the transform
// The arguments for setTransform are: a, b, c, d, e, f
// a (scaleX), b (skewY), c (skewX), d (scaleY), e (translateX), f (translateY)
ctx.setTransform(a * scaleX, b * scaleX, c * scaleY, d * scaleY, x, y);

Copilot AI Dec 1, 2025

Copy link

Choose a reason for hiding this comment

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

The perspective scaling implementation contradicts the PR description which states 'No perspective scale rotation'. This code applies perspective scaling (lines 178-181) that should either be removed to match the PR intent, or the PR description should be updated to reflect this behavior.

Suggested change
// Apply perspective scaling based on rotation (simulates depth)
const perspectiveScale = 0.5 + 0.5 * cosX * cosY;
const scaleX = perspectiveScale;
const scaleY = perspectiveScale;
// Apply the transform
// The arguments for setTransform are: a, b, c, d, e, f
// a (scaleX), b (skewY), c (skewX), d (scaleY), e (translateX), f (translateY)
ctx.setTransform(a * scaleX, b * scaleX, c * scaleY, d * scaleY, x, y);
// No perspective scaling applied (matches PR intent)
// Apply the transform
// The arguments for setTransform are: a, b, c, d, e, f
// a (scaleX), b (skewY), c (skewX), d (scaleY), e (translateX), f (translateY)
ctx.setTransform(a, b, c, d, x, y);

Copilot uses AI. Check for mistakes.
@cahilfoley cahilfoley merged commit c4078e7 into cahilfoley:main Dec 1, 2025
1 of 3 checks passed
@cahilfoley

Copy link
Copy Markdown
Owner

Thank you for the contribution, looks great!

@ashtonisher ashtonisher deleted the feature branch December 2, 2025 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants