Skip to content
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

Restore drawing of orbits on open KeplerOrbits. #3359

Merged
merged 6 commits into from Aug 15, 2023
Merged

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Aug 9, 2023

Description

It seems that a change in the orbit code in 2021 suppressed drawing of Kepler orbits.
This should fix it. Objects on Kepler orbits are drawn orbit_good days around the epoch date.

Another thing to fix here is the removal of the entry "orbit_visualization_period" in ssystem_*.ini.
For closed orbits with special functions, this should be equal to orbit_period.
For Kepler orbits, this is governed by orbit_good, or orbital period computed from the elements.

Fixes # (issue)

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update

How Has This Been Tested?

  • Check that e.g. Hyakutake (C/1996 B1) and Hale-Bopp (C/1995 O1) show their inner orbits.
  • Load 1000_comets.ini and use SolarSystemObserver to see numerous comet orbits flash in and out of visibility as comets come and go in rapid timelapse.

Test Configuration:

  • Operating system: Windows 11
  • Graphics Card: Geforce (irrelevant)

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@gzotti gzotti added bug Something likely wrong in the code importance: high Obvious error, very annoying, but no crash labels Aug 9, 2023
@gzotti gzotti added this to the 23.3 milestone Aug 9, 2023
@gzotti gzotti added this to Needs triage in Solar System via automation Aug 9, 2023
@gzotti gzotti self-assigned this Aug 9, 2023
@gzotti gzotti added this to Needs triage in Visualization via automation Aug 9, 2023
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

@gzotti
Copy link
Member Author

gzotti commented Aug 9, 2023

It seems also closed KeplerOrbits are not plotted :-(

@gzotti
Copy link
Member Author

gzotti commented Aug 10, 2023

OK, I hope it's done.

@gzotti
Copy link
Member Author

gzotti commented Aug 15, 2023

@alex-w , @10110111 any further comments?

@gzotti gzotti moved this from Needs triage to In progress in Solar System Aug 15, 2023
@gzotti gzotti moved this from Needs triage to In progress in Visualization Aug 15, 2023
@alex-w
Copy link
Member

alex-w commented Aug 15, 2023

Sometimes orbits near perihelion is ugly - is it possible doing them smooth within current PR?

@gzotti
Copy link
Member Author

gzotti commented Aug 15, 2023

This was always ugly with discrete time intervals and highly elongated ellipses: 2nd law of Kepler...
Also, the trick for displaying the object inside the orbit when plotted ±1/2 orbit_period does not work with "open" orbits when we show the orbit inside the "orbit_good" time only, so the comet/interstellar object does not run on the orbit line. Solving these may be a future issue.

@gzotti gzotti merged commit 3dcf2cb into master Aug 15, 2023
19 checks passed
Solar System automation moved this from In progress to Done Aug 15, 2023
@gzotti gzotti deleted the fix/draw_comet_orbits branch August 15, 2023 12:14
Visualization automation moved this from In progress to Done Aug 15, 2023
@10110111
Copy link
Contributor

This was always ugly with discrete time intervals and highly elongated ellipses: 2nd law of Kepler

The same Kepler's law could be used for smarter discretization. Not necessarily numerically exactly, but at least by estimating the expected length of the orbit segment vs time slice.

@gzotti
Copy link
Member Author

gzotti commented Aug 15, 2023

Yes, I think it could work to vary eccentric anomaly with regular steps (not time, just regular angular intervals between the angles for begin and end of requested interval) and find true anomaly from that. Or vary true anomaly. Probably the KeplerOrbit class must learn to do that.

@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Sep 2, 2023
@github-actions
Copy link

github-actions bot commented Sep 2, 2023

Hello @gzotti!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Sep 26, 2023
@github-actions
Copy link

Hello @gzotti!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something likely wrong in the code importance: high Obvious error, very annoying, but no crash
Projects
Solar System
  
Done
Visualization
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants