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

updated gltf-sample-viewer goldens #2018

Closed
wants to merge 2 commits into from
Closed

Conversation

elalish
Copy link
Collaborator

@elalish elalish commented Feb 16, 2021

Getting rid of the scrollbars.

@UX3D-haertl Looking at https://modelviewer.dev/fidelity/, it appears you still have a problem with tone-mapping or exposure. It's especially obvious on Cube, Duck, and the simple grey tests toward the bottom. Also the furnace test is wrong, as it doesn't have the proper grey background and the spheres look over-exposed. Would you mind fixing these issues before I merge this PR?

@elalish elalish self-assigned this Feb 16, 2021
@elalish elalish mentioned this pull request Feb 16, 2021
@UX3D-nopper
Copy link

@elalish We have implemented two version of ACES tonemapping. Actually, we need to discuss in 3D Formats / 3D Commerce, which one should be used and/or is preferred.
I try to raise this topic in this evening call.
In any case, we will revisit, if we have a bug somewhere.

@UX3D-haertl
Copy link
Contributor

I updated the gltf-sample-viewer on our branch. Now the issues with the screenshots should be fixed. VC is still brighter than in the other renderers, but the other screenshots look good.

https://github.com/ux3d/model-viewer/tree/addGltfSampleViewer

@elalish
Copy link
Collaborator Author

elalish commented Mar 12, 2021

Great, can you make a PR? I'll update this one after we merge yours.

VC is weird; Filament had a bunch of trouble with it too. Something to do with how ambient occlusion is handled iirc.

@elalish
Copy link
Collaborator Author

elalish commented Mar 15, 2021

Closing in favor of #2128.

@elalish elalish closed this Mar 15, 2021
@elalish elalish deleted the updateSampleGoldens branch March 15, 2021 19:31
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

Successfully merging this pull request may close these issues.

3 participants