Post effects #5742

netpro2k · 2022-10-11T02:09:11Z

This PR adds support for Post Processing effects, starting with Bloom. At first this will be double opt-in, with both users needing to enable a preference, and scene authors needing to export with an updated environment-settings component configured for post effects. The plan is to make the user preference default enabled eventually on systems that can handle it. It is currently not supported in VR and the preference will be hidden if we detect the user is on a standalone VR headset.

Currently when opting into the new "HDR pipeline", tonemapping settings will be ignored and ACES will always be used. It's likely we will want this to be configurable eventually but supporting our existing tonemapping options (namely the Blender LUT one) is a bit complicated and requires some changes to the underlying postprocessing library. Its possible we will want to standardize on ACES and then offer additional LUT after tonemaping for artistic effects. We also likely want to explore adaptive exposure, which will be difficult to support for all tonemapping modes.

This PR also slightly re-organizes the graphics options in the preferences screen. Ideally we will want to eventually have a separate tab for this, but doing so requires rethinking the UI a bit for mobile so is out of scope for this PR. That likely would pair well with a PR around better configuration of graphics defaults.

Blender addon: Hubs-Foundation/hubs-blender-exporter#142

takahirox · 2022-10-12T23:51:21Z

src/systems/camera-system.js

-            scale
-          )
-        );
+        setMatrixWorld(this.avatarRig.object3D, new THREE.Matrix4().compose(position, quat, scale));


Better to avoid to create a new matrix instance in tick (Sorry, this is an existing issue. Making another PR is ok.)

Yeah this was just a formatting change. I see a few spots in here we are creating some garbage.

takahirox · 2022-10-12T23:53:44Z

Thanks for the PR.

This PR also slightly re-organizes the graphics options in the preferences screen

Would you mind putting the screenshot of the new preferences screen here?

Question: Does the post-processing work even in the immersive (VR) mode?

takahirox · 2022-10-12T23:58:52Z

src/app.ts

-      stencil: true,
-      premultipliedAlpha: true,
-      preserveDrawingBuffer: false,
-      logarithmicDepthBuffer: false,


Just to clarify, you have removed premultipliedAlpha, preserveDrawingBuffer, and logarithmicDepthBuffer because they use the default values?

https://threejs.org/docs/#api/en/renderers/WebGLRenderer.preserveDrawingBuffer

And you have toggled stencil to false because we don't use and/or it blocks post processing?

Yep, exactly

I hope we can have a separated clean up PR from a logic addition/change PR because review can be easier.

takahirox · 2022-10-13T00:04:07Z

src/app.ts

@@ -166,7 +174,7 @@ export class App {

    sceneEl.appendChild(renderer.domElement);

-    const camera = new PerspectiveCamera(80, window.innerWidth / window.innerHeight, 0.05, 10000);
+    const camera = new PerspectiveCamera(80, canvas.width / canvas.height, 0.05, 10000);


Curious, what is this change for?

Ah that may not actually be needed anymore. Early on when working on this I was using a static size canvas to simplify things. This value doesn't particularly matter anyway since it gets overwritten again once useResizeViewport runs, but I will ditch the change.

We end up doing way more resizing of the canvas than I would like but its a bit tricky to untangle right now since our initialization is still driven by aframe, then react, so at this point we don't actually know what the dimensions of the canvas will end up being.

takahirox · 2022-10-13T00:08:31Z

src/app.ts

+      (sceneEl as any).addEventListener("rendererresize", function ({ detail }: { detail: DOMRectReadOnly }) {
+        renderer.setSize(detail.width, detail.height, true);
+      });
+    }


Does here mean renderer resize handling is done in the effect composer if post effects is enabled? (Maybe adding comments is good)

IIUC the resizing is handled by the composer based on the useViewport hook resize listener. We are setting the canvas style size in the hook but here we are forcing a style update that we weren't doing before and I'm not sure why.

Also in effects.ts we are also forcing a style update when updating the renderer size but we are already doing that in the useViewport hook so we are overriding that update.

takahirox · 2022-10-13T00:25:41Z

src/effects.ts

+  MSAA_2X = "MASS_2X",
+  MSAA_4X = "MASS_4X",
+  MSAA_8X = "MASS_8X"
+}


Just wondering whether we really need five aa options because it may add complexity in the preferences. For example aren't just two options (ON(MSAA_4X?)/OFF(NONE)) good? No big preference tho...

I think we want at least off, SMAA, and some form of MSAA. I mostly included all of these so that people can test different options for visual differences and perf on different devices.

I did notice the constants for MSAA all have a typo though.. will fix

takahirox · 2022-10-13T00:50:07Z

