-
-
Notifications
You must be signed in to change notification settings - Fork 919
Implemented Mollweide projection #4330
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.
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
Implemented Mollweide projection #4330
Conversation
Great PR! Please pay attention to the following items before merging: Files matching
Files matching
This is an automatically generated QA checklist based on modified files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove temporary testing data from git
For me it looks (mostly) good! Two things:
Maybe this relates to #4328, so it would add to "small/harmless gitches" if there is no trivial solution. |
This artifact is also present in other discontinuous projections, so can be
left here as well.
…On Tue, 20 May 2025, 12:36 Alexander V. Wolf, ***@***.***> wrote:
*alex-w* left a comment (Stellarium/stellarium#4330)
<#4330 (comment)>
Please check artefacts, like this:
stellarium-002.png (view on web)
<https://github.com/user-attachments/assets/3897c762-d4cc-4b21-b3d4-01071fe65dff>
—
Reply to this email directly, view it on GitHub
<#4330 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQU3MQOLXEBIBJJSIB2OKL27K5M5AVCNFSM6AAAAAB5L75462VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOJTGAYTSNBZHA>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Thanks to everyone for valuable feedback! @gzotti The visual glitches for constellation borders are indeed very annoying. I attempted to account for them with more specialized I tried handling this region specifically in the |
@dusan-prascevic please rebase the PR |
…ever some issues with display outside of the projection ellipse
the y (due to asin)
02b588f
to
90c4bb8
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
@alex-w so if I understand you correctly, on MacOS the projection ellipse is getting clipped (even at full FOV)? |
Doesn't sinusoidal projection behave the same way? I think it's inevitable when we limit the vertical FoV instead of the horizontal one, so changing aspect ratio of Stellarium window will clip the sides. |
Yes, of course! Thanks for pointing the source of "issue" |
Yes, but don't worry - this is not a bug in the projection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comparing this to Hammer-Aitoff and Sinusoidal, the glitches made by border lines at the poles also appear there. A general solution would be nice, but is not a blocker here.
The rest just works nicely, it's fine for me. A squash into one commit seems OK for me.
Why the maximum FOV is 185 degrees? |
Compare with Hammer-Aitoff and Sinusoidal. Both have 185°, meaning it just exceeds the from-pole-to-pole vertical fov. The three behave very similar in this respect. |
Wouldn't 180 degrees be more intuitive? Or is it intentionally limited to just above 180, to get both the poles visible at max FOV? |
I think it was a tradeoff that enabled a bit more of the sides to be included at some typical screen/window aspect ratios used by the person who decided on these limits. Ideally the limit should be based on horizontal FoV, or maybe on some kind of minimum or maximum of the two. |
I think the point of 185 for Hammer/Sinusoidal was having the bottom not interact with the bottom status/button bar. But it's another task to rebalance all projections for whatever new demands arise. |
Thank you everybody! |
Thanks to everyone for taking the time to review the PR and provide feedback |
Hello @dusan-prascevic! Please check the fresh version (development snapshot) of Stellarium: |
Hello @dusan-prascevic! Please check the latest stable version of Stellarium: |
Implementation of Mollweide projection (requested in #3095)
Description
Implemented Mollweide projection following the standard mathematical formulation (solving for auxiliary angle via Newton-Raphson iteration). I tried to stick to the same approach as for other projections (e.g. handling out of bounds points etc.)
Fixes # (issue) #3095
Screenshots (if appropriate):
Type of change
How Has This Been Tested?
Tested the projection visually and via added test functions in
testStelProjector.cpp
(kept the same approach as for testing the other projections)Test Configuration:
Checklist: