Famous / engine

MIT License
1.75k stars 250 forks source link

Mesh#onDismount is broken #436

Open alexanderGugel opened 9 years ago

alexanderGugel commented 9 years ago

Steps to reproduce

  1. Create a node
  2. Mount the node
  3. Add a Mesh to the node
  4. Dismount the node via removeChild on parent

    Example (self-contained)

'use strict';

var DOMElement = require('famous/dom-renderables/DOMElement');
var FamousEngine = require('famous/core/FamousEngine');
var Mesh = require('famous/webgl-renderables/Mesh');

FamousEngine.init();

var scene = FamousEngine.createScene();

var parent = scene.addChild();
var child = parent.addChild();

var mesh = new Mesh(parent);
mesh.setGeometry('Sphere');

setTimeout(function () {
    scene.removeChild(parent);
}, 1000);

var domElement = new DOMElement(child, {
    properties: {
        background: 'blue'
    }
});

parent
    .setSizeMode('absolute', 'absolute', 'absolute')
    .setAbsoluteSize(250, 250, 250)
    .setAlign(0.5, 0.5)
    .setMountPoint(0.5, 0.5)
    .setOrigin(0.5, 0.5);

var spinner = parent.addComponent({
    onUpdate: function(time) {
        parent.setRotation(time / 1000, time / 2000, time / 3000);
        parent.requestUpdateOnNextTick(spinner);
    }
});

parent.requestUpdate(spinner);

Relevant code sample

setTimeout(function () {
    scene.removeChild(parent);
}, 1000);

Error

Uncaught TypeError: Cannot read property 'split' of null

Cause

The commands sent by Mesh#onDismount are incorrect. They don't include a node location/ path. Therefore the compositor can't split the command.

/**
 * Queues the command for dismounting Mesh
 *
 * @method
 *
 * @return {undefined} undefined
 */
Mesh.prototype.onDismount = function onDismount () {
    this._initialized = false;
    this._changeQueue.push(Commands.GL_REMOVE_MESH);

    this._requestUpdate();
};
alexanderGugel commented 9 years ago

There is a similar problem with Mesh#onOpacityChange, Mesh#onSizeChange, Mesh#onTransformChange, Mesh#onHide, Mesh#onShow.

Basically, the issue is code like this:

Mesh.prototype.onShow = function onShow () {
    this._changeQueue.push(Commands.GL_MESH_VISIBILITY, true);

    this._requestUpdate();
};

We can't simply append the command to the change queue at this point. The WITH commands hasn't been sent at this point. Therefore the commands won't be interpreted correctly by the Compositor.

CompSciFutures commented 9 years ago

Bug #439 might also be relevant.