Skip to content

Correct algorithm to pass testbench - #90

Open
RechieKho wants to merge 11 commits into
Forceflow:devfrom
RechieKho:main
Open

Correct algorithm to pass testbench#90
RechieKho wants to merge 11 commits into
Forceflow:devfrom
RechieKho:main

Conversation

@RechieKho

Copy link
Copy Markdown

Fix #49

So I have investigated throughout the issue. This is the summary of what happened:

  1. The original error is due to the fact an array is passing into a template function with parameter of generic type, this causes C++ compiler to panic as it can't infer the type, due to array decay happens much latter than template parameter inferencing.
  2. After fixing it, the test bench failed, the result before encoding and after decoding doesn't match up. So I did divide and conquer to understand which part of the algorithm failed.
  3. After isolating the error, it is found out that 3D decoder of LUT and LUT ET is broken, and 3D encoder of LUT ET is broken.
  4. Further trial and error found out that it is the int i = (int)ceil((maxbit + 1) / 8.0f) in for loop of compute3D_ET_LUT_encode causing one off error. While for decoder, there are not non-pre shifted LUT, and using the pre-shifted LUT under typical LUT algorithm causes the error. I supposed for decoder case, it is not as simple as encoder case where y and z component is just x component LUT table shifted by 1 or 2 correspondingly, since for decoder, the shifting is done before passing into morton3D_GetThirdBits. Thus, a simple LUT generate is written and ran to generate the unshifted LUT for decoding LUTs.
  5. Moving on, the 2D is failing on the similar manner, I supposed 2D's algorithm is somewhat copying from 3D, just some changes in byte length, etc. So I apply the similar fix from 3D to 2D.
  6. And finally the test bench passed.

Hopefully what I did is correct. I supposed this patch is somewhat important since LUT implementation is defaulted to be the fallback, and a buggy algorithm as default is kind of hard to be ignored.

@RechieKho

Copy link
Copy Markdown
Author

And hopefully this patch could be integrated and released soon, because I kind of want to write a voxel engine, haha 😄

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