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

Last minute API additions for Gthree #171

Merged
merged 6 commits into from
Sep 8, 2019
Merged

Last minute API additions for Gthree #171

merged 6 commits into from
Sep 8, 2019

Conversation

ebassi
Copy link
Owner

@ebassi ebassi commented Sep 4, 2019

Gthree has some in tree functionality that really belongs to Graphene.

Proposed changes:

  • add graphene_triangle_init_from_float() initializer; this is useful for buffer objects with lots of triangles stored as vectors of floating point values
  • add intersection methods for graphene_ray_t; this is used for hit testing objects like spheres, boxes, and triangles
  • add graphene_triangle_get_uv()

Triangle-based meshes are ususally described as buffers of floating
point values when submitted to the GPU; if we want to easily go back
from buffers to triangles, having a simple initializer would help
keeping the code compact.
A GNU libc extension for checking if a single precision float is NaN.
Since we're now checking three separate GNU libc extensions to libm, we
might as well coalesce the whole thing into a loop.
@ebassi
Copy link
Owner Author

ebassi commented Sep 4, 2019

Would be nice if @alexlarsson could do a review so I can merge it in time for next week's GNOME 3.34.0 release.

@alexlarsson
Copy link
Contributor

So, this looks overall good to me, but there is something weird with the triangle intersection kind.
I ported gthree to use this new API here:
https://github.com/alexlarsson/gthree/tree/use-graphene-intersection

However, with this PR as-is, it gets the tri intersection kind wrong here: https://github.com/alexlarsson/gthree/blob/use-graphene-intersection/gthree/gthreemesh.c#L217 as can be seen in the interactive demo. If i switch SIDE_FRONT/BACK around it works, but otherwise it doesn't.

The fix for this is to switch the two kinds in intersect_triangle like so:


diff --git a/src/graphene-ray.c b/src/graphene-ray.c
index 56bc051..b0bf411 100644
--- a/src/graphene-ray.c
+++ b/src/graphene-ray.c
@@ -661,13 +661,13 @@ graphene_ray_intersect_triangle (const graphene_ray_t      *r,
     }
   else if (DdN > 0)
     {
-      kind = GRAPHENE_RAY_INTERSECTION_KIND_ENTER;
+      kind = GRAPHENE_RAY_INTERSECTION_KIND_LEAVE;
       sign = 1.f;
 
     }
   else
     {
-      kind = GRAPHENE_RAY_INTERSECTION_KIND_LEAVE;
+      kind = GRAPHENE_RAY_INTERSECTION_KIND_ENTER;
       sign = -1.f;
       DdN = -DdN;
     }

This actually matches what three.js does here:

https://github.com/alexlarsson/gthree/blob/use-graphene-intersection/gthree/gthreemesh.c#L217

Where if DdN > 0 and we're backface culling then we have no intersection, which totally smells like a LEAVE kind of intersection to me.

This was actually how I coded the gthree version of the intersector initially, but I had to switch it around to make the code work. Reverting this to what three.js does is nice, but I don't understand what the difference is. Why did I have to revert it to make the previous version in gthree code work???

@alexlarsson
Copy link
Contributor

Ah, i found the difference:

-  kind = ray_intersect_triangle (local_ray, vC, vB, vA, &t);
+  kind = graphene_ray_intersect_triangle (local_ray, triangle, &t);

I.e. the old gthree code passed the vertices in the wrong order, effectively flipping the triangle.

So, the patch about is right and should be applied.

As graphene_ray_t is used to ray cast for hit detection, we should have
intersection methods for objects like spheres, boxes, and triangles.
Use an `else if` instead, since it's what we're doing anyway.
Needs more cases, but it's a start.
Another method useful for Gthree.
@ebassi ebassi merged commit 52eaa2d into master Sep 8, 2019
@ebassi ebassi deleted the master-next branch September 8, 2019 16:27
@alexlarsson
Copy link
Contributor

So, this looks good. I thought there were some issue with the sign of the v coord initially, but that was just due to some weirdness in how the graphene box primitive sets up the uv coords.

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

Successfully merging this pull request may close these issues.

2 participants