Post processing #3948

Beilinson · 2022-11-18T17:49:28Z

Deals with #213, #210

I started working with the inbuilt three.js post-processing pipeline, but switched to the pmndrs/postprocessing library instead after seeing that it has a significant performance increase over the three.js, and seems to have a better developed API overall.

I however have come upon some issues for the implementation for the model-viewer (note, I use bloom-effect here in reference to any possible post-processing effect, as thats what I tested with)

Due to the ThreeRenderer being a singleton shared across all modelviewer elements, enabling/disabling an effect will affect it globally for all modelviewer elements. This behavior isn't desirable in my opinion, but it's up for discussion
Due to 1, the current implementation where the effect is able to be activated or deactivated through adding the attribute to the DOM element ,i.e, <model-viewer ... bloom-effect></model-viewer. then we have the following issue:
If multiple modelviewer elements exist in the same window, the last one in the DOM decides whether or not bloom is activated:
- if it doesn't have the bloom-effect attribute, then it disables the effect globally, regardless of if previous elements declared the bloom-effect attribute
- same issue but vice versa, it may have it declared despite a previous element not wanting the bloom effect
  As such, my current implementation only allows activating the effects, while disabling them has to be done manually through code. This isn't perfect though, would love some input on a better solution
Specifically the BloomEffect has a well-known issue where adding bloom on a scene with a transparent background when the background has a high lightness value (i.e white) causes an unsightly effect. ThreeJS specifically solves the issue by just making the background black with their bloom effect, while the postprocessing library I used doesn't affect the background which is technically better, but still no real solution.

google-cla · 2022-11-18T17:49:32Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

elalish

Thanks a lot for this! Very cool possibilities here, but this is significant change (from 2 dependencies to 3) so this'll require a lot of checking. Also a significant API change that we'll want to get right so we don't have to break it down the road. Have you tried the SSAO? And how many devices have you tested this PR on?

packages/model-viewer/tsconfig.json

packages/modelviewer.dev/examples/scenegraph/index.html

packages/model-viewer/src/three-components/Renderer.ts

packages/model-viewer/src/features/post-processing.ts

packages/model-viewer/src/model-viewer-base.ts

packages/model-viewer/src/three-components/EnvironmentScene.ts

packages/model-viewer/src/three-components/PPRenderPipeline.ts

packages/model-viewer/package.json

Beilinson · 2022-11-21T11:42:35Z

I have checked the SSAO it's very subtle and not a dramatic visual impact (which is good imo). So far I've tested only on Mac (firefox/chrome). I'll test on windows and linux, also edge, but there's nothing that shouldn't work because all of the post processing effects just use the inbuilt three renderer to do some image manipulation, in fact the implementation of most effects is quite similar to the three inbuilt postprocessing

packages/model-viewer/src/features/post-processing.ts

elalish · 2022-11-21T11:49:14Z

Please show off some before/after visuals of these various effects; that will help convince people to use them! Also, how is it on mobile? How significant is the performance reduction?

Beilinson · 2022-11-21T21:26:03Z

Please show off some before/after visuals of these various effects; that will help convince people to use them! Also, how is it on mobile? How significant is the performance reduction?

I haven't checked on mobile, nor have I checked AR as I'm not sure how I can do that with a local-dev server. Testing on my old Macbook Air 2013 (1.3 Dual-Core i5, 4gb DDR3, Intel HD 5000 1.5gb vram) I see little to no performance impact from Bloom and SSAO combined.

elalish · 2022-11-22T14:28:43Z

When I do local testing on my mac, I just do npm run serve and then go to the 192.168... address it serves to on my iphone, which works as long as both devices are on the same wifi. For Android, you can also just plug in via USB, and open chrome://inspect to set up port forwarding.

Beilinson · 2022-11-23T19:43:25Z

Ok sounds great, will test this weekend and upload some screenshots

thibaud-be · 2022-12-05T12:28:14Z

I'm looking into adding SSR (screen space reflections) to my project using modelviewer, so I'm interested in the developement of this, sadly based on it's roadmap it seems like /pmndrs/postprocessing isn't going to implement SSR any time soon.
@Beilinson when you started implementing this, was there any other thing you noticed regarding the three.js inbuilt post-processing I should know about before looking into it ?

