nytimes / aframe-loader-3dtiles-component

A-Frame component using 3D-Tiles
Other
190 stars 26 forks source link

[high priority] Darker 3dtiles shading in A-Frame 1.3 compared to A-Frame 1.2 #14

Closed kfarr closed 2 years ago

kfarr commented 2 years ago

Hi there, after updating a scene to the latest version of A-Frame 1.3 (which includes THREE Version ^0.137.0 from https://github.com/supermedium/three.js) a 3dtiles scene using this loader appears darker than when using A-Frame 1.2 and darker than reference application used to produce 3dtiles output.

Here is the expected output working correctly using a-frame 1.2: https://3dtiles-aframe-issue-1.glitch.me/aframe120.html image

Here is the unexpected output not working correctly using a-frame 1.3: https://3dtiles-aframe-issue-1.glitch.me/aframe130.html image

Here is a reference from Cesium's interface which matches the expected shading when using 3dtiles loader with A-Frame 1.2: Link to cesium sandbox image

Workarounds:

Let me know if there's anything else I can provide to debug. Thanks for the help!

Avnerus commented 2 years ago

Hi there! I am currently in low-availability mode so response times may vary. Three.js r137 changed some sRGB shader behavior which required me to make one fix in three-loader-3dtiles. It also requires that when using sRGB encoded textures you set:

 renderer.outputEncoding = sRGBEncoding;

The aframe13 branch uses the upgraded loader and the corresponding changes. However, I have noticed one regression in which textures look more jittery in VR using the newer aframe/three, which is why the changes are not yet in main. I haven't had a chance yet to figure out the cause for this. Would appreciate if you could also check how it looks to you and if you have any idea about the underlying issue. Doesn't seem to be related to mipmap or texture filtering settings. You could try the branch versions from jsdelivr, let me know how it goes.

kfarr commented 2 years ago

Great, sorry I didn't check the branches, yes we can test this week and report back. I will also test in VR mode to provide feedback on texture jitters on Quest 2.

kfarr commented 2 years ago

Confirmed shading appears as expected with the aframe13 branch on Chrome desktop (macOS) browser! As a bonus iOS / Safari looks nice too.

To accomplish sRGB encoded textures, add renderer="colorManagement: true" to the a-scene tag like this: <a-scene renderer="colorManagement: true">

glitch URL example that uses the aframe13 branch: https://3dtiles-aframe-issue-1.glitch.me/aframe130-3dtiles130.html

screenshot: image

Next step: Test in VR mode on quest 2

kfarr commented 2 years ago

Hi @Avnerus we tested using meta quest2 browser with the aframe13 branch compared to the current main branch.

We do see 2 examples of flickering -- textures in "medium distance" seem to sparkle. Textures in "long distance" sometimes have z-fighting appearance of flickering.

However, we also see the same issues in the main as well as aframe13 branch, so the new release doesn't seem to be contributing to this issue.

I would suggest to merge aframe13 branch as it is at least handling color better than the current, and even if no improvement to VR situation it is not worse than current.

Avnerus commented 2 years ago

Hi @kfarr, thank you for checking this out! I will have to run some more tests before merging the branch, because when I tested this last I was seeing a clear difference switching back and forth between AFrame 1.2 and 1.3. I'll get back to you on this during this week. /Avner

Avnerus commented 2 years ago

Update: I ran some tests and now it doesn't seem like there is a noticeable difference on my headset between the versions, but I am waiting to get some more feedback. The aframe13 branch now uses the newest version of three-loder-3dtiles. As a heads-up, there is one breaking change regarding the orientation of tilesets. It was incorrectly calculated in previous versions which caused some models to be tilted. This is fixed in the new version.

kfarr commented 2 years ago

Ok good to hear and thanks for the heads up on the breaking change, I just happened to notice that last night on a test project using this branch so I'm happy to hear it was intended. FWIW the new orientation seems more "correct" -- the model required less height and rotation adjustment in A-Frame scene to match expected orientation relative to model viewed in Cesium.

Avnerus commented 2 years ago

Hi, This is now published to NPM. Closing issue, but feel free to re-open if any problems occur.