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

Fix stellar appearance of lunar occultation #1959

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Oct 9, 2021

DO NOT MERGE!

Description

Currently stars are drawn as "halo textures". When occulted by the Moon (or other planet, but it's most obvious here!), a star should instantly vanish (or light up again) when the moon covers its centre.

The current attempt here is a way not to do it, though! I tried to set up a moon-sized SphericalCap and exclude stars from rendering. This comes close, but as the Moon is rendered as tessellated sphere (polygonal mesh), it may appear too small by a few arcseconds.

The real way to go would probably involve rendering the Moon first with a stencil buffer, then drawing all stars except those hidden by the stencil. It may even be useful to draw the complete SolarSystem with stencil buffer before rendering the stars, to catch the effect in all cases.

Thinking of it, we use a Z buffer already to mix planets and moons. So, just draw solar system before stars, and test Z buffer for star rendering.

Any OpenGL experts here to take over?

Fixes #1924

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?

Test Configuration:

  • Operating system: Windows 10
  • Graphics Card: Nvidia Geforce 960M

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
  • 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 enhancement Improve existing functionality help wanted We may not have the hardware or expertise importance: low Small problem, rarely visible, no crash labels Oct 9, 2021
@todo
Copy link

todo bot commented Oct 9, 2021

This is a bad test. We should draw the moon into a stencil buffer and check that.

// TODO: This is a bad test. We should draw the moon into a stencil buffer and check that.
if (filterMoon)
{
if (moonCap.contains(vf))
continue;
}


This comment was generated by todo based on a TODO comment in 94f6bd5 in #1959. cc @Stellarium.

@github-actions

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Oct 9, 2021

Hello @gzotti! Thank you for this enhancement.

@guillaumechereau
Copy link
Contributor

guillaumechereau commented Oct 27, 2021

I think the computation of the cap using atan2 is not correct, because the tangent points of intersection to the edge of the Moon generate a circle of radius slightly smaller than the radius of the Moon (see the attached diagram). I think the correct formula should be:

angularSize = asin(moonRadius / moonPos.lenght())

At the distance of the Moon this should barely make a difference, but could explain the small errors in the current code.

image

@gzotti
Copy link
Member Author

gzotti commented Oct 27, 2021

Yes, indeed. This was taken from Planet::getAngularSize() (so I fixed that also there), but for the Moon the difference is most apparent.
However, the fix makes the problem even worse, as the visible diameter of the cap (blue angle) grows even bigger. I see stars vanish at 0...6" from the lunar edge, on both sides of the moon, so either the cap's or the moon's polygonal edge (or rather both) don't match in a way that this approach would work nicely.

@github-actions github-actions bot added the has conflicts The pull request has conflicts label Nov 13, 2021
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

- does not work properly: tessellated Moon sphere is often smaller than its computed radius!
@github-actions
Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the has conflicts The pull request has conflicts label Jan 22, 2023
@gzotti
Copy link
Member Author

gzotti commented Jan 22, 2023

@10110111 maybe you can look into this one? (See also #1924).

The solution attempted here is not good, as the spherical cap misses the lunar edge by potentially a few pixels.

Should we invert plotting solarsystem and stars and plot stars when their locations are not covered in the Z buffer? (Caveat: semitransparent rings of Saturn and Uranus) The "overbleeding" effect mentioned by @guillaumechereau in #1924 can indeed even look better than covering part of the star "halo" by the Moon.

Attn, this may also need to change drawing the other object classes from various plugins (QSR, PSR, SN, ...)

Other ideas?

@10110111
Copy link
Contributor

Ideally, I'd go a very unusual (and longish) route to solve this: render the whole scene into a floating-point framebuffer in physical units of brightness (i.e. with correct relative brightness, sizes, and without halos), and then apply a glare effect, like e.g. a Gaussian-blurred version on top of the initial render.

All the other ways like using the stencil buffer, occlusion queries etc. for solving this are just workarounds for a rendering pipeline that doesn't make much physical sense.

@gzotti
Copy link
Member Author

gzotti commented Jan 22, 2023

Sounds interesting, but isn't this too much for the smaller systems? I mean, the only effect we want to avoid here is the gradual visibility of the star halos as long as the actual star is hidden. The non-stars should not have a glare effect added as this visually only blurs the image, and the test for covering will test the pinpoint star position (1 pixel) only. This should also work with the geometric Moon model you showed elsewhere where we could see the mountains along the Lunar edge.

I wonder about the true cause of the current mismatch. Is this only the tessellation (inscribed polygonal edge vs. circle) mismatch or is there another reason?

@10110111
Copy link
Contributor

Sounds interesting, but isn't this too much for the smaller systems?

Might be too much for Raspberry indeed.

I mean, the only effect we want to avoid here is the gradual visibility of the star halos as long as the actual star is hidden.

I also want to replace the fake glare of the Moon and the Sun, which become incorrect in various circumstances like e.g. upscaled the Sun or crescent Moon. This is why I prefer this solution that would automatically solve these problems.

The non-stars should not have a glare effect added as this visually only blurs the image

If done correctly, this won't blur the image. The blurring kernel is supposed to have a very low-amplitude tail, which will only become noticeable on very bright targets (which the stars should be I think, otherwise they wouldn't look so fat).

I wonder about the true cause of the current mismatch.

The mismatch where? In this PR? I didn't look much into it, but the tesselation should certainly introduce a mismatch between calculation against a sphere vs rendering.

@gzotti
Copy link
Member Author

gzotti commented Jan 22, 2023

Sounds interesting, but isn't this too much for the smaller systems?

Might be too much for Raspberry indeed.

If it else works everywhere (Intel?), it could also become a switchable option. (classic star halo/spikes vs. modern: glare simulation)

I mean, the only effect we want to avoid here is the gradual visibility of the star halos as long as the actual star is hidden.

I also want to replace the fake glare of the Moon and the Sun, which become incorrect in various circumstances like e.g. upscaled the Sun or crescent Moon. This is why I prefer this solution that would automatically solve these problems.

OK. Replacing the large "halo" textures would also be fine.

The non-stars should not have a glare effect added as this visually only blurs the image

If done correctly, this won't blur the image. The blurring kernel is supposed to have a very low-amplitude tail, which will only become noticeable on very bright targets (which the stars should be I think, otherwise they wouldn't look so fat).

OK. But consider generalisation or extensions to have some spiky pattern as well. Happy to see a technically advanced solution to this.

I wonder about the true cause of the current mismatch.

The mismatch where? In this PR? I didn't look much into it, but the tesselation should certainly introduce a mismatch between calculation against a sphere vs rendering.

Yes, if you build this PR you see that stars often vanish just outside the Lunar edge. I suspect that this is just because the tessellated Moon's circumference is an n-gon inscribed into the "spherical cap" used for testing occultation that should have the same geometrical diameter, which is why this solution does not work well.

gzotti referenced this pull request Jul 26, 2023
- also docfix
- make option immediately permanent
@github-actions github-actions bot added the has conflicts The pull request has conflicts label Dec 24, 2023
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@ElonVampire
Copy link

ElonVampire commented Dec 24, 2023 via email

@github-actions github-actions bot removed the has conflicts The pull request has conflicts label Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality help wanted We may not have the hardware or expertise importance: low Small problem, rarely visible, no crash
Development

Successfully merging this pull request may close these issues.

Don't draw stellar halos if the star is obscured
4 participants