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

Adding a position options for the PMREM fromScene generation #30071

Open
Samsy opened this issue Dec 9, 2024 · 4 comments
Open

Adding a position options for the PMREM fromScene generation #30071

Samsy opened this issue Dec 9, 2024 · 4 comments

Comments

@Samsy
Copy link
Contributor

Samsy commented Dec 9, 2024

Description

Currently, calling a PMREM Generation is calling a cubecamera that is always default positionned at 0, 0, 0

For a few versions now, I'm hacking my way to get to generate a PMREM but from an arbitrary cubecamera position by passing a position to pmrem.fromScene, this would be useful

Solution

Passing a position as arg to fromScene

Alternatives

Or maybe passing a callback to be called before each render that passes the camera ?
in that case we'd be able to do anything before render of each pmrem face

Additional context

No response

Repository owner deleted a comment Dec 9, 2024
@Spiri0
Copy link
Contributor

Spiri0 commented Dec 11, 2024

@Mugen87 can you still remember it? #28334
I tried to solve that back then. This was probably one of my first attempts to understand something in threejs.
But I still had a mistake there. With the extension I tried to make size and position optional and to fix the reflection error that Sunag recently fixed with a PR #30063 because that also came up recently. Maybe my approach from back then can be useful

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 13, 2025

Maybe my approach from back then can be useful

I agree. The conclusion of earlier discussions was that having render target size and camera position in control when using fromScene() is favorable. Do you think you can create a new version of #28334? #28334 hasn't been merged because of the reflection issue which should be fixed in the meanwhile.

@Spiri0
Copy link
Contributor

Spiri0 commented Jan 13, 2025

Hi @Mugen87, I'll take a look at this over the weekend. Should be solvable.

@Spiri0
Copy link
Contributor

Spiri0 commented Jan 16, 2025

I've already found time for it and submitted a PR #30334
I admit that the example could be made more extensive, but the essence is easy to see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants