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.

Sign up for GitHub

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

Jump to bottom

I have changed the new PMREMGenerator and the associated PMREMNode #28334

Closed
wants to merge 11 commits into from
Closed

I have changed the new PMREMGenerator and the associated PMREMNode #28334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9c6f24e
I added an adjustable renderTarget size to the new PMREMGenerator and…
Spiri0 May 11, 2024
bc6b3ce
Update PMREMNode.js
Spiri0 May 11, 2024
751bac8
I also made the changes I made in the PMREMGenerator in the node syst…
Spiri0 May 25, 2024
86ccd08
Update PMREMGenerator.js
Spiri0 May 26, 2024
7aa7b91
Update PMREMGenerator.js
Spiri0 May 26, 2024
994b3e7
Update PMREMGenerator.js
Spiri0 May 26, 2024
a51382e
Update PMREMGenerator.js
Spiri0 May 26, 2024
dba7265
Update PMREMGenerator.js
Spiri0 May 26, 2024
b008922
Update PMREMGenerator.js
Spiri0 May 26, 2024
abd9aa4
Merge branch 'mrdoob:dev' into dev
Spiri0 Jun 6, 2024
f0ee877
I made a WebGPURenderTarget because at the moment it is not possible …
Spiri0 Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions examples/jsm/nodes/pmrem/PMREMNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { textureCubeUV } from './PMREMUtils.js';
import { uniform } from '../core/UniformNode.js';
import { NodeUpdateType } from '../core/constants.js';
import { nodeProxy, vec3 } from '../shadernode/ShaderNode.js';
import { WebGLCoordinateSystem } from 'three';

let _generator = null;