src/app.ts

+    const scene = sceneEl.object3D;
+
+    // We manually call scene.updateMatrixWolrd in mainTick
+    scene.autoUpdate = false;


Is this manual updating for post processing? Can auto update break the post processing?

Post processing can call render multiple times, so it makes more sense to just manually update ourselves once. This is also something we wanted to do anyway so that we can easily see where in the frame matrix updates are happening and schedule systems before/after that.

Post processing can call render multiple times, so it makes more sense to just manually update ourselves once.

OK, makese sense to me. I thought postprocessing lib set scene.autoUpdate false but if it doesn't, setting scene.autoUpdate false and manually calling update matrix world in our code makes sense.

takahirox · 2022-10-13T00:59:20Z

src/react-components/room/useResizeViewport.js

+      // In VR mode, three handles canvas resize based on the dimensions returned by
+      // the getEyeParameters function of the WebVR API. These dimensions are independent of
+      // the window size, therefore should not be overwritten with the window's width and
+      // height, // except when in fullscreen mode.


Extra // after height,

takahirox · 2022-10-13T01:05:15Z

src/react-components/room/useResizeViewport.js

-        const isVRPresenting = scene.renderer.xr.enabled && isPresenting;
+      // Do not update renderer, if a camera or a canvas have not been injected.
+      // In VR mode, three handles canvas resize based on the dimensions returned by
+      // the getEyeParameters function of the WebVR API. These dimensions are independent of


Does this mean Three.js still handles WebVR API? I thought it has dropped WebVR API support...

I think this comment is just out of date. It is still true though that WebXRManager handles the size of its framebuffers internally.

takahirox · 2022-10-13T01:22:44Z

src/react-components/preferences-screen.js

+            key: "aaMode",
+            prefType: PREFERENCE_LIST_ITEM_TYPE.SELECT,
+            disableIfFalse: "enablePostEffects",
+            promptForRefresh: true,


Do enableBloom and aaMode toggles really require the refresh? Can't we skip the effect passes if they are turned off?

I think both of these could probably be made to work but it seemed simpler just to have users refresh for now. Toggling the main option definitely requires a refresh since it changes how we initialize our WebGL context.

takahirox · 2022-10-13T01:30:41Z

src/effects.ts

+) {
+  const composer = new EffectComposer(renderer, {
+    frameBufferType: HalfFloatType
+  });


The core post-processing logic is in an external library postprocessing. I don't think I have time to check the lib internal so want to check if it works fine on the test after landing the post-processing feature.

Yep. Its definitely worth us reading through and understanding the internals well. I have read a lot of it myself, particularly for the effects we are using.

takahirox · 2022-10-13T01:39:04Z

src/systems/environment-system.js

+    bloomFolder.add(debugSettings.bloom, "radius", 0, 1, 0.001).onChange(updateDebug).listen();
+    bloomFolder.add(debugSettings.bloom, "smoothing", 0, 1, 0.001).onChange(updateDebug).listen();
+    // bloomFolder.add(bloom.blendMode, "blendFunction", BlendFunction);
+    // bloomFolder.add(bloom.blendMode.opacity, "value", 0, 1).name("Opacity");


Out of the scope in the PR: Wondering if we can separate the core logic code/files and debug code/files more clearly. The mix of core and debug codes can make the code less readable especially for new people.

Yeah I agree. I was debating if I should just remove this debug stuff altogether but it seems really useful for early testing on this. I agree we should figure out a nicer way to split out debug logic.

keianhzo · 2022-10-20T13:30:23Z

src/app.ts

+      (sceneEl as any).addEventListener("rendererresize", function ({ detail }: { detail: DOMRectReadOnly }) {
+        renderer.setSize(detail.width, detail.height, true);
+      });
+    }


IIUC the resizing is handled by the composer based on the useViewport hook resize listener. We are setting the canvas style size in the hook but here we are forcing a style update that we weren't doing before and I'm not sure why.

Also in effects.ts we are also forcing a style update when updating the renderer size but we are already doing that in the useViewport hook so we are overriding that update.

keianhzo · 2022-10-20T13:31:37Z

src/effects.ts

