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

PMREMNode: Only generate PMREM with ready image data. #28970

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jul 25, 2024

Related issue: #28827

Description

This PR fixes point 2. of #28827.

Sometimes when loading a page with a textured background, the texture isn't rendered at all and is black. When refreshing the page, the issue disappears. This does not happen if the code waits until the env map has been loaded (like in webgpu_loader_gltf). However, the new webgpu_materials_basic does not so it can be used as a test case.

The proposed fix is similar to how the previous automatic cube map and cube uv conversion of WebGLRenderer works. Meaning before environment maps are converted, it's important to wait until image data are ready.

Copy link

github-actions bot commented Jul 25, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
684 kB (169.4 kB) 684 kB (169.4 kB) +0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
460.9 kB (111.2 kB) 460.9 kB (111.2 kB) +0 B

@sunag
Copy link
Collaborator

sunag commented Jul 25, 2024

pmremTexture() should contemplate this but it has not been implemented yet, if this update is done inside pmremTexture(), the solution level would be expanded.

@Mugen87 Mugen87 changed the title Nodes: Ensure background update waits for loaded images. PMREMNode: Only generate PMREM with ready image data. Jul 25, 2024
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jul 25, 2024

I've updated the PR and moved the logic inside PMREMNode. This is indeed the more general solution.

@sunag sunag added this to the r168 milestone Jul 25, 2024
@sunag sunag merged commit f8a6f72 into mrdoob:dev Jul 25, 2024
12 checks passed
brunosimon pushed a commit to brunosimon/three.js that referenced this pull request Jul 25, 2024
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jul 31, 2024

The PR fixes the issue for the WebGPU backend, but not when using WebGL.

Previously, the background was black when the source textures weren't ready. Now it renders the PMREM but not with the correct mapping. This can be reproduced with webgpu_materials_basic(I've just removed the spheres for better visibility).

image

I wonder why the backends behave differently 🤔 .

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.

2 participants