Nomenclature plot fixes #3342

gzotti · 2023-07-25T14:46:37Z

Description

This introduces an option to encircle craters and satellite features (which are usually also craters) with ellipses.

I also just slightly simplified the code.

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?

Test Configuration:

Operating system: Windows 11
Graphics Card: Geforce 3070Ti
Qt 6.5.1

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 · 2023-07-25T14:47:30Z

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.

alex-w · 2023-07-25T15:09:43Z

Hmm... It's interesting feature, but IMHO ellipses have troubles for scale factor:

10110111 · 2023-07-25T15:30:44Z

When I enable craters outline, I get the following repeated messages in the log:

Distance greater 1
Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
Distance greater 1
Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
Distance greater 1
Distance greater 1
Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

Distance greater 1
Distance greater 1
Distance greater 1
/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

/home/ruslan/src/stellarium/src/core/modules/AtmosphereShowMySky.cpp:789: OpenGL error: 1281 (GL_INVALID_VALUE)

gzotti · 2023-07-25T15:36:49Z

Hmm... It's interesting feature, but IMHO ellipses have troubles for scale factor:...

Not sure, do you mean display scaling? Planet scale factor (like 4x for the moon) works for me.

10110111 · 2023-07-25T15:42:33Z

Not sure, do you mean display scaling?

I experience the same issue with HiDPI scaling of 150%. If I set 100%, the size problem disappears.

gzotti · 2023-07-25T15:51:38Z

I also see this in master:

D:\StelDev\GIT\stellarium\src\core\StelApp.cpp:910: OpenGL error: 1281 (GL_INVALID_VALUE)

10110111 · 2023-07-25T15:55:20Z

I also see this in master

Constantly repeating?

gzotti · 2023-07-25T16:12:22Z

Yes. I only have excluded a few plugins from building.

$ git log
commit a79ca04 (HEAD -> master, origin/master)

alex-w · 2023-07-25T16:16:08Z

Hmm... It's interesting feature, but IMHO ellipses have troubles for scale factor:...

Not sure, do you mean display scaling? Planet scale factor (like 4x for the moon) works for me.

Please show screenshot with outlines, but without scaling the Moon

gzotti · 2023-07-25T16:40:06Z

alex-w · 2023-07-25T16:45:07Z

See screenshot from macOS (HiDPI monitor, device pixel ratio: 2):

alex-w · 2023-07-25T17:13:38Z

OK, I see pixel ratio correction is applied twice - one time in StelPainter::drawEllipse() method and one time in NomenclatureItem::draw() method (see obtaining screenRadius by the fact). Please see tentative fix.

