search

loicgasser / quantized-mesh-tile

Quantized-Mesh encoder/decoder and topology builder
MIT License
89 stars 20 forks source link

remove padding bytes to correct lighting #22

Closed thiloSchlemmer closed 6 years ago

thiloSchlemmer commented 6 years ago

hi,

removing of extra-padding bytes and fixing the loop produce correct normal values.

Tested with bavarian dem-data and elevation-styled material correct_lighting

loicgasser commented 6 years ago

hi thanks! Unit vectors should already be in the correct order. So I removed some code here: https://github.com/loicgasser/quantized-mesh-tile/pull/23/commits/5c45f6e787ca28148ae277a7e0423f5002ddf6c0 This should be even a little better (and faster)

can you tell me if it works for you? This was one of the idea I had of why the unit vectors were failing. (direction) Do you mind if I add you to the section of the contributors?

thiloSchlemmer commented 6 years ago

Hi,

sorry for the unbalanced and untestet PR (i forgot the "read"-side).

Yes it works for me and i'm pleased to be contributor ;)

Surprisingly (only for me, i guess) i see now the problem with the normals on tile-edge vertices. earlier processings don't show these effects in my data.

if you still working on this topic (tile-edge vertices), i can share my ideas and solutions in then next weeks.

loicgasser commented 6 years ago

At this point I must really thank you. No need to be sorry. You actually solved a bug that was here for more than a year. When you see earlier processing didn t show that effect. You mean with the current PR, the first solution with the moved padding or with the PR I just opened? I am gona have to solve the padding mystery. I think it s possible I am missing some pasding before the index data, which creates a valid terrain tile but not entirely optmized. I ve tried with tiles from my sample data and padding is not always added. It also seems to be the case in the Cesium implementation. I ll dig a little deeper in Cesium code to see if any padding is added in the light extension to make sure they didn t omit it in the doc, in the piece of code you mentionned light extension is returned as a buffer. There might be some later processing involved.

thiloSchlemmer commented 6 years ago

No problem ...i just want to solve my challenges with our terrain... i mean the first PR, which only take care of method '_writeTo' and leaved 'fromBytesIO' as it is and tests are failing... my first PR should have be better, nothing else.