Entity Color Options #1005
Entity Color Options #1005
Conversation
| // Save the original material color values | ||
| const newMaterials = getMaterials(entity.object3D); | ||
| newMaterials.forEach((material) => { | ||
| material.userData.origColor = material.color.clone(); |
There was a problem hiding this comment.
This should be done in the aframe component, not here. There is probably an issue here, it will override material.userData.origColor with the current customized color if you select another car and back this car because the callback will be recreated.
There was a problem hiding this comment.
That makes sense. I changed it to be initialized once whenever the component is added. Since this presents an issue if the model is changed, I will make it so that changing the model will remove the custom-colors component. This makes most sense, as most models will have unique materials
| mapping[mat] = color; | ||
| }); | ||
| return mapping; | ||
| }, [customColorData]); |
There was a problem hiding this comment.
You can remove all this and use
const baseColorMapping = entity.getAttribute('custom-colors') ?? {}after you redefined the custom-colors schema to use the custom parse/stringify.
|
|
||
| const newColorsString = Object.entries(newColorMapping) | ||
| .map(([mat, color]) => `${mat}:${color}`) | ||
| .join(';'); |
There was a problem hiding this comment.
You don't need to serialize yourself, just give the object to execute.
| } else { | ||
| entity.addEventListener('model-loaded', () => { | ||
| updateMaterials(); | ||
| }); |
There was a problem hiding this comment.
You can add {once: true} for this listener.
src/components/custom-colors.js
Outdated
| } else { | ||
| this.el.addEventListener('model-loaded', () => { | ||
| this.update(); | ||
| }); |
There was a problem hiding this comment.
Properly unregister this model-loaded listener in remove.
updateMaterials() {
this.update():
},
init() {
this.updateMaterials = this.updateMaterials.bind(this);
if (this.el.getObject3D('mesh')) {
this.update();
} else {
this.el.addEventListener('model-loaded', this.updateMaterials);
}
},
remove() {
this.el.removeEventListener('model-loaded', this.updateMaterials);
...
}There was a problem hiding this comment.
Move the this.updateMaterials = this.updateMaterials.bind(this); at the beginning. Listening to model-loaded shouldn't be needed anymore if you listen on object3dset. object3dset is fired just before model-loaded https://github.com/aframevr/aframe/blob/cc43ea67f8170d8ca7e5bdcbaade031df103801e/src/components/gltf-model.js#L48-L49
|
The new commits address the feedback and enable the coverage of the case when a user changes the model parameter of an entity that has the custom color component. Initially I was thinking to just remove the custom color component, but then I think it makes more sense to keep it if possible. There was a problem with the timing of The handling of the original colors is moved to the component instead |
|
The new PR for assets-dist here adds the named materials for the entities in |
|
hey @ewei2406 finally getting a chance to review this, it's really fun and useful, congrats! here's some feedback assets PR feedback (3DStreet/3dstreet-assets-dist#38)
for this pr:
next step:
|
from vincent's feedback
|
Hey Kieran, thanks for the feedback!
|
|
@ewei2406 yes it is good to add original blender files here: This is not an urgent item so please feel free to work on this as you have time It sounds like we could convert the entirety of the 3d models to vertex colors instead of atlas / material? That'd potentially be a lot of work across all the 3d models, so not asking you to do this, just getting an idea of what might be a better long-term structure. |
[Revisal of #992]
Following the discussion, I implemented a widget to customize the colors of materials in each entity.
There is a button to add custom colors for an entity.
Once added, the user can pick which materials to modify. Multiple materials can be modified at the same time.
The data is serialized in the format:
<material_name>:<overriden_color>;<material_name>:<overriden_color>;.... The component is not added by default - only when the customize button is clicked will it be created.