Three.js: PointLightShadow is not exported

[dmnsgn]

Description of the problem

https://github.com/mrdoob/three.js/blob/5a8c62fe37420429377de25885acf8878f2bcb1e/src/Three.js#L56-L63 LightShadow, DirectionalLightShadow and SpotLightShadow are exported but not PointLightShadow.

Three.js version

• [X] Dev

Browser

• [x] All of them

  OS

• [x] All of them

Hardware Requirements (graphics card, VR Device, ...)

N/A

[Mugen87]

I don't think any LightShadow class should be exported. They are for internal use only.

[WestLangley]

I don't think any LightShadow class should be exported. They are for internal use only.

Actually, I use this pattern:

// setting a custom shadow frustum
light.shadow = new THREE.LightShadow( new THREE.PerspectiveCamera( 22.5, 1, 200, 700 ) );

The default frustum is unnecessarily oversized for my preference.

[Mugen87]

In this case, I think it's better to modify the existing shadow object instead. Besides, none of the examples uses this pattern.

Don't get me wrong, I don't want to criticize your coding style. I'm just having problems with replacing such properties with new objects. The engine does even prevent this style for position, rotation and scale.

Another thing that concerns me: This approach might break renderer internal code if shadow objects are cached.

[Mugen87]

Another reasons why the engine should not promote this style: Users might assign wrong light shadow configurations (e.g. incompatible cameras) to lights. The default assignment is correct.

[WestLangley]

This code block changes the shadow camera helper, but not the shadow camera itself -- likely because the shadow camera is auto-sized each frame.

light.shadow.camera.fov = THREE.MathUtils.RAD2DEG * 2 * light.angle / 2; //  half as big as the default

So what is the recommended pattern to focus the shadow camera frustum -- both prior to rendering, and after rendering at least once?

[WestLangley]

20139 Has now broken my app.

Uncaught TypeError: THREE.LightShadow is not a constructor

I think we should continue to export these classes until we can develop an appropriate alternate API.

[Mugen87]

This code block changes the shadow camera helper, but not the shadow camera itself -- likely because the shadow camera is auto-sized each frame.

I would like to investigate this use case in more detail. Do you mind creating a live example that demonstrates the issue?

[mrdoob]

Reminds me to when we used to allow mesh.position = new THREE.Vector3() or mesh.position = mesh2.position.

[mrdoob]

@WestLangley

So what is the recommended pattern to focus the shadow camera frustum -- both prior to rendering, and after rendering at least once?

Yes, we desperately need something like light.shadow.contain( scene ) or something like that.

[Mugen87]

Instead of reverting #20139 we could also only add LightShadow back in the meanwhile. I just feel we should not expose LightShadow classes for the long-term.

[dmnsgn]

So as a broader discussion: what do you consider "exportable"?

In my opinion, any Three.js class, property or constants that can be assigned to another component of the API or are a default value of a component should be accessible to the end user. Use cases for Three.js are widely diverse (as seen from @WestLangley example above). These classes might be meant for internal use but many people need to go beyond the obvious documented usages and hack stuff with internals, which sometimes brings enhancement and contribution to the lib in return.

[mrdoob]

In my opinion, any Three.js class, property or constants that can be assigned to another component of the API or are a default value of a component should be accessible to the end user.

In this case, that property shouldn't have been re-assignable.

These classes might be meant for internal use but many people need to go beyond the obvious documented usages and hack stuff with internals, which sometimes brings enhancement and contribution to the lib in return.

That's okay. If there is a compelling use case for having these exposed we can expose them, but the only use case shared in this thread is a pattern we have had problems with in the past.

[WestLangley]

Please see https://github.com/mrdoob/three.js/pull/17228#issuecomment-524289339.

If there is a compelling use case for having these exposed we can expose them

There is a compelling use case for being able to focus the spotlight shadow frustum.

I know @Mugen87 is sensitive to breaking capabilities that have existed for years, so I would expect he would prefer to avoid breakage until an alternative API can be implemented.

[Mugen87]

Yeah, that's the reason why I have made #20147. It's not clear to me how many users will be affected by this removal (although I've never seen the explicit creation of light shadow instances so far in user code).

[WestLangley]

It's not clear to me how many users will be affected by this removal

At least 1.

The feature was supported by @mrdoob (https://github.com/mrdoob/three.js/issues/7690#issuecomment-202001593).

And the pattern I use was suggested by @mrdoob. (https://github.com/mrdoob/three.js/commit/ebf272066c910bb933930861dedb7731bdf9b657).

A better API would be preferable -- or perhaps you no longer want to support a user's ability to control the shadow camera FOV.

[RemusMar]

@WestLangley @Mugen87 @mrdoob

Actually, I use this pattern:

// setting a custom shadow frustum
light.shadow = new THREE.LightShadow( new THREE.PerspectiveCamera( 22.5, 1, 200, 700 ) );

The default frustum is unnecessarily oversized for my preference.

+1 (for many years).
I had to remove the THREE.LightShadow from my samples a couple of days ago.
And I didn't like that (Michael, the backwards compatibility !?!)
On top of that, the default frustum does not provide the best results in any scenario.
The bottom line: if LightShadow is not exported, an alternative API is a must.
My 2 cents.

[WestLangley]

ref: Restore LightShadow export for now #20171

[Mugen87]

I suggest to close this issue and create a new one which focuses on a replacement for exporting LightShadow. It seems the engine only needs to find out a way so users can actually set the fov value of a spot light's shadow camera. One possible solution is presented here:

https://github.com/mrdoob/three.js/pull/20147#issuecomment-679092113

[Mugen87]

Closing in favor of #20216.