google / model-viewer

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

Fix Postprocessing for r163 #4745

Closed elalish closed 2 months ago

elalish commented 2 months ago

Needed to bump the postprocessing version to keep up with the removal of deprecated encodings in Three.js. However, now we have a new problem:

postprocessing.js:274 Uncaught TypeError: Cannot read properties of undefined (reading 'Camera')

At this example: https://modelviewer.dev/examples/postprocessing/index.html#custom-effects

@Beilinson would you mind taking a look? I'm guessing this is related to your comment, which might not be true anymore:

// The camera is set automatically when added to the <effect-composer>

Beilinson commented 2 months ago

Sure, I'll look into it

Beilinson commented 2 months ago

seems at some point three.js stopped putting a global window.THREE variable, which postprocessing still expects when imported using a non-esm build. solutions:

  1. import from https://cdn.jsdelivr.net/npm/postprocessing@6.35.3/build/index.js instead (which is esm built so it runs import ... from "three" as expected), however it isn't minified and is about 2x as large as postprocessing.min.js
  2. set the global THREE variable manually before the postprocessing import:
    import * as THREE from "three";
    window.THREE = THREE;

    however this seems to cause all sorts of other problems, mainly that the minified file doesn't seem to export anything ...

tl;dr - changing the import to https://cdn.jsdelivr.net/npm/postprocessing@6.35.3/build/index.js fixes everything and the custom effects work as expected

elalish commented 2 months ago
  1. import from https://cdn.jsdelivr.net/npm/postprocessing@6.35.3/build/index.js instead (which is esm built so it runs import ... from "three" as expected), however it isn't minified and is about 2x as large as postprocessing.min.js

This sounds good to me - I'm not concerned about the library size for the example, as someone using it for real would likely make their own bundle anyway. I'll update next week. Thanks!