+  }
+
+  (sceneEl as any).addEventListener("rendererresize", function ({ detail }: { detail: DOMRectReadOnly }) {
+    composer.setSize(detail.width, detail.height, true);


Do we want to override the canvas style update here? useViewport is already setting the style size so we are overriding it here.

Ah yeah good catch. Updated here and in EffectComposer setSize call as well. As written its actually a bug since sometimes we want to render at a lower resolution than the canvas is sized to.

keianhzo · 2022-10-20T13:33:32Z

Note: I mistakenly set the review to "request changes" but my comments are mostly questions.

jywarren · 2022-12-11T20:09:52Z

Many wows! ❤️ This is very exciting!

Looking at https://www.npmjs.com/package/postprocessing, i see it can support depth of field (#5415), god rays, ambient occlusion, glitch, pixellation, and others.

Nice demos too: https://pmndrs.github.io/postprocessing/public/demo/#depth-of-field

Are there plans to enable many of these?

Also, I noticed a difference in how some items are lit, or maybe opacity differences, I believe, when this is enabled; these are alpha-transparent cloud PNGs in a particle system to simulate smoke; i can just adjust settings to fix but in case it's an issue you want to see reported:

Before:

After:

tanfarming closed this Oct 11, 2022

tanfarming deleted the post-effects branch October 11, 2022 15:53

tanfarming restored the post-effects branch October 11, 2022 15:53

tanfarming reopened this Oct 11, 2022

netpro2k mentioned this pull request Oct 12, 2022

Add bloom to environment settings Hubs-Foundation/hubs-blender-exporter#142

Merged

netpro2k requested review from takahirox and johnshaughnessy October 12, 2022 18:58

netpro2k added 11 commits October 12, 2022 14:13

WIP post effects testbed

0a80bec

WIP integrate FX into environment settings & prefs

41812e7

Cleanup

d0f558c

More cleanup

bd35c63

Delint + remove dead code

5d19ae7

Fix Layer import defining AFRAME component

40bbcc4

Remove commented defaults from WebGLRenderer init

ac8a705

Fix canvas not resizing when posteffects are off

7592168

Fix bloom debug UI

3658dda

delint

88e218d

cleanup

0b4a425

netpro2k force-pushed the post-effects branch from d822fc7 to 0b4a425 Compare October 12, 2022 21:13

takahirox reviewed Oct 12, 2022

View reviewed changes

takahirox reviewed Oct 13, 2022

View reviewed changes

Fix admin build

c195866

netpro2k had a problem deploying to hc-bio October 13, 2022 01:22 Failure

takahirox reviewed Oct 13, 2022

View reviewed changes

keianhzo requested changes Oct 20, 2022

View reviewed changes

netpro2k added 3 commits October 20, 2022 13:53

Hide postFX pref on VR devices & tag experimental

8e3cd10

Fix canvas style incorrectly being overridden

84bac6c

Cleanup

38d5cbf

netpro2k had a problem deploying to hc-bio October 20, 2022 21:26 Failure

keianhzo approved these changes Oct 21, 2022

View reviewed changes

netpro2k merged commit ecadbbc into master Oct 25, 2022

netpro2k deleted the post-effects branch October 25, 2022 00:00

This was referenced Dec 11, 2022

Depth of field shader #5415

Open

Introduce postprocessing API aframevr/aframe#3645

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Post effects #5742

Post effects #5742

netpro2k commented Oct 11, 2022 •

edited

Loading

takahirox Oct 12, 2022

netpro2k Oct 20, 2022

takahirox commented Oct 12, 2022 •

edited

Loading

takahirox Oct 12, 2022 •

edited

Loading

netpro2k Oct 20, 2022

takahirox Oct 20, 2022

takahirox Oct 13, 2022

netpro2k Oct 20, 2022

takahirox Oct 13, 2022 •

edited

Loading

keianhzo Oct 20, 2022

takahirox Oct 13, 2022 •

edited

Loading

netpro2k Oct 20, 2022

netpro2k Oct 20, 2022

takahirox Oct 13, 2022

netpro2k Oct 20, 2022

takahirox Oct 20, 2022

takahirox Oct 13, 2022

takahirox Oct 13, 2022

netpro2k Oct 20, 2022

takahirox Oct 13, 2022

netpro2k Oct 20, 2022

takahirox Oct 13, 2022

netpro2k Oct 20, 2022

takahirox Oct 13, 2022

netpro2k Oct 20, 2022

keianhzo Oct 20, 2022

keianhzo Oct 20, 2022

netpro2k Oct 20, 2022

keianhzo commented Oct 20, 2022

jywarren commented Dec 11, 2022

Post effects #5742

Post effects #5742

Conversation

netpro2k commented Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox commented Oct 12, 2022 • edited Loading

takahirox Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo commented Oct 20, 2022

jywarren commented Dec 11, 2022

netpro2k commented Oct 11, 2022 •

edited

Loading

takahirox commented Oct 12, 2022 •

edited

Loading

takahirox Oct 12, 2022 •

edited

Loading

takahirox Oct 13, 2022 •

edited

Loading

takahirox Oct 13, 2022 •

edited

Loading