Plus I see flashing outlines for craters near lunar limb :(

10110111 · 2023-07-25T17:18:07Z

Yes, with this fix it's now better.

The option to outline craters need to be disabled when nomenclature is off, for consistency with other options that get disabled.

alex-w · 2023-07-25T17:41:17Z

If tentative fix is acceptable, then only flashes near limb need to be fixed

gzotti · 2023-07-26T00:30:20Z

Thanks for the hints. The flashing is better, but it seems some flicker remains due to rounding effects.

gzotti · 2023-07-26T09:13:55Z

I still have a problem to define the rotation angle in antiquity.

gzotti · 2023-07-26T13:34:51Z

I now exclude all ellipses which would extend over the limb. I think it's the best possible solution with that approach. Mare Orientale and other possible "large tangent disks" will hopefully always be excluded with that.

alex-w · 2023-07-26T13:58:28Z

Probably GUI option should be renamed now (Outline circles?) and tooltip with short description for this option really need.

10110111 · 2023-07-26T14:35:35Z

src/core/modules/NomenclatureMgr.cpp

+void NomenclatureMgr::setFlagOutlineCraters(bool b)
+{
+	NomenclatureItem::flagOutlineCraters = b;
+	conf->setValue("astro/flag_planets_nomenclature_outline_craters", b);


This immediate saving is inconsistent with the rest of the options here (except the terminator ones, which are also inconsistent with the rest).

Ops... I agree

I know. I like the immediate storage of details much better than having to remember whether I have made any other detail changes before I save all options. Immediate here are color, terminator, solar altitudes, and now circles. The global (not-immediate) option is nomenclature or not. But indeed I would like to have many more immediate-store settings.

This inconsistency is very confusing.

Immediate storage of details is very bad for educational purposes (esp. for using Stellarium in classes) - it's good for personal uses only. Please move saving option into ConfigurationDialog class.

Or, at least, we should add support profiles - #439

I fully disagree. The proposed solution is consistent with storing the three details about terminator visibility. I would rather add immediate mode for "show special" and "hide local" for local consistency.
The global "store settings" is super old and frequently causes confusion. But probably we need yet another global program option for immediate vs. deferred storage.

- Tried craters as circles, but need ellipse. - When time is not running, it seems returning from moon to earth, moon craters are distributed over the sky.

- leave diagnostic message commented away.

- also docfix - make option immediately permanent

- add immediate config storage mode for all detail settings of NomenclatureItems

github-actions · 2023-09-02T14:19:21Z

Hello @gzotti!

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

github-actions · 2023-09-26T03:02:14Z

Hello @gzotti!

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

gzotti added the enhancement Improve existing functionality label Jul 25, 2023

gzotti added this to the 23.3 milestone Jul 25, 2023

gzotti self-assigned this Jul 25, 2023

gzotti added this to Needs triage in Visualization via automation Jul 25, 2023

github-actions bot requested review from 10110111 and alex-w July 25, 2023 14:46

alex-w approved these changes Jul 26, 2023

View reviewed changes

Visualization automation moved this from Needs triage to In progress Jul 26, 2023

10110111 reviewed Jul 26, 2023

View reviewed changes

gzotti mentioned this pull request Aug 2, 2023

Fix: orbit details #3348

Merged

13 tasks

gzotti added 2 commits August 6, 2023 10:50

A few tiny tests.

5a7ee31

- Tried craters as circles, but need ellipse. - When time is not running, it seems returning from moon to earth, moon craters are distributed over the sky.

Draw crater type NomenclatureItems as ellipse

83e4fa8

gzotti and others added 10 commits August 6, 2023 10:50

Avoid drawing ellipses for craters behind Lunar limb

82e0296

- leave diagnostic message commented away.

tentative fix for HiDPI devices

c1b406b

visual fix

8dcd222

Avoid flickering of borderline ellipses

af7657f

- also docfix - make option immediately permanent

Fix angle for dates off J2000

7e9f583

Further exclusion for overshooting ellipses

11d3264

SUG: Describe new features

8bc3c9f

add tooltip

35de3a3

Improve usability

43fe83c

- add immediate config storage mode for all detail settings of NomenclatureItems

Use optional immediateSave for Nomenclature detail settings

834d702

gzotti force-pushed the gz_NomenclaturePlotFixes branch from e99a0f6 to 834d702 Compare August 6, 2023 09:05

alex-w approved these changes Aug 6, 2023

View reviewed changes

Cosmetics

5ae169b

gzotti merged commit d677f1e into master Aug 6, 2023
18 of 20 checks passed

Visualization automation moved this from In progress to Done Aug 6, 2023

gzotti deleted the gz_NomenclaturePlotFixes branch August 6, 2023 11:41

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nomenclature plot fixes #3342

Nomenclature plot fixes #3342

gzotti commented Jul 25, 2023

github-actions bot commented Jul 25, 2023

alex-w commented Jul 25, 2023

10110111 commented Jul 25, 2023

gzotti commented Jul 25, 2023

10110111 commented Jul 25, 2023

gzotti commented Jul 25, 2023

10110111 commented Jul 25, 2023

gzotti commented Jul 25, 2023

alex-w commented Jul 25, 2023

gzotti commented Jul 25, 2023

alex-w commented Jul 25, 2023

alex-w commented Jul 25, 2023

10110111 commented Jul 25, 2023

alex-w commented Jul 25, 2023

gzotti commented Jul 26, 2023

gzotti commented Jul 26, 2023

gzotti commented Jul 26, 2023

alex-w commented Jul 26, 2023

10110111 Jul 26, 2023

alex-w Jul 26, 2023

gzotti Jul 26, 2023

10110111 Jul 26, 2023

alex-w Jul 26, 2023

gzotti Jul 26, 2023

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 26, 2023

Nomenclature plot fixes #3342

Nomenclature plot fixes #3342

Conversation

gzotti commented Jul 25, 2023

Description

Screenshots (if appropriate):

Type of change

How Has This Been Tested?

Checklist:

github-actions bot commented Jul 25, 2023

alex-w commented Jul 25, 2023

10110111 commented Jul 25, 2023

gzotti commented Jul 25, 2023

10110111 commented Jul 25, 2023

gzotti commented Jul 25, 2023

10110111 commented Jul 25, 2023

gzotti commented Jul 25, 2023

alex-w commented Jul 25, 2023

gzotti commented Jul 25, 2023

alex-w commented Jul 25, 2023

alex-w commented Jul 25, 2023

10110111 commented Jul 25, 2023

alex-w commented Jul 25, 2023

gzotti commented Jul 26, 2023

gzotti commented Jul 26, 2023

gzotti commented Jul 26, 2023

alex-w commented Jul 26, 2023

10110111 Jul 26, 2023

Choose a reason for hiding this comment

alex-w Jul 26, 2023

Choose a reason for hiding this comment

gzotti Jul 26, 2023

Choose a reason for hiding this comment

10110111 Jul 26, 2023

Choose a reason for hiding this comment

alex-w Jul 26, 2023

Choose a reason for hiding this comment

gzotti Jul 26, 2023

Choose a reason for hiding this comment

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 26, 2023