Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Geometries get deformed because translation is applied too early #9

Open
wlinna opened this issue May 5, 2024 · 4 comments
Open

Geometries get deformed because translation is applied too early #9

wlinna opened this issue May 5, 2024 · 4 comments

Comments

@wlinna
Copy link

wlinna commented May 5, 2024

cityjson-loader-broken-translation-handling

This is practically the same issue that cjio has. Geometries are deformed because the whole transform is applied very early in the process. This leads to floating point inaccuracies.

To reproduce the problem, load files in CityJSON Ninja (which uses cityjson-threejs-loader), and notice the difference. The test files are below

simplified2.city.json
simplified2_less_translation.city.json

My suggested solution is to only apply the scale from the transform at first, do all the processing normally, and then add the translation afterwards to the position of loader.scene (or if there's a better root node, then that).

@StylianosVitalis-TomTom
Copy link

Hey @wlinna,

Thanks for reporting the issue 🙌

I think this is already fixed by this: d6a9837. It's on the develop branch. I need to merge it to main soon.

I still haven't released a version of ninja that contains this fix as I had some issue with building it and I need to find some time to resolve it. If you are using this library directly, you may think about switching to the develop branch. If your concern is about how this propagates to ninja, I'll do my best to release a new version of it soon so that the fix is included there.

@wlinna
Copy link
Author

wlinna commented May 5, 2024

Hello, thanks for the quick response.
I don't use ninja directly, I'm mainly concerned how my CityJSON looks/works elsewhere.

I was looking at that commit, and something that I found interesting is that it doesn't seem to care about the difference of Y and Z conventions between CityJSON and ThreeJS. I would have thought that the scales would be applied in the order of s[0], s[2], s[1], and then similarly for translations, but with the minus sign applied i.e t[0], t[2], -t[1]. Or is the final transformation to ThreeJS's Y-up world handled elsewhere?

@StylianosVitalis-TomTom
Copy link

StylianosVitalis-TomTom commented May 6, 2024

In this library, I do not transform the coordinates to Y-up. You can either use a transformation matrix to finally transform everything to Y-up, or you can set the camera to assume a Z-up coordinate system (see here).

@wlinna
Copy link
Author

wlinna commented May 7, 2024

Okay, thanks for the clarification

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

No branches or pull requests

2 participants