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

Satellites: Restore shadow circle at arbitrary altitude #3538

Merged
merged 4 commits into from
Dec 9, 2023

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Dec 8, 2023

Description

The functionality to show a shadow limit for a hypothetical satellite in arbitrary altitude above ground was botched and therefore disabled in 23.1. This now aims to fix that.

  • also resets cutoff altitude to 80km
  • also no commLink info for nonoperational satellites.

As discussed: We had envisioned shadow enlargement like for Lunar eclipses. However, the original evaluation for visibility conditions does not use shadow enlargement. I have removed the according code.

Fixes #3100 (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
  • Housekeeping

How Has This Been Tested?

Test Configuration:

  • Operating system: Win11
  • Graphics Card: Geforce 3700Ti

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

- also resets cutoff to 80km
- also no commLink info for nonoperational satellites.
@gzotti gzotti added the feature Entirely new feature label Dec 8, 2023
@gzotti gzotti added this to the 23.4 milestone Dec 8, 2023
@gzotti gzotti self-assigned this Dec 8, 2023
@github-actions github-actions bot requested review from 10110111 and alex-w December 8, 2023 23:24
@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

However, the original evaluation for visibility conditions does not use shadow enlargement. Either remove the code, or add it to the original model?

I think the code for shadow enlargement should be at least disabled or removed. In the future we need improve brightness model for satellites in penumbra and we need a real observations to obtain info for accuracy of our umbra/penumbra for sats.

alex-w
alex-w previously approved these changes Dec 9, 2023
@alex-w alex-w dismissed their stale review December 9, 2023 06:29

Stop! The planetarium is crashed at exit when umbra is visible.

@gzotti
Copy link
Member Author

gzotti commented Dec 9, 2023

I see no crash at exit in about 10 runs. Can you be a bit more specific?

@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

I see no crash at exit in about 10 runs. Can you be a bit more specific?

Steps to reproduce:

  1. Run Stellarium
  2. Open Satellites settings and enable the umbra
  3. Select ISS
  4. Quit from Stellarium
Downloaded 0 files (0 kbytes) in a session of 49.06 sec (average of 0 kB/s + 0 files from cache (0 kB)).
LandscapeMgr: Clearing cache of 0 landscapes totalling about  0 MB.
StelGLWidget destroyed
[Thread 0x7fffd5476640 (LWP 55222) exited]
[Thread 0x7fffdd1ff640 (LWP 55144) exited]
[Thread 0x7fff5a111640 (LWP 55224) exited]
[Thread 0x7fff59910640 (LWP 55228) exited]
[Thread 0x7fffd4c75640 (LWP 55223) exited]
[Thread 0x7fffdc9fe640 (LWP 55145) exited]
[Thread 0x7fffe59fe640 (LWP 55140) exited]

Thread 1 "stellarium" received signal SIGSEGV, Segmentation fault.
0x0000000000000031 in ?? ()

@gzotti
Copy link
Member Author

gzotti commented Dec 9, 2023

Still cannot reproduce it on W11. Stellarium on my WSL currently would use the Intel GPU but segfaults on startup. Due to other tasks I cannot reboot now. Can you run a Debug build and check where it breaks?

@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

Still cannot reproduce it on W11. Stellarium on my WSL currently would use the Intel GPU but segfaults on startup. Due to other tasks I cannot reboot now. Can you run a Debug build and check where it breaks?

I've shared log from gdb, so, the main problem - I see the segmentation fault, but I see no source. Something strange happened in new getUmbraData method.

@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

Oops... I've checked the master - same steps and same results - a crash O_O

So, your code is not a bad guy

@alex-w alex-w self-requested a review December 9, 2023 11:24
@10110111
Copy link
Contributor

10110111 commented Dec 9, 2023

Thread 1 "stellarium" received signal SIGSEGV, Segmentation fault.
0x0000000000000031 in ?? ()

If it's 100% reproducible, try running under Valgrind. It may be able to tell where the bad jump/call target address came from.

@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

If it's 100% reproducible, try running under Valgrind. It may be able to tell where the bad jump/call target address came from.

Hmm...

master, debug mode, gdb:

Downloaded 0 files (0 kbytes) in a session of 34.195 sec (average of 0 kB/s + 0 files from cache (0 kB)).
LandscapeMgr: Clearing cache of 0 landscapes totalling about  0 MB.
StelGLWidget destroyed
[Thread 0x7fffdd413640 (LWP 122542) exited]
[Thread 0x7fffddc14640 (LWP 122541) exited]
[Thread 0x7fffdf298640 (LWP 122539) exited]
[Thread 0x7fffdcc12640 (LWP 122543) exited]
[Thread 0x7fff6f0fc640 (LWP 122566) exited]
[Thread 0x7fff700fe640 (LWP 122564) exited]
[Thread 0x7fff708ff640 (LWP 122563) exited]
[Thread 0x7fffdfba5ec0 (LWP 122534) exited]
[Thread 0x7fff6f8fd640 (LWP 122565) exited]
[New process 122534]
[Inferior 1 (process 122534) exited normally]

