-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Post effects #5742
Post effects #5742
Changes from 11 commits
0a80bec
41812e7
d0f558c
bd35c63
5d19ae7
40bbcc4
ac8a705
7592168
3658dda
88e218d
0b4a425
c195866
8e3cd10
84bac6c
38d5cbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import type { AElement, AScene } from "aframe"; | |
import HubChannel from "./utils/hub-channel"; | ||
import MediaDevicesManager from "./utils/media-devices-manager"; | ||
|
||
import { EffectComposer, EffectPass } from "postprocessing"; | ||
import { | ||
Audio, | ||
AudioListener, | ||
|
@@ -21,6 +22,7 @@ import { | |
WebGLRenderer | ||
} from "three"; | ||
import { AudioSettings, SourceType } from "./components/audio-params"; | ||
import { createEffectsComposer } from "./effects"; | ||
import { DialogAdapter } from "./naf-dialog-adapter"; | ||
import { mainTick } from "./systems/hubs-systems"; | ||
import { waitForPreloads } from "./utils/preload"; | ||
|
@@ -87,6 +89,12 @@ export class App { | |
CURSOR: 3 | ||
}; | ||
|
||
fx: { | ||
composer?: EffectComposer; | ||
bloomAndTonemapPass?: EffectPass; | ||
tonemapOnlyPass?: EffectPass; | ||
} = {}; | ||
|
||
constructor() { | ||
// TODO: Create accessor / update methods for these maps / set | ||
this.world.eid2obj = new Map(); | ||
|
@@ -142,20 +150,20 @@ export class App { | |
event.preventDefault(); | ||
}); | ||
|
||
const enablePostEffects = this.store.state.preferences.enablePostEffects; | ||
|
||
const renderer = new WebGLRenderer({ | ||
// TODO we should not be using alpha: false https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/WebGL_best_practices#avoid_alphafalse_which_can_be_expensive | ||
alpha: true, | ||
antialias: true, | ||
depth: true, | ||
stencil: true, | ||
premultipliedAlpha: true, | ||
preserveDrawingBuffer: false, | ||
logarithmicDepthBuffer: false, | ||
// TODO we probably want high-performance | ||
antialias: !enablePostEffects, | ||
depth: !enablePostEffects, | ||
stencil: false, | ||
powerPreference: "high-performance", | ||
canvas | ||
}); | ||
|
||
// We manually handle resetting this in mainTick so that stats are correctly reported with post effects enabled | ||
renderer.info.autoReset = false; | ||
|
||
renderer.setPixelRatio(window.devicePixelRatio); | ||
|
||
renderer.debug.checkShaderErrors = qsTruthy("checkShaderErrors"); | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious, what is this change for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
const audioListener = new AudioListener(); | ||
this.audioListener = audioListener; | ||
|
@@ -179,13 +187,25 @@ export class App { | |
}; | ||
|
||
this.world.scene = sceneEl.object3D; | ||
const scene = sceneEl.object3D; | ||
|
||
// We manually call scene.updateMatrixWolrd in mainTick | ||
scene.autoUpdate = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this manual updating for post processing? Can auto update break the post processing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
OK, makese sense to me. I thought |
||
|
||
if (enablePostEffects) { | ||
this.fx = createEffectsComposer(canvas, renderer, camera, scene, sceneEl, this.store); | ||
} else { | ||
(sceneEl as any).addEventListener("rendererresize", function ({ detail }: { detail: DOMRectReadOnly }) { | ||
renderer.setSize(detail.width, detail.height, true); | ||
}); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does here mean renderer resize handling is done in the effect composer if post effects is enabled? (Maybe adding comments is good) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC the resizing is handled by the composer based on the Also in |
||
|
||
// This gets called after all system and component init functions | ||
sceneEl.addEventListener("loaded", () => { | ||
waitForPreloads().then(() => { | ||
this.world.time.elapsed = performance.now(); | ||
renderer.setAnimationLoop(function (_rafTime, xrFrame) { | ||
mainTick(xrFrame, renderer, sceneEl.object3D, camera); | ||
mainTick(xrFrame, renderer, scene, camera); | ||
}); | ||
sceneEl.renderStarted = true; | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// NOTE when changing be sure to check hardcoded mask values in hub.html | ||
export enum Layers { | ||
// These 3 layers are hardcoded in THREE | ||
CAMERA_LAYER_DEFAULT, | ||
CAMERA_LAYER_XR_LEFT_EYE, | ||
CAMERA_LAYER_XR_RIGHT_EYE, | ||
|
||
CAMERA_LAYER_REFLECTION, | ||
CAMERA_LAYER_INSPECT, | ||
CAMERA_LAYER_VIDEO_TEXTURE_TARGET, | ||
|
||
CAMERA_LAYER_THIRD_PERSON_ONLY, | ||
CAMERA_LAYER_FIRST_PERSON_ONLY, | ||
|
||
CAMERA_LAYER_UI, | ||
CAMERA_LAYER_FX_MASK | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, you have removed
premultipliedAlpha
,preserveDrawingBuffer
, andlogarithmicDepthBuffer
because they use the default values?https://threejs.org/docs/#api/en/renderers/WebGLRenderer.preserveDrawingBuffer
And you have toggled
stencil
tofalse
because we don't use and/or it blocks post processing?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, exactly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope we can have a separated clean up PR from a logic addition/change PR because review can be easier.