Cycles: Deduplicate sphere intersection test #112031

Alaska · 2023-09-06T15:27:08+02:00

Alaska commented

2023-09-06 15:27:08 +02:00

Cleanup sphere intersection test by reusing the same code for point cloud intersection and sphere light intersection.

Alaska added the

Module

Render & Cycles

label 2023-09-06 15:27:08 +02:00

Alaska commented

2023-09-06 15:46:45 +02:00

Edit: This is irrelevant now.

As a side effect of this change (specifically using the better intersection from the point intersection test), The issues with sphere light rendering in #112005 is fixed.

Edit: This is irrelevant now. As a side effect of this change (specifically using the better intersection from the point intersection test), The issues with sphere light rendering in #112005 is fixed.

Alaska requested review from Weizhen Huang 2023-09-06 15:46:57 +02:00

Alaska requested review from Brecht Van Lommel 2023-09-06 15:47:08 +02:00

Alaska requested review from Sergey Sharybin 2023-09-06 15:47:17 +02:00

Alaska changed title from ~~WIP: Cycles: Cleanup sphere intersection test~~ to Cycles: Cleanup sphere intersection test

2023-09-06 15:48:17 +02:00

Weizhen Huang commented

2023-09-06 15:50:11 +02:00

Hi, I was about to push a fix to #112005

Alaska commented

2023-09-06 15:52:37 +02:00

Hi, I was about to push a fix to #112005

Feel free to use which ever method you want. This one or your own.

I probably should of noted I was working on this issue on the original report. Sorry about that.

> Hi, I was about to push a fix to #112005 Feel free to use which ever method you want. This one or your own. I probably should of noted I was working on this issue on the original report. Sorry about that.

Alaska changed title from ~~Cycles: Cleanup sphere intersection test~~ to WIP: Cycles: Cleanup sphere intersection test

2023-09-06 15:55:58 +02:00

Alaska changed title from ~~WIP: Cycles: Cleanup sphere intersection test~~ to Cycles: Cleanup sphere intersection test

2023-09-06 16:10:52 +02:00

Weizhen Huang requested changes 2023-09-06 16:49:30 +02:00

Weizhen Huang left a comment

Hi, I committed the fix in fedeaab30f, thanks for investigating.
It appears that point_intersect_test() is using the same approach to compute l2 by subtracting first and then take the dot product. However, it adds quite some complexity that we don't need here. For example, the copysignf() in ray_sphere_intersect() means we always use t_front when we are outside of the sphere and the so-called t_back when we are inside the sphere, actually just the first intersection in both cases.
However, I'm not closing since this cleanup still makes sense to me, probably both can use ray_sphere_intersect() now, but not sure with the logic inside the sphere for the other case.

Hi, I committed the fix in fedeaab30f, thanks for investigating. It appears that `point_intersect_test()` is using the same approach to compute `l2` by subtracting first and then take the dot product. However, it adds quite some complexity that we don't need here. For example, the `copysignf()` in `ray_sphere_intersect()` means we always use `t_front` when we are outside of the sphere and the so-called `t_back` when we are inside the sphere, actually just the first intersection in both cases. However, I'm not closing since this cleanup still makes sense to me, probably both can use `ray_sphere_intersect()` now, but not sure with the logic inside the sphere for the other case.

Alaska force-pushed cleanup-sphere-intersect from 95133da96d to 319bf618f5

2023-09-07 02:27:11 +02:00

Compare

Alaska commented

2023-09-07 02:29:55 +02:00

@weizhen I looked into transitioning the point intersection over to using ray_sphere_intersect(). But I encountered some issues, specifically issues with non-uniform points (E.G. A point with a scale of 1.0, 1.0, 0.5).

Since that resulted in the rendering of non-uniform points no longer matching Embree, I decided to just use a modified version of the intersection test for points instead. Quick tests reveal no noticeable issues with this approach.

@weizhen I looked into transitioning the point intersection over to using `ray_sphere_intersect()`. But I encountered some issues, specifically issues with non-uniform points (E.G. A point with a scale of 1.0, 1.0, 0.5). Since that resulted in the rendering of non-uniform points no longer matching Embree, I decided to just use a modified version of the intersection test for points instead. Quick tests reveal no noticeable issues with this approach.

Alaska requested review from Weizhen Huang 2023-09-07 02:30:02 +02:00

Alaska changed title from ~~Cycles: Cleanup sphere intersection test~~ to Cycles: Deduplicate sphere intersection test

2023-09-07 03:25:39 +02:00

Alaska added 1 commit 2023-09-07 06:17:43 +02:00

05354d72c7 Simplify math

Alaska added 1 commit 2023-09-07 13:01:17 +02:00

97abfaee5a Simplify/clarify some math

Alaska added 1 commit 2023-09-07 13:21:19 +02:00

a968b66dfe Add code comments and simplify

Alaska added 1 commit 2023-09-07 14:49:16 +02:00

16a1d6857d Clean-up code comment

Alaska added 1 commit 2023-09-07 15:13:56 +02:00