feat: adds customizable colors to CameraHelper#23829
Conversation
| const colorFrustum = new Color( 0xffaa00 ); | ||
| const colorCone = new Color( 0xff0000 ); | ||
| const colorUp = new Color( 0x00aaff ); | ||
| const colorTarget = new Color( 0xffffff ); | ||
| const colorCross = new Color( 0x333333 ); | ||
| const _colorFrustum = new Color( colorFrustum ); | ||
| const _colorCone = new Color( colorCone ); | ||
| const _colorUp = new Color( colorUp ); | ||
| const _colorTarget = new Color( colorTarget ); | ||
| const _colorCross = new Color( colorCross ); |
There was a problem hiding this comment.
This naming convention here is ugly, but I'd rather the arguments have the nice names - as they are user-facing
| class CameraHelper extends LineSegments { | ||
|
|
||
| constructor( camera ) { | ||
| constructor( camera, colorFrustum = 0xffaa00, colorCone = 0xff0000, colorUp = 0x00aaff, colorTarget = 0xffffff, colorCross = 0x333333 ) { |
There was a problem hiding this comment.
An alternative API would be passing an object with all colors as
{
frustum: 0xff0000,
cone: 0xff00ff,
// ...etc
}But I can't recall objects that also use this pattern, so this felt more appropriate
There was a problem hiding this comment.
I think that the object is a better approach. And it will also solve the naming problem.
|
How about... const helper = new THREE.CameraHelper( camera ).setColors( frustum, cone, up, target, cross ); |
|
@mrdoob that API would be nice indeed, but it would require require rebuilding the vertices and colors attributes OR a possibly signifcant refactor of the class to just update the colors buffer - moving the two buffers creation to a method. Think it's worth pursuing? 🤔 |
|
In the past we have agreed to no introduce larger ctor signatures nor
|
|
I'll give that a shot 👍 |
|
I think it's more clear if |
|
Closing in favor of #24235. |
4 participants
Found myself needing to color-code camera helpers and wanting to use toned-down colors in other occasions.
This PR wouldn't let users change the colors after construction, but it could be a good middle ground - as helpers are cheap to rebuild anyway
Usage:
TODO