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

New 1.123 Environment Map Rotates Together With Model #12310

Open
xtassin opened this issue Nov 15, 2024 · 9 comments
Open

New 1.123 Environment Map Rotates Together With Model #12310

xtassin opened this issue Nov 15, 2024 · 9 comments

Comments

@xtassin
Copy link
Contributor

xtassin commented Nov 15, 2024

What happened?

Testing the new 1.123 Environment Map implementation, I noticed that the env map is rotated along with the 3D model.

Reproduction steps

  1. Load PBR based GLTF2 model
  2. Rotate model
  3. Environment map (reflection) rotate together with the model (up side down at roll=3.14)
    ...

Sandcastle example

https://sandcastle.cesium.com/#c=tVkJbxu5Ff4rrFFg5UIazX04dtDYcY7WaQLb2wW2Lhx6hpK45pBTkmNHG+S/95FzaCQrsrJog2A0Q/J97358pHPBlUYPlDwSiU4QJ4/ojChal84/7djo5iC332eCa0w5kTcHY/T1hiOEOS2xpoIfoRlmiozNoKYlYbBsOFYrcirFoyLykuSiLAkvSHFJlGD1Ovm3wxc3/IbnVqavSOWEkzHKcUkkhl8m8nv0DaRsxLVr6QyN/mQXOqoiec2wPOcPVAoObPQHXKmruqqE1KQ4bMR+pLwQjw5mRGrQ7npBFbprxEOFIApxoZFqiFCHicgKFJWA6twcGGG/reRdEDpfaJDOdx33RT9abZj1HcEF5fNPVOeLS8HYCFaPkX0Ejhce9pRC0jnlQNwSnmFQQlHMA2cmRfmazCUhajTx/MBxkzCMvWyMwtBxIzdI3HjcCrQCLEVB2AesJf2yQr2WmKuZkKVyFhuSXYs39Asp3kiw/6iRZmz0aZzU2Jy1Og8UfE0lyY1fMbswsyNr9aIbPRquXek0MosQ2MEPo9BP0sB1o9D1Mm/cTaRp6McBTEZeFmSgbDcRZImfhannZVEYpakb2ZlD+6RcE66oXh5ZE/chNp0i6/gNt6JHrFAlBYS8IgUELtgD/X0BK4T6SaE5u36D3p9eoCtcVhA/DroW3XK0FLVE4pEfWXQPTdBr+GICF5ApBbqrKSuQXpAO7wkaGi20rtTRdDqnelHfOZAr03bxWynqamooJkAxaSkOHcvLB16XNUefc0bRhPKq1p+wXqBpBc+pFlPI0zlxFoVEE1Hrs/qOQGKspmEMSJx7/cX/7AyQFp8RRAbCjCFRGd9B1LexRNZy7OfLC3RizH1z0KnQFA2rQlXfMZpPr8AKOVaakUb611jj6T3FEt/Gt3imieRC8Fv//pbeMSvMzUHvK4HmEG4Sa2JNqKoFkTTHDC2wLAWnOcoFmc1oTkEiyGfCxOPYFB70Vog5I+C8N5Rh62dw2G8Qi1v8ZKD7ZZIwggFgm1fmFnQ6a9dO27XqiUNKEBtNJlxMSiolGHMy0cuKnIB68FqQionlyYYjtvjtc4Nr/n0EIaXxC9RehbAk4B8luoJFCmed84KwapsbAS0AKa/xfWPRzrzFuiFN5HamKSGb2sT92xWCRFELMFxj6z4wLlx3vRxspLjn+EGYZkmQRJ4bp54/bkd9D5LbcwPfjxI37UaTIEhCN3XjKMkik/Krcnbh3XrPsPKC2IMi4qbwP/K9FtRLvNDLsiSFnySNOlZpkmZhACIlXpz5G6yeVQqqludCVfIAJArG/WiSeW6SxF6QBl7PKXDjBIZ91439YIPTszq5ieeHgZ8Cpp/2Onl+nCVQM/0EZOiM6kUBaOrBdgBGCNJ1Tv6t/wwrN41DLwLM0AuysMV0kyxzQ3iGQeJHccfJ9T0/csM0DpIg22T0rE6Z64JKYMA08JMeM43dLPMzNw2TJOlDwgtjP8mg3KcRqLbBarefYAvwACmIQwiqwGsjAkZ9F+IkyOIwgEhJ+9Eo8BI/cQM3hTjc4OQ9a7wEQAEgAsP0HrG6ZH7sm03Oj3rvmVCMIw8eTfQMOe120wQEBQ/5cZKmWRRlfjLuhkOIOXBfFMcp7JurYQ/SL40gNuIw9NaYreX+CfoXZPPYJpp52lfzBpFjn/bdjPr2zf/3oHuzpesUamJhmwCzj65p8f7JfNMn9JX9XVvYzwYiHa0JOG7Wb236jp7uUJv9JWv5fmxqolHXAH5t2gpNvugj2M/ONxqEjgq64GYhkELxh+0EjQ7RycuOHm2xgLNTORBgqN2LnThblQaEJ1rvgnk69AbnWsgtPafvfPzHeY816P2cvsMCqqbxNSu+Wes0zw2LfjL9UlFL2LzV/fL/a9AaThozOI0Uf8yae5H/z63o72PFpsvurQdWY8s/ZMIfEv/X88uPz8rvbZHflIZVA+jgorgWgt1h+YHwerSRimPQrxtqlzWnrRvOSHuOWT/V7M7hM2jOJDoTWKNfaH5vj7DbDWVanQ8GcORHKexI2eEOD2BjuQK9I6wkeh9I6C1gm9wB+Qs0YBKdCg022gsQDjxZsgPwFdf0PzVBZ/YIvbeM/g7Ia7EEuL1NGOzS9xOctMXMeIapvQHdHYCnWHJ0AceL/dCgg4l3ebjO7/e22S43DDeRa6L2D5Z0B+iH5kBxCn39XnhJCG3CLjyiMZtcinq+4OY8e2VqK9nLL9C5Q0+1A/uqu0bZV/sI+q4k2xnZVAktRbVEV1oSPofz7t7g0A7tdPwA/DVV+Y8Ap1G6H/Cl0Pby7MfAd5lkUOb2x4QiF++HCUmPPmEo9HsDDyyxu/4Pa7gp/vZbrZX+p5Fs9wOsljxHs5rbu6WBBPD1Simi33fXflouO+WajQNiW9QyJ7Bp4EdMdd+ZCn7ZTtmLthZmiNhub1bMTXLL3hK+ZXr2ysg36vffWrKjnvG4Gx1czB0NP8bf37bbqbahXe3GlaQl1fSBKEeSUjyQV4yNOnmfLAE3NLZfwdhPoMXF8vwB6pVZY18uqNLmjmC00VL8LkR5LRqj91i9bPCDcqzzBRoRU64OO8K1W9jP57aUGe+ZXsbCHKE/f7Uk3z63QO1ta+/sp5wb8O4owyHRGLPX2u0FcS4J4VcVzkmzIZ71a160hO2nU1JOy7r8FVhABdCY2zhpbqLBubXS4GoOubFJeNtS2vQml+bCquuIzErTv3xFEvSslb3Kbux9J6DVBNWbmmsxzcV2u+4YedBZ96abTtHV9zWxd9j8J40WkG6MIFWai58mo9Ajgc1ikAMKWmcrIOCjaStX678unm0ohk5ZM00rtjxd/sypuX66MrSjtRh2tgWv5TF+bl0XNa3Kfzlp6DrPr/za3lKv2lOAWThaXMIw5moE53Ww1YsVQWXusweHk9aJTIj7V3o0jPp1LzgQM9COtfJ97wYf8zkZtTKNG17jXgnTyjc1sP1bwcH44FjpJSMvO3v8lZb2Dw1QGUaOM9WkrBgEjZreQfNBtJMr1ZnmeDokPS7oA6LFyZa/z6CcYaVgZlYzdkV/h17y5fEU1j8hbTPu4wORDC/NsoX38qIZdBzneAqf2yl1V5x7TQaom737GsZw5UahX+c1eP8v

