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.

Sign up for GitHub

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

Jump to bottom

Implement limb darkening for the Sun #3017

Merged
merged 1 commit into from
Jan 24, 2023
Merged

Implement limb darkening for the Sun #3017

merged 1 commit into from
Jan 24, 2023

Conversation

10110111
Copy link
Contributor

Description

This implements limb darkening for the Sun.

Fixes #2719

Screenshots

Original, flat solar disk

sun-flat

New, limb-darkened solar disk

sun-limb-darkened

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?

Test Configuration:

  • Operating system: Ubuntu 20.04
  • Graphics Card: Intel UHD Graphics 620

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

@github-actions github-actions bot requested review from alex-w and gzotti January 23, 2023 20:04
@github-actions
Copy link

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
gzotti reviewed Jan 23, 2023
View reviewed changes
data/shaders/planet.frag Outdated
// We are drawing the Sun
vec4 texColor = texture2D(tex, texc);
texColor.rgb = srgbToLinear(texColor.rgb * sunInfo.rgb);
// Reference: Arthur N. Cox, "Allen's Astrophysical Quantities", 4th edition,
Copy link
Member

Choose a reason for hiding this comment

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

please add year and publisher/location here or even better, add a full bib entry to the User Guide, with citation at the end of Appendix H.4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I add a bib entry, how do I refer to it here?

Copy link
Contributor Author

@10110111 10110111 Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As for the year... I have no idea what year to use, and the publisher doesn't make it easier to determine it. The copyright year? The publication year of the given format? What?

I can just add DOI or ISBN, will it be sufficient?

Copy link
Member

Choose a reason for hiding this comment

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

A short notification about visualization in the User Guide (see chapter for Solar system data) with citation of source ;) For the code the current info may be sufficient, but adding DOI or ISBN is very good addition.

Copy link
Member

Choose a reason for hiding this comment

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

No, the .bib item is referred to in the User Guide where we refer to the sources of our collected wisdom. Here the reference should at least be complete enough with year. Right now it's good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I have no idea where in the guide this citation should go, and whether it has to be hotlinked somewhere. I haven't found the "Solar system data" chapter or anything similar, so this doesn't help.
Since the citation here in the code is OK, I'm merging this PR.

gzotti
gzotti approved these changes Jan 23, 2023
View reviewed changes
Copy link
Member

@gzotti gzotti left a comment

Choose a reason for hiding this comment

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

Apart from the tiny detail, I have no objection, thank you!

@gzotti gzotti added this to the 23.1 milestone Jan 23, 2023
@gzotti gzotti added the enhancement Improve existing functionality label Jan 23, 2023
@10110111 10110111 force-pushed the solar-limb-darkening branch from 91ca578 to 9ecaa1f Compare January 24, 2023 07:43
@10110111 10110111 merged commit 6fa8ebf into Stellarium:master Jan 24, 2023
@10110111 10110111 deleted the solar-limb-darkening branch January 24, 2023 15:41
@github-actions
Copy link

Hello @10110111!

The enhancement or feature has been merged into source code and you may test it via building Stellarium from source code or wait the weekly development snapshot...

@github-actions
Copy link

Hello @10110111!

The fix has been merged into source code and you may test it via building Stellarium from source code or wait the weekly development snapshot...

@alex-w alex-w added state: published The fix has been published for testing in weekly binary package and removed state: fixed labels Mar 13, 2023
@github-actions
Copy link

Hello @10110111!

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 Mar 27, 2023
@github-actions
Copy link

Hello @10110111!

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
Reviewers

@gzotti gzotti gzotti approved these changes

@alex-w alex-w alex-w approved these changes

Assignees

@10110111 10110111

Labels
enhancement Improve existing functionality
Milestone
23.1
Development

Successfully merging this pull request may close these issues.

Solar disk lacks limb darkening
3 participants
@10110111 @alex-w @gzotti