PacktPublishing / Mastering-Graphics-Programming-with-Vulkan

MIT License
562 stars 77 forks source link

Fix point light assignment #60

Open FlexW opened 3 months ago

FlexW commented 3 months ago

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.

if (sortedLight->projected_z_min < 0.0f && sortedLight->projected_z_max < 0.0f)
{
    // The light is behind the camera, skip it
    continue;
}

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.

theWatchmen commented 2 months ago

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.

theWatchmen commented 2 weeks ago

I had another look at this and I agree with the changes to compute the tiling address. Each tile in x also take k_num_words so we should multiply by that value. Thanks for spotting this!

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 :)