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

Fix Postprocessing for r163 #4745

Merged
merged 2 commits into from
Apr 15, 2024
Merged

Fix Postprocessing for r163 #4745

merged 2 commits into from
Apr 15, 2024

Conversation

elalish
Copy link
Collaborator

@elalish elalish commented Apr 12, 2024

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
Copy link
Contributor

Sure, I'll look into it

@Beilinson
Copy link
Contributor

Beilinson commented Apr 13, 2024

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
Copy link
Collaborator Author

elalish commented Apr 13, 2024

  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!

@elalish elalish merged commit c9b75ba into master Apr 15, 2024
6 checks passed
@elalish elalish deleted the gridEffect branch April 15, 2024 18:17
JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request Apr 22, 2024
* not quite fixed

* update library link
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.

None yet

2 participants