sinisterchipmunk / jax

Framework for creating rich WebGL-enabled applications using JavaScript and Ruby
http://jaxgl.com
MIT License
96 stars 16 forks source link

Exposing intersectRay "bug" when passing vec4 as cp #86

Closed Goutte closed 11 years ago

Goutte commented 11 years ago

This took me a week to pinpoint, and made me loose clawfuls of hair along the way.

It does not "bug" with all triangles, just some of them. All of the buggy triangles I could find had one vertex on the z=0 plane, but I don't know if it's related. And even with them, it's not all rays that bug. This is why the values in the test are not simple ones.

I'm solving this by using [] instead of vec4.create() in my app for now.

Also, Jasmine runs fast, especially when I target specific specs, but I takes ruby a dozen seconds to compile the assets needed to run. Is this normal, or am I missing optimization tweaks ?

sinisterchipmunk commented 11 years ago

I am investigating the error. Thanks for exposing this.

takes ruby a dozen seconds to compile the assets needed to run

This is normal, but it should not be compiling the assets on every request -- only once, the first time they are requested. That is a bug in jasmine-rails that has already been fixed.

sinisterchipmunk commented 11 years ago

The previous implementation's inconsistency was due to floating point inaccuracy. Values were the slightest bit different in the Float32Arrays vs. standard Arrays. The values being different is not uncommon, but it is strange that the difference impacted the algorithm as much as it did. The issue came to a head with the calculation of local variable b, which ended up wildly different using Float32Array. I was not able to reproduce this inconsistency by typing the algorithm into the console, and was not able to find any problem within the source code itself.

Ultimately I replaced the algorithm completely. The new one satisfies all tests, and though I haven't bothered benchmarking (because correct > fast), I believe it should be faster since there are fewer conditions in the new implementation.

Goutte commented 11 years ago

THIS IS AWESOME

It works perfectly ! I had the weirdest intersection bugs even after using [] instead of vec4 with the old algorithm. (which was way too complicated for me to understand) After pulling your changes, everything's fine !

One of the best birthday presents ever ! Thanks again ;)

sinisterchipmunk commented 11 years ago

No problem and happy birthday :)

Goutte commented 11 years ago

In case you don't know about this (and for future dwellers), I found a really interesting concept : barycentric coordinate system I already used it for tiling hexagons in a plane ; I suspect you used something similar for pointInTri, right ?

sinisterchipmunk commented 11 years ago

Yes, I used barycentric coordinates in the new pointInTri implementation. (I have no idea what the previous implementation was supposed to be using... lol)