Helpers: Ensure light helpers are not frame-late.

[Mugen87]

trafficstars

Related issue: Fixed #14801.

Description

Ensures the world matrices of lights and targets are up-to-date when evaluated in light helpers.

[Mugen87]

Just want to clarify that this issue can't be fixed by overwriting updateMatrixWorld() in the helpers. Doing this does not guarantee that the world matrix of the reference object (the light) is up-to-date.

[mrdoob]

Just want to clarify that this issue can't be fixed by overwriting updateMatrixWorld() in the helpers. Doing this does not guarantee that the world matrix of the reference object (the light) is up-to-date.

Hmm, in what case the reference object's wold matrix won't be up to date by the time the helper's updateMatrixWorld() runs?

[Mugen87]

Consider this hierarchy:

Scene - Helper
      - Light

And assume scene.updateMatrixWorld() is called. When the helper's world matrix is updated first, it will use an outdated version of the light's world matrix.

And even when the helpers call light.updateMatrixWorld(), this method will probably not update a light's world matrix correctly since it does not update the matrices of the ancestor 3D objects.

[acu192]

This is EXACTLY the PR I was planning to send in when I wrote issue #21608. Nice work!

Well, one difference actually: I would do the same change for PointLightHelper, address this line: https://github.com/mrdoob/three.js/blob/5f8443265024b831a9de5a5177bc5aed4f4093ee/src/helpers/PointLightHelper.js#L15

[acu192]

PS: This is a big problem being addressed here. It's easy to create a light helper that shows you the wrong thing, in which case it does the opposite of helping (it hurts you).

The main fix here is calling this.light.target.updateWorldMatrix(...) in the same places that you call this.light.updateWorldMatrix(...). Consistency in updating the light's matrix and target's matrix is kind of important. :) I ran into this bug in my project and it seems (based on the other issued filed) others have as well.

Granted, the long-term solution is to remove targets from lights (and use the light's quaternion instead), but until that solution is in-place, we need to address the current bug. This PR fixes the current bug and does not add any breaking changes (at least, not that I can imagine).

[Mugen87]

I've updated PointLightHelper as requested!

[acu192]

@mrdoob Do you think we get this in the r128 release?

[Mugen87]

BTW: This is what users do in order to desperately make light helpers work:

const lightHelper = new THREE.DirectionalLightHelper(light, 10);
lightHelper.name = 'lightHelper';
scene.add(lightHelper);
lightHelper.parent.updateMatrixWorld();
lightHelper.update();

From: https://discourse.threejs.org/t/directionallight-shadow-camera-helper-pointing-in-the-wrong-direction/27392

With this PR, the user could just add the helper and things work out-of-the-box. Would love to see this get merged in r130 😇 .

[acu192]

@mrdoob Anything we can do to help you feel comfortable with this?

Excited to get it in r136. 😉