-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Ensure that the result of |constructStichedFromIRResult| is a number (issue 6113) #6114
Conversation
tmpBuf[0] = rmin + (v - dmin) * (rmax - rmin) / (dmax - dmin); | ||
// Ensure that this is always a valid number, since rounding errors | ||
// may cause it to become NaN (fixes issue6113.pdf). | ||
tmpBuf[0] = (rmin + (v - dmin) * (rmax - rmin) / (dmax - dmin)) || 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shall return value in range from rmin..rmax, can we use rmin + (((v - dmin) * (rmax - rmin) / (dmax - dmin)) || 0);
here?
Also, it might be useful to check for 0 before div, e.g. dmax === dmin
and return 0, e.g. dmax === dmin ? rmin : (rmin + (v - dmin) * (rmax - rmin) / (dmax - dmin))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good point, thanks! I'll update the patch to clamp the value to the range of the function.
…(issue 6113) Fixes 6113.
The one time that I don't bother testing locally before pushing a patch, since I figured it's probably fine anyway, I obviously break something. :-( /botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/88c7b0f6339cb78/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/88c7b0f6339cb78/output.txt Total script time: 0.69 mins Published |
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/d1140eb72f296ab/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/5ea1692f55feb48/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/d1140eb72f296ab/output.txt Total script time: 18.13 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/5ea1692f55feb48/output.txt Total script time: 18.18 mins
|
/botio makeref |
From: Bot.io (Linux)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/ba10e5d80831865/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/d10cf3635af19d5/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/d10cf3635af19d5/output.txt Total script time: 17.93 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/ba10e5d80831865/output.txt Total script time: 18.39 mins
|
Ensure that the result of |constructStichedFromIRResult| is a number (issue 6113)
Nice, thanks! |
Fixes #6113.
It appears that when we switched to using
Float32Array
(in PR #5134), we may now occasionally encounter rounding errors, which cause the result to becomeNaN
thus leading to incorrect rendering.