Skip to content

Fix GravityLabels with mixed-directional labels (and higher-level Unicode)#4519

Merged
gzotti merged 5 commits intomasterfrom
fix/gravityLabels_u32
Sep 20, 2025
Merged

Fix GravityLabels with mixed-directional labels (and higher-level Unicode)#4519
gzotti merged 5 commits intomasterfrom
fix/gravityLabels_u32

Conversation

@gzotti
Copy link
Copy Markdown
Member

@gzotti gzotti commented Sep 16, 2025

Description

The multipart cultural label causes problems with gravityLabels when

  • it contains characters not covered in 16-bit strings (e.g. Cuneiform): characters missing.
  • it mixes Latin and right-to-left script like Arab: Arab characters are shown reversed

TODO:

  • split string into the isolated Unicode strings, handle each part separately.
  • Convert to u32string and process from there
  • make sure multi-part labels are RTL-fit

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
  • Housekeeping

How Has This Been Tested?

Test Configuration:

  • Operating system: Windows 11
  • Graphics Card: 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 this to the 25.4 milestone Sep 16, 2025
@gzotti gzotti self-assigned this Sep 16, 2025
@gzotti gzotti added the importance: medium A bit annoying, minor miscalculation, but no crash label Sep 16, 2025
@github-actions github-actions bot requested review from 10110111 and alex-w September 16, 2025 14:19
@github-actions
Copy link
Copy Markdown

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
Copy Markdown
Member Author

gzotti commented Sep 19, 2025

Apart from code cleanup, I think functionality is working.
@alex-w not sure, put this still into 25.3?
@kajaji is RTL for multipart labels correct?

@gzotti gzotti marked this pull request as ready for review September 19, 2025 13:40
@gzotti gzotti force-pushed the fix/gravityLabels_u32 branch from 4e45441 to 2d38a48 Compare September 19, 2025 16:22
@alex-w
Copy link
Copy Markdown
Member

alex-w commented Sep 20, 2025

@alex-w not sure, put this still into 25.3?

Currently 25.4...

@gzotti
Copy link
Copy Markdown
Member Author

gzotti commented Sep 20, 2025

I know. I did not expect to have a solution by yesterday. A few comments can still be cleaned up, but else it seems to work OK. It also does not provide new translatable strings.

@alex-w alex-w modified the milestones: 25.4, 25.3 Sep 20, 2025
@gzotti gzotti force-pushed the fix/gravityLabels_u32 branch from 4624ca3 to 2277f4a Compare September 20, 2025 18:58
@gzotti gzotti moved this from Backlog to In review in Skycultures 2.0 Sep 20, 2025
@gzotti gzotti merged commit a768b41 into master Sep 20, 2025
29 of 31 checks passed
@gzotti gzotti deleted the fix/gravityLabels_u32 branch September 20, 2025 21:22
@github-project-automation github-project-automation bot moved this from In review to Done in Skycultures 2.0 Sep 20, 2025
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Sep 22, 2025
@github-actions
Copy link
Copy Markdown

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 29, 2025
@github-actions
Copy link
Copy Markdown

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

importance: medium A bit annoying, minor miscalculation, but no crash

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants