Skip to content

Commit e529132

Browse files
committed
Fix race condition in gltf-model when changing src while loading
1 parent 170b7a0 commit e529132

File tree

2 files changed

+87
-3
lines changed

2 files changed

+87
-3
lines changed

src/components/gltf-model.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@ export var Component = registerComponent('gltf-model', {
3636
var el = this.el;
3737
var src = this.data;
3838

39-
if (!src) { return; }
40-
4139
this.remove();
4240

41+
if (!src) { return; }
42+
4343
this.ready.then(function () {
4444
self.loader.load(src, function gltfLoaded (gltfModel) {
45+
if (src !== self.data) { return; }
4546
self.model = gltfModel.scene || gltfModel.scenes[0];
4647
self.model.animations = gltfModel.animations;
4748

@@ -58,5 +59,6 @@ export var Component = registerComponent('gltf-model', {
5859
remove: function () {
5960
if (!this.model) { return; }
6061
this.el.removeObject3D('mesh');
62+
this.model = null;
6163
}
6264
});

tests/components/gltf-model.test.js

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,62 @@ suite('gltf-model', function () {
6060
el.sceneEl.appendChild(el2);
6161
});
6262

63+
test('can change model after first model has loaded', function (done) {
64+
var el = this.el;
65+
el.addEventListener('model-loaded', function () {
66+
assert.ok(el.getObject3D('mesh'));
67+
68+
el.addEventListener('model-loaded', function () {
69+
assert.ok(el.getObject3D('mesh'));
70+
done();
71+
}, {once: true});
72+
el.setAttribute('gltf-model', `url(${SRC_NO_DEFAULT_SCENE})`);
73+
74+
// While loading, model is already removed from scenegraph
75+
assert.notOk(el.getObject3D('mesh'));
76+
}, {once: true});
77+
el.setAttribute('gltf-model', '#gltf');
78+
});
79+
80+
test('can change model while first model is loading', function (done) {
81+
var el = this.el;
82+
var firstGlbMock = { scene: new THREE.Object3D() };
83+
var secondGlbMock = { scene: new THREE.Object3D() };
84+
85+
el.addEventListener('componentinitialized', function (event) {
86+
if (event.detail.name !== 'gltf-model') {
87+
return;
88+
}
89+
90+
// Mock loader to only complete loading the first model after the second model finished loading
91+
var resolveSecondModel;
92+
var promise = new Promise((resolve) => {
93+
resolveSecondModel = resolve;
94+
});
95+
var loader = el.components['gltf-model'].loader;
96+
loader.load = function (url, onLoad) {
97+
if (url === 'first.glb') {
98+
promise.then(() => {
99+
onLoad(firstGlbMock);
100+
allLoaded();
101+
});
102+
} else if (url === 'second.glb') {
103+
onLoad(secondGlbMock);
104+
resolveSecondModel();
105+
}
106+
};
107+
});
108+
109+
// Wait for both models to be loaded
110+
function allLoaded () {
111+
assert.equal(el.components['gltf-model'].model, secondGlbMock.scene);
112+
done();
113+
}
114+
115+
el.setAttribute('gltf-model', 'first.glb');
116+
el.setAttribute('gltf-model', 'second.glb');
117+
});
118+
63119
test('attaches animation clips to model', function (done) {
64120
var el = this.el;
65121
var sceneMock = new THREE.Object3D();
@@ -82,7 +138,6 @@ suite('gltf-model', function () {
82138
loader.load = function (url, onLoad) {
83139
setTimeout(onLoad.bind(null, gltfMock));
84140
};
85-
loader.setDRACOLoader = function () {};
86141

87142
// Start loading model
88143
el.setAttribute('gltf-model', '#gltf');
@@ -106,4 +161,31 @@ suite('gltf-model', function () {
106161
});
107162
el.setAttribute('gltf-model', `url(${SRC_NO_DEFAULT_SCENE})`);
108163
});
164+
165+
test('removes model when src is emptied', function (done) {
166+
var el = this.el;
167+
el.addEventListener('model-loaded', function () {
168+
assert.ok(el.components['gltf-model'].model);
169+
assert.equal(el.components['gltf-model'].model, el.getObject3D('mesh'));
170+
171+
el.setAttribute('gltf-model', '');
172+
assert.notOk(el.components['gltf-model'].model);
173+
assert.notOk(el.getObject3D('mesh'));
174+
done();
175+
});
176+
el.setAttribute('gltf-model', '#gltf');
177+
});
178+
179+
test('removes model when component is removed', function (done) {
180+
var el = this.el;
181+
el.addEventListener('model-loaded', function () {
182+
assert.ok(el.components['gltf-model'].model);
183+
assert.equal(el.components['gltf-model'].model, el.getObject3D('mesh'));
184+
185+
el.removeAttribute('gltf-model');
186+
assert.notOk(el.getObject3D('mesh'));
187+
done();
188+
});
189+
el.setAttribute('gltf-model', '#gltf');
190+
});
109191
});

0 commit comments

Comments
 (0)