Circular dependency in WebGLCubeRenderTarget.fromEquirectangularTexture

[looeee]

The last circular dependency in the library is:
WebGLCubeRenderTarget -> CubeCamera -> WebGLCubeRenderTarget

This is caused by WebGLCubeRenderTarget.fromEquirectangularTexture, which was added in #16671 following discussion in #15331 (comment).

This is the last blocker for closing #6241.

To remove this, we could extract .fromEquirectangularTexture to an external utility (or maybe move it to CubeTexture?). I'm not sure if it's worth doing so just to avoid a circular dependency though.

@WestLangley, @Mugen87 and @mrdoob - you guys were involved in adding this method. Do you think we need to fix this circular dependency? If not, then we can probably just mark as "won't fix" and close #6241.

[Mugen87]

Do you think we need to fix this circular dependency?

As far as I understand, this circular dependency is not harmful. However, circular dependency is always an indicator that something with the class design is not right. This issue can only be solved by refactoring.

I've already though about this in the past and my preferred option is to inject the internal render target of CubeCamera to solve this issue. So instead of:

var camera = new THREE.CubeCamera( 1, 1000, 256, { format: THREE.RGBAFormat, magFilter: THREE.LinearFilter, minFilter: THREE.LinearFilter } );

we do this:

var renderTarget = new THREE.WebGLCubeRenderTarget( 256, { format: THREE.RGBAFormat, magFilter: THREE.LinearFilter, minFilter: THREE.LinearFilter }  );
var camera = new THREE.CubeCamera( 1, 1000, renderTarget );

I like latter one since the last two parameters of CubeCamera are obviously intended for WebGLCubeRenderTarget. And since users usually use cubeCamera.renderTarget to access the internal render target, they could instead refer to the variable created on app level (so just renderTarget).

[looeee]

Yeah, that looks better to me. Passing in parameters intended for the render target to the CubeCamera constructor doesn't feel right.

[Mugen87]

If @mrdoob approves (and consider no other objections), I can make a PR with the change.

[Mugen87]

I've already made the PR so it's easier to see the changes.

[mrdoob]

@Mugen87 Thank you! I'll review this week.

[Mugen87]

@mrdoob /bump