Environment

Browser: Chrome
CesiumJS Version: 1.123
Operating System: Windows 10

@xtassin
Copy link
Contributor Author

xtassin commented Nov 20, 2024

Just updated the sandcastle example to make the issue more obvious...

@ggetz
Copy link
Contributor

ggetz commented Nov 20, 2024

Thanks for the report @xtassin!

@jjhembd Could you please triage this? Is this expected behavior or no?

@xtassin
Copy link
Contributor Author

xtassin commented Nov 21, 2024

Just to add some info, the environnement map (reflections) orientation was fine before the overall in version 1.123 so it appears to be a regression bug.

@jjhembd
Copy link
Contributor

jjhembd commented Nov 21, 2024

@xtassin thanks for the Sandcastle! I agree that this behavior is not what we want. However, I re-labeled the issue as an enhancement rather than a bug, since the desired behavior is something we never implemented.

The environment map is referenced in the shader via model_iblReferenceFrameMatrix, which in most cases is simply the camera's viewMatrix multiplied by the model's modelMatrix. The only exceptions relate to tilesets, clipping planes, and height clamping, which I don't think are relevant here.

This means that the environment map will be oriented to the model, and when you rotate the model, the environment will rotate along with it. This works well for simplistic cases where the model is fixed along the local ENU axes. But it doesn't work for the (relatively common!) case where your model is moving.

