Base color for 3D cube #7354
Base color for 3D cube #7354
Conversation
| ]; | ||
| const materials: THREE.Material[] = []; | ||
|
|
||
| for (let i = 0; i < 6; i++) { |
There was a problem hiding this comment.
Good idea to use a loop!
In JS, there's a more elegant way to do this, with a map.
So you could write:
const materials = new Array(6).fill(0).map((_, index) => {
return material;
})
| const basicMaterial: THREE.MeshBasicMaterial = new THREE.MeshBasicMaterial(); | ||
| basicMaterial.copy(material); | ||
| materials.push( | ||
| basicMaterial.map | ||
| ? getFaceMaterial(runtimeObject, materialIndexToFaceIndex[i]) | ||
| : new THREE.MeshBasicMaterial({ vertexColors: true }) | ||
| ); |
There was a problem hiding this comment.
This seems unnecessary to me.
I feel like you could just do:
const material = ...
materials.push(material)since getFaceMaterial already returns a MeshBasicMaterial if no resource
| let colors: number[] = []; | ||
| for (let i = 0; i < geometry.attributes.position.count; i++) { | ||
| colors.push( | ||
| runtimeObject._color.r, | ||
| runtimeObject._color.g, | ||
| runtimeObject._color.b | ||
| ); | ||
| } | ||
|
|
||
| geometry.setAttribute( | ||
| 'color', | ||
| new THREE.BufferAttribute(new Float32Array(colors), 3) | ||
| ); |
There was a problem hiding this comment.
I see MeshBasicMaterial as a color property.
It doesn't work with this property only? So that is wouldn't impact the geometry attribute?
| @@ -902,6 +903,12 @@ module.exports = { | |||
| .setLabel(_('Depth')) | |||
| .setMeasurementUnit(gd.MeasurementUnit.getPixel()) | |||
| .setGroup(_('Default size')); | |||
| objectProperties | |||
| .getOrCreate('cubeColor') | |||
There was a problem hiding this comment.
I think you can name it simply color, we can deduce from the context that it's for the cube
| @@ -902,6 +903,12 @@ module.exports = { | |||
| .setLabel(_('Depth')) | |||
| .setMeasurementUnit(gd.MeasurementUnit.getPixel()) | |||
| .setGroup(_('Default size')); | |||
| objectProperties | |||
| .getOrCreate('cubeColor') | |||
| .setValue(objectContent.color || (255, 255, 255)) | |||
There was a problem hiding this comment.
Check for similar attributes (such as fillColor in the text input), how it's done
| this._faceResourceNames[faceIndex] = resourceName; | ||
| this._renderer.updateFace(faceIndex); | ||
| } | ||
| setCubeColor(color: string): void { | ||
| if (rgbOrHexToRGBColor(color) === this._color) return; |
There was a problem hiding this comment.
Welcome to JS! Can you enter [1, 2, 3] === [1, 2, 3] in your browser JS console and see the result?
There was a problem hiding this comment.
Or you could use rgbOrHexStringToNumber instead. And when you need the 3 components, you can use hexNumberToRGBArray
There was a problem hiding this comment.
I think that rgbOrHexStringToNumber is better to use: this way, we only store one number, and not an array, which is more efficient memory-wise
| this._faceResourceNames[faceIndex] = resourceName; | ||
| this._renderer.updateFace(faceIndex); | ||
| } | ||
| setCubeColor(color: string): void { | ||
| if (rgbOrHexToRGBColor(color) === this._color) return; | ||
| this._color = rgbOrHexToRGBColor(color); |
There was a problem hiding this comment.
You do the operation twice, you could store the result in a variable
Added support for color in 3DCubeRuntimeObject and renderer,
added prototype of a property to control it;
The color renders only in preview mode, not in editor yet