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

Tweak screen intersect callback #3059

Merged
merged 6 commits into from
Mar 5, 2023
Merged

Tweak screen intersect callback #3059

merged 6 commits into from
Mar 5, 2023

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Feb 14, 2023

This is a small correction to the screen intersect callback to label SkyLines. It shall avoid loss of a few characters which are shifted out of the actual screen area by shallow angles.

There are two possible ways. Shifting the text, or changing the placement position. Currently it seems the first is enough, but I have left the other way commented out.

Please TEST!

Description

Fixes #2955 (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: Win11
  • Graphics Card: Geforce

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

- avoid text intersection
@gzotti gzotti added the enhancement Improve existing functionality label Feb 14, 2023
@gzotti gzotti added this to the 23.1 milestone Feb 14, 2023
@gzotti gzotti self-assigned this Feb 14, 2023
@github-actions github-actions bot requested review from 10110111 and alex-w February 14, 2023 11:09
@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
Copy link
Member Author

gzotti commented Feb 14, 2023

Upward curves in bottom right edge are still not OK.

@gzotti gzotti marked this pull request as ready for review February 14, 2023 12:47
@10110111
Copy link
Contributor

In the following screenshot, the 75.0000° at the top (two copies to the left and right of the horizontal center) are still clipped. Also, they seem to be too far from the line that they denote (or do I assign them to the wrong line?).

stellarium-013

Similarly in the following screenshot, the 200.0000° on the left border is clipped, and the same about 10.0000° (?) in the lower half of the image. Also notice the 40.0000° at the top in the left half.

stellarium-014

@gzotti
Copy link
Member Author

gzotti commented Feb 14, 2023

The solution may not be perfect, but certainly better than before.

It seems the text angles along the extremes, just where the text would be shifted a lot, come in discrete steps, so that the labels appear detached from their lines, but just at the very edge of visibility.

Maybe I can add a minimum angle of applicability to avoid this thing:
image

-also remove second approach
@gzotti
Copy link
Member Author

gzotti commented Mar 5, 2023

@alex-w @10110111 please also test this one. As long as I don't place the labels completely on the other side of the line (which I don't intend to do), a few pixels may still be clipped off, but IMO the situation is much better now.

Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Not complete solution, but acceptable

Copy link
Contributor

@10110111 10110111 left a comment

Choose a reason for hiding this comment

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

OK, at least the clipping is not as severe as it used to be, although text placement and orientation still leaves a lot to be desired.

@gzotti gzotti merged commit 781724d into master Mar 5, 2023
@gzotti gzotti deleted the fix/edge-callback-labels branch March 5, 2023 12:39
@github-actions
Copy link

Hello @gzotti!

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 @gzotti!

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 @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 Mar 27, 2023
@github-actions
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
enhancement Improve existing functionality
Development

Successfully merging this pull request may close these issues.

Text of coordinates is clipped at the top of the screen
3 participants