Expand Down Expand Up @@ -162,7 +161,7 @@ class PMREMNode extends TempNode {

const texture = this.value;

if ( builder.renderer.coordinateSystem === WebGLCoordinateSystem && texture.isPMREMTexture !== true && texture.isRenderTargetTexture === true ) {
if ( texture.isRenderTargetTexture === true ) {
Copy link
Collaborator

@Mugen87 Mugen87 May 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: Please avoid addressing multiple issues with a single PR.

We can leave it for now but in the future try to fix different issues in different PRs please. That makes them easier to review and merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that it had to be one PR because the changes in both files (PMREMGenerator and PMREMNode) belong together for it to work properly. Was the idea wrong?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, I don't understand. It seems the issue in the fiddle can be fixed without enhancing the API of PMREMGenerator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fiddle seems to offers more options than CodePen, so I'll look into that too. But I'm happy that it worked and that it can now be experienced live with the fiddle.
Thanks for doing that, Mugen

The one with the two issues: Of course thats already possible. If you just change the pmremNode without the sign correction in the x axis in the PMREMGenerator, then the x axis reflection is not correct and that is independent of the camera translation.
The camera translation has no influence at all and is therefore actually a different topic. The bottom line for me is that it's just important to get it done properly and if those are two issues I can live with that very well.

I'm happy that the PMREMGenerator has come and the pmremNode, as the WebGLCubeRenderTarget previously seemed simply out of place in WebGPU, even though it worked well.


uvNode = vec3( uvNode.x.negate(), uvNode.yz );

Expand Down
74 changes: 74 additions & 0 deletions examples/jsm/renderers/common/WebGPURenderTarget.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { RenderTarget } from 'three';


class WebGPURenderTarget {

constructor( width = 1, height = 1, options = {} ) {

this.readBuffer = new RenderTarget( width, height, options );
this.writeBuffer = new RenderTarget( width, height, options );

}

swap() {

[this.readBuffer, this.writeBuffer ] = [ this.writeBuffer, this.readBuffer ];

}

get read() {

return this.readBuffer;
}

get write() {

return this.writeBuffer;
}

get texture() {

return this.readBuffer.texture;
}

setSize( width, height ) {

this.readBuffer.setSize( width, height );
this.writeBuffer.setSize( width, height );

}

dispose() {

this.readBuffer.dispose();
this.writeBuffer.dispose();

}

render( renderer, scene, camera ) {

renderer.setRenderTarget( this.writeBuffer );
renderer.render( scene, camera );
renderer.setRenderTarget( null );

this.swap();

return this;

}

async renderAsync( renderer, scene, camera ) {

renderer.setRenderTarget( this.writeBuffer );
await renderer.renderAsync( scene, camera );
renderer.setRenderTarget( null );

this.swap();

return this;

}

}

export default WebGPURenderTarget;
22 changes: 13 additions & 9 deletions examples/jsm/renderers/common/extras/PMREMGenerator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,22 @@ class PMREMGenerator {
* Generates a PMREM from a supplied Scene, which can be faster than using an
* image if networking bandwidth is low. Optional sigma specifies a blur radius
* in radians to be applied to the scene before PMREM generation. Optional near
* and far planes ensure the scene is rendered in its entirety (the cubeCamera
* is placed at the origin).
* and far planes ensure the scene is rendered in its entirety
* Optional size, the renderTarget default size is 256
* Optional cubeCamera position, the cubeCamera default position is the origin
*/
fromScene( scene, sigma = 0, near = 0.1, far = 100 ) {
fromScene( scene, sigma = 0, near = 0.1, far = 100, size = 256, position = new Vector3( 0, 0, 0 ) ) {

_oldTarget = this._renderer.getRenderTarget();
_oldActiveCubeFace = this._renderer.getActiveCubeFace();
_oldActiveMipmapLevel = this._renderer.getActiveMipmapLevel();

this._setSize( 256 );
this._setSize( size );

const cubeUVRenderTarget = this._allocateTargets();
cubeUVRenderTarget.depthBuffer = true;

this._sceneToCubeUV( scene, near, far, cubeUVRenderTarget );
this._sceneToCubeUV( scene, near, far, cubeUVRenderTarget, position );

if ( sigma > 0 ) {

Expand Down Expand Up @@ -324,7 +325,7 @@ class PMREMGenerator {

}

_sceneToCubeUV( scene, near, far, cubeUVRenderTarget ) {
_sceneToCubeUV( scene, near, far, cubeUVRenderTarget, position ) {

const cubeCamera = _cubeCamera;
cubeCamera.near = near;
Expand Down Expand Up @@ -394,17 +395,20 @@ class PMREMGenerator {
if ( col === 0 ) {

cubeCamera.up.set( 0, upSign[ i ], 0 );
cubeCamera.lookAt( forwardSign[ i ], 0, 0 );
cubeCamera.position.set( position.x, position.y, position.z );
cubeCamera.lookAt( position.x - forwardSign[ i ], position.y, position.z );

} else if ( col === 1 ) {

cubeCamera.up.set( 0, 0, upSign[ i ] );
cubeCamera.lookAt( 0, forwardSign[ i ], 0 );
cubeCamera.position.set( position.x, position.y, position.z );
cubeCamera.lookAt( position.x, position.y + forwardSign[ i ], position.z );

} else {

cubeCamera.up.set( 0, upSign[ i ], 0 );
cubeCamera.lookAt( 0, 0, forwardSign[ i ] );
cubeCamera.position.set( position.x, position.y, position.z );
cubeCamera.lookAt( position.x, position.y, position.z + forwardSign[ i ] );

}

Expand Down
21 changes: 13 additions & 8 deletions src/extras/PMREMGenerator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,12 @@ class PMREMGenerator {
* Generates a PMREM from a supplied Scene, which can be faster than using an
* image if networking bandwidth is low. Optional sigma specifies a blur radius
* in radians to be applied to the scene before PMREM generation. Optional near
* and far planes ensure the scene is rendered in its entirety (the cubeCamera
* and far planes ensure the scene is rendered in its entirety
* Optional size, the renderTarget default size is 256
* Optional cubeCamera position, the cubeCamera default position is the origin
* is placed at the origin).
*/
fromScene( scene, sigma = 0, near = 0.1, far = 100 ) {
fromScene( scene, sigma = 0, near = 0.1, far = 100, size = 256, position = new Vector3( 0, 0, 0 ) ) {

_oldTarget = this._renderer.getRenderTarget();
_oldActiveCubeFace = this._renderer.getActiveCubeFace();
Expand All @@ -112,12 +114,12 @@ class PMREMGenerator {

this._renderer.xr.enabled = false;

this._setSize( 256 );
this._setSize( size );

const cubeUVRenderTarget = this._allocateTargets();
cubeUVRenderTarget.depthBuffer = true;

this._sceneToCubeUV( scene, near, far, cubeUVRenderTarget );
this._sceneToCubeUV( scene, near, far, cubeUVRenderTarget, position );

if ( sigma > 0 ) {

Expand Down Expand Up @@ -306,7 +308,7 @@ class PMREMGenerator {

}

_sceneToCubeUV( scene, near, far, cubeUVRenderTarget ) {
_sceneToCubeUV( scene, near, far, cubeUVRenderTarget, position ) {
Copy link
Collaborator

@Mugen87 Mugen87 May 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this code locally and it changes the result of PMREMGenerator which seems wrong. You can clearly see this with webgl_loader_gltf_dispersion which lighting is now different.

Making the size and origin of fromScene() configurable should not affect current usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have to take a closer look at that. The PMREMGenerator in the src differs in some parts from the one in the node system.
I've been busy analyzing another issue over the last few days #28519. This was then quickly solved by aardgoose with #28523. I then tested that straight away and it works just fine.


const fov = 90;
const aspect = 1;
Expand Down Expand Up @@ -358,17 +360,20 @@ class PMREMGenerator {
if ( col === 0 ) {

cubeCamera.up.set( 0, upSign[ i ], 0 );
cubeCamera.lookAt( forwardSign[ i ], 0, 0 );
cubeCamera.position.set( position.x, position.y, position.z );
cubeCamera.lookAt( position.x - forwardSign[ i ], position.y, position.z );

} else if ( col === 1 ) {

cubeCamera.up.set( 0, 0, upSign[ i ] );
cubeCamera.lookAt( 0, forwardSign[ i ], 0 );
cubeCamera.position.set( position.x, position.y, position.z );
cubeCamera.lookAt( position.x, position.y + forwardSign[ i ], position.z );

} else {

cubeCamera.up.set( 0, upSign[ i ], 0 );
cubeCamera.lookAt( 0, 0, forwardSign[ i ] );
cubeCamera.position.set( position.x, position.y, position.z );
cubeCamera.lookAt( position.x, position.y, position.z + forwardSign[ i ] );

}

Expand Down
Loading