I don't understand why the orientation would have been different before 1.123. That version introduced a new procedural environment map, but as far as I can see, it did not change the transforms for supplied environment maps like the one in your Sandcastle. Are you saying the procedural environment orientation changed?

@xtassin
Copy link
Contributor Author

xtassin commented Nov 21, 2024

Here are three screenshots of the same GLB model (attached), exported with PBR material, from 3DS Max, using Babylon exporter.

Cesium version is mentioned on each one of them. Environment map (reflection) use to be static compared to earth frame.

Since version 1.123, the env map is rotated along with the model (in model frame).

image
test.zip

@ggetz ggetz self-assigned this Nov 21, 2024
@xtassin
Copy link
Contributor Author

xtassin commented Nov 26, 2024

Sorry to be so insisting but is this still considered an "enhancement"? I hate to see the ground reflected by the bottom of my planes when they fly inverted ;)

image

@ggetz
Copy link
Contributor

ggetz commented Nov 26, 2024

@xtassin I believe this was labeled an enhancement because technically this behavior worked previously by accident. I don't think it was intended to indicate a low priority or "won't fix".

I do hear that you, and potential other users, were depending on the unintended behavior, so because of this it should be considered a regression.

In the meantime, you should be able to workaround this issue by adjusting mode.referenceMatrix, the additional internal transformation used for image-based lighting, to the inverse of the rotation in order to counteract the rotation applied to the model.

@javagl
Copy link
Contributor

javagl commented Dec 11, 2024

I tried to find where this was introduced, but had to skip some intermediate states where it didn't render the test object properly, ... so... the following is likely not helpful, but I'll post it nevertheless:

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
7532157e04095970b32ddf67f6c3fcd3e5beb58e
3e7e4ec74c9e3d76352216d6b89b4a8dbf87016b
a59ef548c2dfcb71213ed8dde097ecd2b72f3c06
d0a72cd6dfa9c5c520b13bcbcf13282630809e80
e303b72a787dfea2c39b02c98113265c08c672fe
c4c734cec5f7e0494b2e9dc679c5ed14d8960d62
f0f1cd41278b3f2f50ecf88eff8d51b6ff66968f
1e1ef3cd00ed3c7c8ef3232f4ea744a9f84209c7
ba9410100812abf28b3058adb3015506b158310e
af28f30a4d516a70542a32430b62112a79874c6f
e9ac951a5efd887e0129d42b5efa7e4657762188
99305873fc5921fca18262bffd14ea734118f5cf
d835dd11ea9debbaf5111e02725c86352d5fb3c8
7458f0a2a0d54b1b4b3e827d8c322ad9a9f12913
c0f6f8977b9fb7a45d4e4d9ea8aa83c1e3ef282f
1c587ce519915db4c8058e15cc2b5bb0b1587242
84bbd77ba6804cb7286c3b2c0d8e707d49badce4
53d45a0fdcfcca43c0d13275dd9dfef5cd9d26a8
We cannot bisect more!

@javagl
Copy link
Contributor

javagl commented Dec 11, 2024

I didn't go into all depth of the code, and didn't fully understand the referenceMatrix. From a bit of trial-and-error, it looks like changing the referenceMatrix here to context.uniformState.enuToModel
seems to help for the test model

Cesium Environment Map Object

and (referring to the linked issue), the OSM buildings don't look entirely wrong...

Cesium Environment Map

But the proper way of combining that referenceMatrix with the ENU matrix probably has to be figured out by someone who knows more context of the computations that are done there....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants