-
Notifications
You must be signed in to change notification settings - Fork 118
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
Image scale change improvements #1260
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/star.cpp
…nor changes in pixel size or focal length
agalasso
reviewed
Nov 30, 2024
Good point. This was originally a private static function in camera.cpp but I later promoted it to advanced_dialog.cpp and neglected to think about the other use-cases. I just used your suggested change so I went ahead and merged the PR without asking for another review.
Thanks.
From: Andy Galasso ***@***.***>
Sent: Friday, November 29, 2024 7:40 PM
To: OpenPHDGuiding/phd2 ***@***.***>
Cc: bwdev01 ***@***.***>; Author ***@***.***>
Subject: Re: [OpenPHDGuiding/phd2] Image scale change improvements (PR #1260)
@agalasso commented on this pull request.
_____
In src/advanced_dialog.cpp <#1260 (comment)> :
@@ -592,6 +592,21 @@ double AdvancedDialog::DetermineGuideSpeed()
}
return sidRate;
}
+
+// Floating point equality comparisons can be a problem due to rounding and trivial inequalities.
+// PercentChange can be used to see if a change is worth reacting to
+double AdvancedDialog::PercentChange(double oldVal, double newVal)
+{
+ double chg;
+ if (oldVal > 0)
+ {
+ chg = fabs(1.0 - newVal / oldVal);
+ }
+ else
+ chg = fabs(newVal);
hmm, the negative case (oldVal < 0) seems odd -- probably won't happen in this pr but could be an unexpected result if somebody else wants to use this method. how about something like this:
if (fabs(oldVal) < 0.0001)
return 100. * newVal; // not meaningful, but avoids divide by zero
return 100. * fabs(1. - newVal / oldVal);
—
Reply to this email directly, view it on GitHub <#1260 (review)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADDHSV2EZO6OCD3SBDEVGYT2DEXP7AVCNFSM6AAAAABSR47S2WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINZQG4YDENBVHE> .
You are receiving this because you authored the thread. <https://github.com/notifications/beacon/ADDHSV7BQI3U7RFRBUI2IQD2DEXP7A5CNFSM6AAAAABSR47S2WWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTUTIPWXW.gif> Message ID: ***@***.*** ***@***.***> >
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes to pixel size, focal length, or binning currently trigger clearing of calibration. This is unnecessary for minor changes in focal length or pixel size. Also, the current floating point equality check always fails for reported pixel sizes on SX cameras because of approximation.