google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.76k stars 801 forks source link

Add anisotropy tests to render-fidelity-tests (and update Babylon + Filament to latest) #4535

Closed bhouston closed 8 months ago

bhouston commented 9 months ago

This builds upon my previous PR that upgrades from glTF-Sample-Models to glTF-Sample-Assets: https://github.com/google/model-viewer/pull/4534

This PR pulls in additional tests from the new glTF-Sample-Asset repository, specifically 3 new anisotropy tests.

I have also upgraded Babylon.JS and Filament to their latest versions and updated their goldens as well as model-viewer's goldens.

bhouston commented 9 months ago

Fixes #4486

bhouston commented 9 months ago

I can not reproduce this Github unit test failure:

🎨 Scenario: khronos-AnisotropyBarnLamp
Start to compare model-viewer's golden with other renderers' goldens:

🔍 Comparing <model-viewer> to Filament

  📊 Decibels of root mean square color distance: -5.15

🔍 Comparing <model-viewer> to Babylon

  📊 Decibels of root mean square color distance: -5.35

🔍 Comparing <model-viewer> to gltf-sample-viewer

  📊 Decibels of root mean square color distance: -5.36

🔍 Comparing <model-viewer> to three-gpu-pathtracer

  📊 Decibels of root mean square color distance: -3.46

💾 Recording analysis
start compare model-viewer's golden with model-viewer's screenshot generated from fidelity test:
🚀 Launching browser
🗺  Navigating to http://localhost:9030/packages/render-fidelity-tools/test/renderers/model-viewer/?hide-ui&config=../../config.json&scenario=khronos-AnisotropyBarnLamp
🖌  Rendering khronos-AnisotropyBarnLamp with <model-viewer>
🖼  Capturing screenshot

  📊 Decibels of root mean square color distance: -19.90

When I run this exact same code locally I get this result:

🎨 Scenario: khronos-AnisotropyBarnLamp
Start to compare model-viewer's golden with other renderers' goldens:

🔍 Comparing <model-viewer> to Filament

  📊 Decibels of root mean square color distance: -5.15

🔍 Comparing <model-viewer> to Babylon

  📊 Decibels of root mean square color distance: -5.35

🔍 Comparing <model-viewer> to gltf-sample-viewer

  📊 Decibels of root mean square color distance: -5.36

🔍 Comparing <model-viewer> to three-gpu-pathtracer

  📊 Decibels of root mean square color distance: -3.46

💾 Recording analysis
start compare model-viewer's golden with model-viewer's screenshot generated from fidelity test:
🚀 Launching browser
🗺  Navigating to http://localhost:9030/packages/render-fidelity-tools/test/renderers/model-viewer/?hide-ui&config=../../config.json&scenario=khronos-AnisotropyBarnLamp
(node:83665) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
(Use `node --trace-deprecation ...` to show where the warning was created)
🖌  Rendering khronos-AnisotropyBarnLamp with <model-viewer>
🖼  Capturing screenshot

  📊 Decibels of root mean square color distance: -Infinity
💾 Recording configuration
✅ Results recorded to /Users/bhouston/Coding/Work/model-viewer/packages/render-fidelity-tools/test/results
bhouston commented 9 months ago

@elalish I removed the anisotropyBarnLamp test so that we pass the fidelity test suite. So I believe this is ready to merge.

bhouston commented 9 months ago

@elalish wrote:

I take it this takes the place of https://github.com/google/model-viewer/pull/4534? Not a big deal, but it's a bit easier to review these things if you just push changes to that branch/PR instead of creating a new one.

Technically each of these PR's build upon the previous. So if you wanted to merge https://github.com/google/model-viewer/pull/4534 first as it is good.

Then we can modify this one, and then once it is in, we can then discuss the next one that adds more models: https://github.com/google/model-viewer/pull/4537

bhouston commented 9 months ago

I've merged in this other PR into this one: https://github.com/google/model-viewer/pull/4537. They both only add tests, so we can reduce the review overhead by having less PRs.

bhouston commented 9 months ago

I've updated the branch and added back the BarnLamp example but I excluded it from "model-viewer" as it doesn't render properly in the Github Action (although it works on my machine.) I then modified the CLI tools so that "npm run test" excludes fidelity tests of the current renderer so that it doesn't check if BarnLamp is good for the default "model-viewer".

So I think if all tests pass, this is now good to merge.

gkjohnson commented 8 months ago

This looks great, thanks! @gkjohnson what's the status with three-gpu-pathtracer these days? Are you planning to upkeep it?

I've tried to but browser vendors (mostly Apple and Safari) keep adding api regressions that cause it to break after fixes have been added 😓 The recent blocker is that Apple's recent OS update introduced issues that cause the path tracer to crash hard so I can barely test it at the moment since I don't have easy access to my other machine. The bug is being tracked here.

I'm going to wait this out a bit to see what happens with this fix before diving into some of the new features too much more. Some others are helping to improve the renering on iOS so the project is still moving - but at the moment it's finding a few browser bugs 😅

elalish commented 8 months ago

@gkjohnson Gotcha, thanks. I'm having trouble even on Chrome, I think because of updating three.js? Do you want to take a look at why we seem to just get blank renders now?

bhouston commented 8 months ago

@elalish could we just commit this as is? Bugs in three-gpu-pathtracer is a bit orthogonal to his PR. And what is great is that @gkjohnson can use the new tests in this PR to further improve his renderer.

gkjohnson commented 8 months ago

@gkjohnson Gotcha, thanks. I'm having trouble even on Chrome, I think because of updating three.js? Do you want to take a look at why we seem to just get blank renders now?

Without knowing your platform it's hard to say. On Mac these issues are operating system-level driver issues that affect all browsers recently introduced with MacOS 14.0. But like I said I can't run things on my machine on Safari or Chrome due to these system regressions and no reported errors. I'm hoping there's some progress from Apple sooner rather than later.