Conversation
The min and max light id calculation needs to include lights that have their projected z coordinates smaller and greater than then bin z bounds. The address calculation of tiles need the x value multiplied by the words count. Otherwise the address is not correct because one tile covers words count unsigned integers.
|
Thanks for the PR and apologies for the late reply, I was away on holiday :) I'll review the changes in the next few days. |
| const SortedLight& light = sorted_lights[ i ]; | ||
|
|
||
| if ( ( light.projected_z >= bin_min && light.projected_z <= bin_max ) || | ||
| if ( ( light.projected_z_min <= bin_min && light.projected_z_max >= bin_max ) || |
There was a problem hiding this comment.
Not sure this is correct, as this would include lights that have their min/max depth bounds outside the bin bounds. It could well be that this line is not needed though and that we just need to check the min/max z against the bin bounds. I haven't run the code yet, I'll verify more thoroughly later.
|
I had another look at this and I agree with the changes to compute the tiling address. Each tile in x also take As I mentioned in the other comment, not sure the bound check per light needs to change. Happy to be proven wrong though as I might be missing something :) |
Thank you for writing the book. I enjoyed it a lot. I think I spotted two mistakes. There might be other places in the codebase that have the same issues:
The min and max light id calculation needs to include lights that have their projected z coordinates smaller and greater than then bin z bounds.
I think it may be also needed to include this condition before checking the point lights projected z min/max to skip lights that are behind the camera.
The address calculation of tiles need the x value multiplied by the words count. Otherwise the address is not correct because one tile covers words count unsigned integers.