Skip to content

Update cart2polar.glsl - #236

Closed
jane00 wants to merge 2 commits into
patriciogonzalezvivo:mainfrom
jane00:main
Closed

Update cart2polar.glsl#236
jane00 wants to merge 2 commits into
patriciogonzalezvivo:mainfrom
jane00:main

Conversation

@jane00

@jane00 jane00 commented Dec 7, 2024

Copy link
Copy Markdown
Contributor

Add description for convention changed.

Add by jane00
Refer to https://en.wikipedia.org/wiki/Spherical_coordinate_system , there are two different definition for phi and theta, which are opposite to each other.

Before the changes by shadielhajj committed on 2024 Jul 26, The physics convention(ISO convention) is followed, phi means "azimuthal angle", theta for "polar angle"
but now, The "mathematics convention" is followed, phi means "polar angle", theta for "azimuthal angle"

So be careful with the old code !!!

Add description for convention changed.
@patriciogonzalezvivo

Copy link
Copy Markdown
Owner

@jane00 feels like this description belongs to the 'description' section on the YAML formatted header (in comments). And should be added to: 'cart2polar.glsl', 'cart2polar.hlsl', 'cart2polar.msl' and also 'polar2cart.glsl', 'polar2cart.hlsl' and 'polar2cart.msl'

@jane00

jane00 commented Dec 10, 2024

Copy link
Copy Markdown
Contributor Author

@jane00 feels like this description belongs to the 'description' section on the YAML formatted header (in comments). And should be added to: 'cart2polar.glsl', 'cart2polar.hlsl', 'cart2polar.msl' and also 'polar2cart.glsl', 'polar2cart.hlsl' and 'polar2cart.msl'

Sorry, I'm not very familiar with the correct format conventions, so my submission might not be very standardized. However, I happened to use this function. I originally wanted to add a "vec3 polar2cart(in vec3 polar)" function myself, but I found that the new version already includes it. Then I noticed the definition had changed, so I added a note about it.

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