search

OpenPHDGuiding / phd2

PHD2 Guiding
https://openphdguiding.org
BSD 3-Clause "New" or "Revised" License
249 stars 114 forks source link

Star profile layout tweaks #1163

Closed Eyeke2 closed 8 months ago

Eyeke2 commented 8 months ago

A set of commits fixing few scaling factors used to prevent HFD metrics clipping at the window edges or by other window elements, enhancing accuracy of displayed star centroid, clearing previously uninitialized state variables - all resulting in a cleaner UI and predictable behavior.

Eyeke2 commented 8 months ago

would be great to see some pictures in the pr description of what this would look like

Sure, I'm attaching first a screenshot from official PHD2 v2.6.13dev1 using ASCOM Sky Simulator. old

Eyeke2 commented 8 months ago

Here is the screenshot after applying PR (I'm using my custom version of PHD2 with this PR integrated). It's hard to set the same screen gamma values, as I'm using different PHD2 profiles, but generally it shows a bit more sharpness. The HFD fonts will be generally bigger with this PR as it utilizes all available horizontal space. In this example this difference is not so prominent, I can new add more screenshots to demonstrate what I mean. In my view, bigger letters in HFD are easier to see from a distance.

Eyeke2 commented 8 months ago

Also, you can pay attention that after applying this PR, the star profile shows star not as bloated as in the original version (result of applying the scaling factor).

Eyeke2 commented 8 months ago

Sorry for mixing up the optimization PR and Star Profile PR. I will provide images specific for this PR a bit later.

Eyeke2 commented 8 months ago

Here are two screenshots showing before and after. Please pay attention where the star image and the small red crosshairs are located on both images (in fact, due to scaling bug you won't see the star at all in the image before the fix, just the red crosshairs way below the star profile display area closer to FWHM metrics). When window area is rather small, the PR will make the HFD font smaller but it won't crop it. old-star-profile1 new-star-profile1

Eyeke2 commented 8 months ago

Another pair of images showing a larger Star Profile window where HFD metrics font is clearly larger after PR and it's also displayed in front of the star image and not obscured by it as in the image before the PR. old-star-profile2 new-star-profile2

bwdev01 commented 8 months ago

It’s ok AFAICT but I would like to look at it after the merge but before we do a release.

Bruce

From: Andy Galasso @.> Sent: Saturday, January 27, 2024 1:03 PM To: OpenPHDGuiding/phd2 @.> Cc: bwdev01 @.>; Mention @.> Subject: Re: [OpenPHDGuiding/phd2] Star profile fixes (PR #1163)

@agalasso approved this pull request.

thanks for the screenshots. nice work!

@bwdev01 https://github.com/bwdev01 any concerns? If not I think we should merge this

— Reply to this email directly, view it on GitHub https://github.com/OpenPHDGuiding/phd2/pull/1163#pullrequestreview-1847281889 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDHSV4S2BDR4ZC2UVEXWXTYQVTRFAVCNFSM6AAAAABCMA7BC6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNBXGI4DCOBYHE . You are receiving this because you were mentioned. https://github.com/notifications/beacon/ADDHSV7DN2FUL7D2IUCJUX3YQVTRFA5CNFSM6AAAAABCMA7BC6WGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTTODNEOC.gif Message ID: @. @.> >

Eyeke2 commented 8 months ago

Sorry for misleading comment about star not visible in the Star Profile window. It just drifted away (due to star field simulator behavior) faster than on the fixed version. However, I'll post two new images showing side by side comparative screenshots which were snipped after equal time running the simulation. You can notice that as farther away the star profile image will be from the center, the less accurate will be position of the red crosshairs mark - with the original PHD2 version. The star also looks different due to a missing scaling factor. The fixed version shows star image properly scaled and the red crosshairs is marking star's center precisely due to fixing another bug related to the code drawing the crosshairs. FYI. old new

Eyeke2 commented 8 months ago

@agalasso I think I will create one more PR to limit the size of the large font in the Star Profile window. Without an upper limit of its size, the large font can be too overwhelming and conceal the star image. I'm suggesting you try running it by yourself and let me know what you think.