Beilinson · 2022-12-07T10:34:26Z

I'm looking into adding SSR (screen space reflections) to my project using modelviewer, so I'm interested in the developement of this, sadly based on it's roadmap it seems like /pmndrs/postprocessing isn't going to implement SSR any time soon.

@Beilinson when you started implementing this, was there any other thing you noticed regarding the three.js inbuilt post-processing I should know about before looking into it ?

Mainly that the three postprocessing is less customizable, some effects like Bloom are of lower render quality (but I'm sure that can be fixed), and that it's actually hidden in the /examples/jsm/postprocessing and not where they say in their documentation.

elalish · 2022-12-12T11:57:26Z

Where are we with this? I think I'm waiting for some screenshots and some details about transparent backgrounds. No rush, just making sure no one is waiting for me?

Beilinson · 2022-12-16T18:14:30Z

Uni is a bit busy right now, I'll start working on this again probably next week. Haven't sent screenshots coz I had some trouble getting the project to run on my vmware ubuntu and I don't have access to the macbook anymore

elalish · 2022-12-16T21:21:21Z

No problem. If you're on windows, we have directions for setup on WSL2: https://github.com/google/model-viewer#windows-1011-setup if that helps.

packages/modelviewer.dev/examples/postprocessing/index.html

elalish

Hey, tests passing, congrats! I'm going to go ahead and push a release before we merge this, since I'd like plenty of time for our users to find bugs through our docs pages before we release it. However, I also think we need to get pretty consistent rendering across browsers before we merge, or we'll get flooded with issues. Do you have access to any other test machines?

packages/model-viewer-effects/src/effects/fxaa.ts

packages/modelviewer.dev/examples/postprocessing/index.html

packages/model-viewer-effects/src/effect-composer.ts

Beilinson · 2023-04-05T19:20:48Z

I do have a Mac air 2013 that I'll have access to next weekend, and maybe a Mac Pro (non M1)

…viewer into post-processing

elalish

I think this one is ready, what do you think? With a PR of this magnitude I'm sure we could keep reviewing it forever, but I think I'd rather get it merged so users can test it out on our docs pages. And of course, further PRs are certainly welcome, and it'll make reviewing easier if they're separated.

Beilinson · 2023-04-23T08:24:36Z

I think it's ready as well! 🎆

zelghrabi-edu · 2023-04-25T19:48:55Z

Hello guys,
Great job 👏🏻
Is there any custom version released for testing purpose? I can do QA for this new feature
Thanks

elalish · 2023-04-25T22:59:52Z

@zelghrabi-edu Since I just released v3.1.1, I'm merging this now to make it easier to get feedback on. It'll be visible on modelviewer.dev, and you can pull the pre-release libraries by adjusting the URLs from https://modelviewer.dev/docs/faq.html#entrydocs-general-questions-api.

elalish · 2023-04-25T23:41:43Z

@Beilinson FYI: https://twitter.com/modelviewer/status/1651007361862225921 Happy to mention you if you have a twitter handle.

zelghrabi-edu · 2023-04-26T08:33:15Z

Thanks @elalish
I did checks on https://modelviewer.dev/examples/postprocessing/
Is it possible to have the bloom effect working on AR viewer as well like it’s the case now for 3D viewer please?

Thank you

Best

Beilinson · 2023-04-26T11:27:42Z

@Beilinson FYI: https://twitter.com/modelviewer/status/1651007361862225921 Happy to mention you if you have a twitter handle.

That would be nice! I'm part of a programmer collective @galaxyhuntersil . Thanks!

zelghrabi-edu · 2023-04-28T12:46:40Z

Hey @Beilinson
did you have time to check the issue I raised above?
Thanks in advance

Beilinson · 2023-04-28T13:19:20Z

Hey @zelghrabi-edu, there are two main issues with implementing postprocessing effects on AR:

For the native WebXRRenderer for three.js, it requires some tinkering with the renderers that isn't fully supported yet by pmndrs/postprocesssing, which shouldn't enter model-viewer as it is meant to be a fully stable API.
On most mobile devices <model-viewer> actually defaults to the native AR viewer, meaning <model-viewer-effects> actually has no control over the rendering. On IOS for example, this is because WebXR is mostly unsupported.

As such for now theres not too much to be done. Sorry :(

Beilinson · 2023-04-28T13:56:47Z

More on WebXR iOS support:

It is currently in an "experimental state", which is accessible through:
Settings -> Safari -> Advanced -> Experimental Features and all the way at the bottom:

It is disabled by default, and I am not sure how well it will work in the case of model-viewer, though it might be interesting to test.

Additionally, apple has the RealityKit API for AR experiences on IOS, which does allow for custom postprocessing, however this is only for full on IOS apps, while browsers are limited to quick look which is not customizable.

* Draft, began working on mixin and new rendering * first attempt at mixin and rendering with only bloom effect * updated imports * first attempt: working, but globally * switched post-processing libraries * better default settings * First iteration of enabling/disabling effects per scene * added effect map * Added basic docs * revert scenegraph docs changes * stash before pivot to webcomponent composition * effects-composer, pixelate-effect webcomponents * Created <model-viewer-effects> library, fixed bugs * updated package.json * Tests passing, dirtyRender for glitch, better custom Passes * added prepare back * Improved Glitch API, added BlendMode mixin * removed prefix, fixed SSAOEffect * Fixed SSAOEffect with transparency, tested three importmap * remove goldens from mve * Added no-three bundle to rollup, added test for image similarity * Better examples, easier to create custom effect component * . * fixed example * updated readme * depthWrite * added beforeRender callback * removed accidental styling changes from model-viewer * fixed double import issue * CR fixes, added selective bloom example with rocketship * removed browserstack from karma conf * improved screenshot tests * Added selective bloom effect * disconnect bug, rename to @google * updated versions, removed unneeded comments * Bootstrap works locally * updating dependencies * Tests passing, fixed bugs * added mve-docs, added msaa to effect-composer * Completed docs, fixed bugs * minor styling * fixed test * removed fxaa effect * update deps * improved string literal type safety * removed clearPass, fixed "empty" effectComposer * tonemapping + example (fixes bloom color) * minor fixes all around * fixed package-lock --------- Co-authored-by: Emmett Lalish <elalish@google.com>

samaneh-kazemi · 2024-08-06T22:26:26Z

Hi @Beilinson, I'm trying to introduce a new lit component to model-viewer and was looking at your changes as an example. I noticed that the main model-viewer class uses a template, while your changes don't.

https://github.com/aardvarkk/model-viewer/blob/master/packages/model-viewer/src/template.ts

Am I reading your design correctly? If yes, why did you decide to do it differently?

Beilinson · 2024-08-10T16:57:28Z

Hi @samaneh-kazemi, I recommend that you read a bit about Lit (the web component library powering model-viewer and model-viewer-effects). The template is required only if you are adding or affecting dom elements in the document. Model-viewer-effects only adds behavior to model-viewer through JS, and does not require any template.

Beilinson added 7 commits November 8, 2022 21:26

Draft, began working on mixin and new rendering

4c26d5c

first attempt at mixin and rendering with only bloom effect

0259d36

updated imports

af50f05

Merge branch 'master' into post-processing

979993a

first attempt: working, but globally

7153cc6

switched post-processing libraries

206dcaf

better default settings

2f6268b

Beilinson marked this pull request as draft November 18, 2022 17:49

Beilinson changed the title ~~Post processing [DRAFT]~~ Nov 18, 2022

First iteration of enabling/disabling effects per scene

acda054

Beilinson marked this pull request as ready for review November 18, 2022 22:05

elalish requested changes Nov 21, 2022

View reviewed changes

elalish reviewed Nov 21, 2022

View reviewed changes

packages/model-viewer/src/features/post-processing.ts Outdated Show resolved Hide resolved

Beilinson added 4 commits November 29, 2022 12:49

Merge branch 'master' into post-processing

fc85c9a

added effect map

05f91f5

Added basic docs

5f9c53b

revert scenegraph docs changes

f4d27a2

elalish reviewed Mar 6, 2023

View reviewed changes

packages/modelviewer.dev/examples/postprocessing/index.html Outdated Show resolved Hide resolved

stash before pivot to webcomponent composition

dd4e1f9

fixed test

1e37d9b

elalish reviewed Apr 5, 2023

View reviewed changes

packages/model-viewer-effects/src/effects/fxaa.ts Outdated Show resolved Hide resolved

packages/modelviewer.dev/examples/postprocessing/index.html Show resolved Hide resolved

packages/model-viewer-effects/src/effect-composer.ts Show resolved Hide resolved

Beilinson and others added 10 commits April 5, 2023 23:21

Merge branch 'master' into post-processing

f1bc4fb

removed fxaa effect

03d8b39

Merge branch 'post-processing' of https://github.com/Beilinson/model-…

d2832dd

…viewer into post-processing

update deps

3f5fb42

improved string literal type safety

21ad92a

removed clearPass, fixed "empty" effectComposer

c4574de

tonemapping + example (fixes bloom color)

b4902ef

minor fixes all around

6196d1a

fixed package-lock

ac77514

Merge branch 'master' into post-processing

f317808

elalish previously approved these changes Apr 19, 2023

View reviewed changes

Merge branch 'master' into post-processing

b9c8a4e

elalish dismissed their stale review via b9c8a4e April 25, 2023 22:37

elalish merged commit abbc1a0 into google:master Apr 25, 2023

Beilinson mentioned this pull request Apr 28, 2023

WebXR Postprocessing #4264

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Post processing #3948

Post processing #3948

Beilinson commented Nov 18, 2022

google-cla bot commented Nov 18, 2022

elalish left a comment

Beilinson commented Nov 21, 2022

elalish commented Nov 21, 2022

Beilinson commented Nov 21, 2022

elalish commented Nov 22, 2022

Beilinson commented Nov 23, 2022

thibaud-be commented Dec 5, 2022 •

edited

Loading

Beilinson commented Dec 7, 2022

elalish commented Dec 12, 2022 •

edited

Loading

Beilinson commented Dec 16, 2022

elalish commented Dec 16, 2022

elalish left a comment

Beilinson commented Apr 5, 2023 •

edited

Loading

elalish left a comment

Beilinson commented Apr 23, 2023

zelghrabi-edu commented Apr 25, 2023

elalish commented Apr 25, 2023

elalish commented Apr 25, 2023

zelghrabi-edu commented Apr 26, 2023 •

edited

Loading

Beilinson commented Apr 26, 2023

zelghrabi-edu commented Apr 28, 2023

Beilinson commented Apr 28, 2023 •

edited

Loading

Beilinson commented Apr 28, 2023 •

edited

Loading

samaneh-kazemi commented Aug 6, 2024

Beilinson commented Aug 10, 2024

Post processing #3948

Post processing #3948

Conversation

Beilinson commented Nov 18, 2022

google-cla bot commented Nov 18, 2022

elalish left a comment

Choose a reason for hiding this comment

Beilinson commented Nov 21, 2022

elalish commented Nov 21, 2022

Beilinson commented Nov 21, 2022

elalish commented Nov 22, 2022

Beilinson commented Nov 23, 2022

thibaud-be commented Dec 5, 2022 • edited Loading

Beilinson commented Dec 7, 2022

elalish commented Dec 12, 2022 • edited Loading

Beilinson commented Dec 16, 2022

elalish commented Dec 16, 2022

elalish left a comment

Choose a reason for hiding this comment

Beilinson commented Apr 5, 2023 • edited Loading

elalish left a comment

Choose a reason for hiding this comment

Beilinson commented Apr 23, 2023

zelghrabi-edu commented Apr 25, 2023

elalish commented Apr 25, 2023

elalish commented Apr 25, 2023

zelghrabi-edu commented Apr 26, 2023 • edited Loading

Beilinson commented Apr 26, 2023

zelghrabi-edu commented Apr 28, 2023

Beilinson commented Apr 28, 2023 • edited Loading

Beilinson commented Apr 28, 2023 • edited Loading

samaneh-kazemi commented Aug 6, 2024

Beilinson commented Aug 10, 2024

thibaud-be commented Dec 5, 2022 •

edited

Loading

elalish commented Dec 12, 2022 •

edited

Loading

Beilinson commented Apr 5, 2023 •

edited

Loading

zelghrabi-edu commented Apr 26, 2023 •

edited

Loading

Beilinson commented Apr 28, 2023 •

edited

Loading

Beilinson commented Apr 28, 2023 •

edited

Loading