Am I an idiot?

OK, this branch, debug mode, gdb:

Downloaded 0 files (0 kbytes) in a session of 47.111 sec (average of 0 kB/s + 0 files from cache (0 kB)).
LandscapeMgr: Clearing cache of 0 landscapes totalling about  0 MB.
malloc(): unsorted double linked list corrupted

Thread 1 "stellarium" received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140736946921152) at ./nptl/pthread_kill.c:44
44      ./nptl/pthread_kill.c: Нет такого файла или каталога.

Next iteration:

LandscapeMgr: Clearing cache of 0 landscapes totalling about  0 MB.
StelGLWidget destroyed
[Thread 0x7fffddc14640 (LWP 135903) exited]
[Thread 0x7fffdd413640 (LWP 135904) exited]
[Thread 0x7fffdf298640 (LWP 135901) exited]
[Thread 0x7fffdcc12640 (LWP 135905) exited]
[Thread 0x7fff6f8fd640 (LWP 135931) exited]
[Thread 0x7fff700fe640 (LWP 135930) exited]
[Thread 0x7fff708ff640 (LWP 135929) exited]
[Thread 0x7fffdfba5ec0 (LWP 135897) exited]
[Thread 0x7fff6f0fc640 (LWP 135932) exited]
[New process 135897]
[Inferior 1 (process 135897) exited normally]

WTF?!?

@10110111
Copy link
Contributor

10110111 commented Dec 9, 2023

malloc(): unsorted double linked list corrupted

Well, this looks like that same intermittent crash that we've already seen and failed to diagnose.

@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

this branch, debug mode, gdb

  1. Run Stellarium
  2. Change time to 3 hours early (a daylight on the screen)
  3. Open settings Satellites plugin
  4. Umbra and fixed altitude enabled
  5. Quit
Downloaded 0 files (0 kbytes) in a session of 51.29 sec (average of 0 kB/s + 0 files from cache (0 kB)).
LandscapeMgr: Clearing cache of 0 landscapes totalling about  0 MB.

Thread 1 "stellarium" received signal SIGSEGV, Segmentation fault.
0x0000555555e035e3 in std::__atomic_base<int>::operator++ (this=0x6c006c00650074) at /usr/include/c++/11/bits/atomic_base.h:385
385           { return __atomic_add_fetch(&_M_i, 1, int(memory_order_seq_cst)); }

@10110111
Copy link
Contributor

10110111 commented Dec 9, 2023

What's the stack trace?

@alex-w
Copy link
Member

alex-w commented Dec 9, 2023

Next iteration in same steps:

LandscapeMgr: Clearing cache of 0 landscapes totalling about  0 MB.
StelGLWidget destroyed
[Thread 0x7fffdf298640 (LWP 138397) exited]

Thread 1 "stellarium" received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()

@gzotti
Copy link
Member Author

gzotti commented Dec 9, 2023

Ah, now running debug mode build in debug run. Exited, and was pointed to an issue in a file atomic.
image

Debugger shows this function (with mark arrow)

  _NODISCARD _TVal load(const memory_order _Order) const noexcept { // load with given memory order
        const auto _Mem = _Atomic_address_as<int>(_Storage);
-->     auto _As_bytes  = __iso_volatile_load32(_Mem);
        _Load_barrier(_Order);
        return reinterpret_cast<_TVal&>(_As_bytes);
    }

I admit I have seriously no idea where this comes from. But such error in "atomic" was also the error when I gave up on the latest discussion about TelescopeControl.

Is the call display helpful:
image

Anything problematic in removing a StelTexture?

@gzotti gzotti marked this pull request as ready for review December 9, 2023 12:47
@github-actions github-actions bot requested a review from alex-w December 9, 2023 12:47
@10110111
Copy link
Contributor

10110111 commented Dec 9, 2023

I suppose we need to make a separate issue for tracking this crash. I've caught it once now in release mode run under Valgrind, with the same stack trace (in master). Apparently something is going wrong while deleting StelTextureMgr. From your stack trace it looks like StelTextureMgr::idMap was being deleted when the crash occurred.

@gzotti gzotti merged commit e4d9cc8 into master Dec 9, 2023
17 of 20 checks passed
@gzotti gzotti deleted the fix/Sat-shadow-circle branch December 9, 2023 13:58
@gzotti
Copy link
Member Author

gzotti commented Dec 9, 2023

Maybe this is #3427 after all?

@10110111 10110111 mentioned this pull request Dec 9, 2023
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Dec 10, 2023
Copy link

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 Dec 23, 2023
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
feature Entirely new feature
Development

Successfully merging this pull request may close these issues.

sat plugin: umbra at "distance" (should be "altitude") does not correspond to selected sat's umbra
3 participants