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

Gray Edge Threshold set to 100% should make all edges gray #832

Closed
kdahlquist opened this issue Jan 27, 2020 · 3 comments
Closed

Gray Edge Threshold set to 100% should make all edges gray #832

kdahlquist opened this issue Jan 27, 2020 · 3 comments

Comments

@kdahlquist
Copy link
Collaborator

When I slide the Gray Edge Threshold all the way to the right and it says 100% using demo file 2, all the edges turn gray except for one that is still red. I think they should all be gray.

igreen1 added a commit that referenced this issue Sep 15, 2020
1. Changed line 484 to use 'normalize' function instead of performing operation inline. This format follows the convention elsewhere in code
2. Changed all lines comparing normalize(d) to grayThreshold to round the result of normalize.
	Normalize was returning a value of greater precision that grayThreshold, which would make it 'larger than' simply from the extra precision.
@igreen1 igreen1 mentioned this issue Sep 15, 2020
@igreen1
Copy link
Collaborator

igreen1 commented Sep 15, 2020

Fixed in pull request from igreen1 branch #876 Awaiting review

(link since I'm not entirely sure the number is correct!)
#876 (comment)

@igreen1
Copy link
Collaborator

igreen1 commented Sep 18, 2020

Issue resolved in beta 4.0.19, pr #876 by merging igreen1 into beta branch :)

Bug was caused by a mismatched precision error when comparing grayThreshold and normalized values for edge thickness. The normalized values were more precise so when they should have been equal, the extra decimal digits caused normalized values to be computed as larger than grayThreshold.
Solution was to add .toPrecision(4) to the normalize function to match the precision of maxWeights defined elsewhere.

@kdahlquist
Copy link
Collaborator Author

Confirmed fixed in Beta 4.0.19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants