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

Floating point issue #124

Open
nine-fox opened this issue Jul 19, 2022 · 1 comment
Open

Floating point issue #124

nine-fox opened this issue Jul 19, 2022 · 1 comment

Comments

@nine-fox
Copy link

Hi Alex,

This is a really great project! As far as I can see, the implementation is cool and clear, and I am willing to use it in CAD projects.

However, I am struggling with the following issues, would you like to take a look? thanks,

  1. usage of Flatten.Utils.EQ
    When I dig into the polygon intersect code, I found EQ(Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY) return false, which is confusing, because in Math, these 2 things are equal.

The implementation in EQ explains, because POSITIVE_INFINITY - POSITIVE_INFINITY will be NaN and return false.

To make better understanding, should EQ return true in this case?

  1. floating point issue
    Flatten.js is using float for each storage, like Point.x, Circle.r etc. It can be enhanced to use more accurate numbers in some CAD system. For example, to use decimal.js to accept more large numbers. I know the cost would be high, however, would it be better to provide big decimal option to user?

Thanks,
nine-fox

@mahaidong
Copy link

https://github.com/alexbol99/flatten-js/blob/master/src/classes/box.js

contains(shape) {
        if (shape instanceof Flatten.Point) {
            return (shape.x >= this.xmin) && (shape.x <= this.xmax) && (shape.y >= this.ymin) && (shape.y <= this.ymax);
        }

I guess it should change to !Flatten.Utils.LT(shape.x, this.xmin) && ... , it will be more correct for floating point

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