BudgiePanic / PushingP

A Whitted ray tracing implementation written in Java
Apache License 2.0
1 stars 0 forks source link

fix lighting bugs in dice cube #76

Closed BudgiePanic closed 8 months ago

BudgiePanic commented 8 months ago

lighting artefacts on a rotated cube

investigating:

Originally posted by @BudgiePanic in https://github.com/BudgiePanic/PushingP/issues/73#issuecomment-1974166987

BudgiePanic commented 8 months ago

debugging pixel c: 785 r: 762

BudgiePanic commented 8 months ago
BudgiePanic commented 8 months ago

moving the light source to the camera location while keeping the cube rotated -45 degrees on y axis produced this image

erroneous shadows on dice dimples

suffice to say these shadows should not be present, why are these pixels being flagged as in shadow?

BudgiePanic commented 8 months ago

I think this would be easier to debug with a normal cube instead of a compound shape as the shape a in the dice compound shape

BudgiePanic commented 8 months ago

I think what is happening is:

but this needs to be confirmed

BudgiePanic commented 8 months ago
BudgiePanic commented 8 months ago

I think we got it wrong maybe we are supposed to perform all intersection tests first and then do the filtering of intersections at the end

the only difference is we need to curry the predicate down to the composite shapes like we are doing now?

lets make the unit tests and try this approach

BudgiePanic commented 8 months ago

for reference (to place a cube in the middle of the dice dimples)

new Cube(Transforms.identity().translate(1f, 1f, 1f).scale(0.50f).assemble(), diceMaterial), 
BudgiePanic commented 8 months ago
BudgiePanic commented 8 months ago

new unit tests to capture the 3 lighting conditions were created in commit https://github.com/BudgiePanic/PushingP/pull/73/commits/4fe548091468feb7db391db4053889add86e15e7

all of these tests pass when the dimple material does not cast shadows

BudgiePanic commented 8 months ago

the dimples are supposed to cast shadows

5 test conditions to verify in compound shape demo

BudgiePanic commented 8 months ago

it would be nice if concrete shapes had a name attribute to help identify which shape is which in the compound union cube

BudgiePanic commented 8 months ago

found Heisenbug with TimingWrapper camera class

in the unit test I wrote, a normal camera was used. The output was different to the CompoundShapeDemo despite being the exact same scene

changing the unit test to use a TimingWrapper camera caused the output to become the same, but even more confusing, the output remained the same even after changing back to a normal camera

BudgiePanic commented 8 months ago

in a simplified test case, it looks like the bug is in the filtering of compound intersections method,

specifically in the case where left shape only produces one intersection (such as a plane, or high angle intersect on a cylinder with no lids)

BudgiePanic commented 8 months ago

when this is the case, the shadow ray intersects with the cylinder behind it, and has 2 intersections with the difference sphere in front of it. These sphere intersections are not being filtered, when logically they ought to

BudgiePanic commented 8 months ago

minimum test to show shadow errors

BudgiePanic commented 8 months ago

optimization for the future:

BudgiePanic commented 8 months ago

I believe the root cause of the issue is that for shapes that do not enclose a volume, it is possible to cast a ray that will intersect with the shape only 1 time. examples include

when this occurs the iterative algorithm in the CompoundShape::filter function will be in the following state:

we need to be more selective about when we set the inLeft flag because the ray might actually be leaving the object when it's first intersection is encountered

BudgiePanic commented 8 months ago

The proposed fix in the above comment is not working because under its logic, a ray entering a shape and a ray leaving a shape are the same, which isn't true

Instead we will try use the trick from Intersection::computeShadingInfo by looking at the dot product between the shape's normal vector at the intersection point and the negation of the ray direction

if this value is less than zero, the ray has come from inside the shape, and this we can set the 'inLeft' flag

BudgiePanic commented 8 months ago

not sure if the above approach will work because the method is not designed to be provided with the ray

of course I can make a new method that is though, I'll give it s try

edit: doesn't work either

open cylinder compound shape intersection diagram

we need a set up that will pass this scenario, with and open cylinder

BudgiePanic commented 8 months ago

As a temporary work around to this problem, we need only avoid the use of shapes that do not enclose a volume in compound shapes

BudgiePanic commented 8 months ago

now that I think about it, the real problem here is that the shapes that don't enclose a volume aren't solid shapes, which defeats the purpose of constructive solid geometry

to wrap this issue up, we'll

  1. make an issue for adding constructive non-solid geometry (I believe it will need a whole new algorithm)
  2. log warnings if a non solid shape is added to a CSG shape
  3. modify the shape API to allow testing for whether a shape is solid or not
  4. add unit tests for the shape to ensure they comply with this new requirement