Famous / engine

MIT License
1.75k stars 250 forks source link

Refactor Mesh#onUpdate/ Color component #435

Open alexanderGugel opened 9 years ago

alexanderGugel commented 9 years ago

The Color component in utilities/Color.js has an opacity property that is being managed using a Transitionable (just like the RGB values).

Aside from that, the Mesh itself - more or less - has a dependency on the Color component (or at least requires it to expose an isActive method), therefore the two concepts are not really decoupled.

I propose the following changes:

  1. Remove Color component
  2. Store color simply as a Mesh property
  3. Don't requestUpdate in Mesh when a component is active (this violates the current abstraction - the Color component should do that) - see Mesh#onUpdate
Mesh.prototype.onUpdate = function onUpdate() {
    // ...

    if (node) {

        // ...

        // If any invalidations exist, push them into the queue
        if (this.value.color && this.value.color.isActive()) {
            // ...
            this._node.requestUpdateOnNextTick(this._id);
        }
        if (this.value.glossiness && this.value.glossiness[0] && this.value.glossiness[0].isActive()) {
            // ...
            this._node.requestUpdateOnNextTick(this._id);
        }
        else {
            // ...
        }

        // If any invalidations exist, push them into the queue
        // ...
    }
};
alexanderGugel commented 9 years ago

@michaelobriena

jd-carroll commented 9 years ago

So I might be asking dumb questions here...

alexanderGugel commented 9 years ago

@jd-carroll The Color component in a way violates the existing abstractions in the platform. The Color component is not a component. Components define behavior on Nodes, but the color component is only being used for defining the base color of meshes etc.

The color component can only be added to a Mesh, not to a Node.

Can an individual color component be tied to an individual DOMElement?

No. The Color component is only being used for GL.

michaelobriena commented 9 years ago

Color is not a component. I don't think it was ever intended to be. It is just Transitionables for tweening colors.

If we roll the functionality into Mesh I am ok with this. But for solid base colors it is super nice to tween them between values.