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

Object3D: Add onBeforeShadow and onAfterShadow callbacks #25933

Closed
wants to merge 0 commits into from

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Apr 25, 2023

Fixed #14921.

This PR introduces two new optional callbacks to the Object3D class in three.js. The onBeforeShadow and onAfterShadow callbacks allow developers to execute custom code immediately before and after the shadow of a 3D object is rendered, respectively. These callbacks provide the following parameters: renderer, scene, camera, shadowCamera, geometry, depthMaterial, and group.

Example usage:

cube.onBeforeShadow = function ( renderer, scene, camera, shadowCamera, geometry, depthMaterial, group ) {

	anothercube.visible = true;
	anothercube.modelViewMatrix.multiplyMatrices( shadowCamera.matrixWorldInverse, anothercube.matrixWorld );
	renderer.renderBufferDirect( shadowCamera, null, anothercube.geometry, depthMaterial, anothercube, group );

};

cube.onAfterShadow = function ( ) {

	anothercube.visible = false;

};

image

These new callbacks offer developers more control and flexibility when working with shadows in their projects.

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
636.3 kB (157.7 kB) 636.5 kB (157.8 kB) +173 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
426.8 kB (103.6 kB) 427 kB (103.6 kB) +173 B
@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 25, 2023

Related: It was tried to implement this once via #17932. Based on the discussion in #14921 and the PR I'm not sure why the change was never merged. But maybe the signature of the callback was an issue...

@RenaudRohlinger
Copy link
Collaborator Author

Oh, didn't check the existing PRs, my bad.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 25, 2023

I see you keep the signature similar to onBeforeRender() and onAfterRender(). I think that makes sense.

At first glance, the change looks good to me!

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 25, 2023

The names of the callback could maybe more compact. Meaning onBeforeShadow() and onAfterShadow(). I guess I prefer those but the ultimate decision is up to @mrdoob .

@Mugen87 Mugen87 added this to the r153 milestone Apr 27, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented May 3, 2023

@sunag Just to clarify: Do you think these callbacks would also be useful in context of WebGPURenderer? Or does the node system enable other potential approaches to achieve the same (meaning influencing the shadow map rendering process).

@RenaudRohlinger We should clarify this because ideally new callbacks on Object3D level make sense for both renderers. If we merge this PR, we eventually have to add support to WebGPURenderer, too.

Copy link
Collaborator Author

@RenaudRohlinger RenaudRohlinger left a comment

Choose a reason for hiding this comment

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

Renamed both methods to onBeforeShadow and onAfterShadow

@RenaudRohlinger RenaudRohlinger changed the title Object3D: Add onBeforeShadowMapRender and onAfterShadowMapRender callbacks May 3, 2023
@sunag
Copy link
Collaborator

sunag commented May 3, 2023

@sunag Just to clarify: Do you think these callbacks would also be useful in context of WebGPURenderer? Or does the node system enable other potential approaches to achieve the same (meaning influencing the shadow map rendering process).

WebGPURenderer is more simplified to the point that we don't need a WebGPUShadowMap for example, and the controls are done in the nodes. We also don't have a renderBufferDirect() and if we implement it we would have to have more arguments, although in this example we could also replace the geometry in the object itself after it recovers once renderBufferDirect() is called anyway after onBeforeShadowMapRender() if the case is LOD only, we would have other use cases for that?

Or does the node system enable other potential approaches to achieve the same (meaning influencing the shadow map rendering process).

I think it makes sense for WebGPURenderer, maybe we need to add support for this in nodes. Maybe something like .depthNode could solve this.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 4, 2023

In this case I suggest to stall this PR a bit until we a more confident on the WebGPU side.

Mugen87
Mugen87 previously approved these changes May 12, 2023
@Mugen87 Mugen87 modified the milestones: r153, r154 Jun 1, 2023
@mrdoob mrdoob modified the milestones: r154, r155 Jun 29, 2023
@mrdoob mrdoob modified the milestones: r155, r156 Jul 27, 2023
@mrdoob mrdoob modified the milestones: r156, r157 Aug 31, 2023
@mrdoob mrdoob modified the milestones: r157, r158 Sep 28, 2023
@mrdoob mrdoob modified the milestones: r158, r159 Oct 27, 2023
@mrdoob mrdoob removed this from the r159 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants