Skip to content

Commit e3ea31a

Browse files
vidartfvidartf
committed
Refactor rendering / fix WebGLRenderer
The rendering calls were streamlined to avoid unneeded calls + fix cases where things were not called but should have.
1 parent de2ecba commit e3ea31a

File tree

6 files changed

+196
-253
lines changed

6 files changed

+196
-253
lines changed

‎js/src/_base/Preview.js

Lines changed: 55 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,47 @@ var BLACK = new THREE.Color('black');
1414

1515
var PreviewView = RenderableView.extend({
1616

17+
initialize: function() {
18+
RenderableView.prototype.initialize.apply(this, arguments);
19+
20+
this._rebuildNeeded = true;
21+
22+
},
23+
1724
render: function() {
1825
// ensure that model is fully initialized before attempting render
1926
return this.model.initPromise.bind(this).then(this.doRender);
2027
},
2128

29+
30+
setupEventListeners: function() {
31+
RenderableView.prototype.setupEventListeners.call(this);
32+
var child = this.model.get('child');
33+
this.listenTo(child, 'change', this.onChildChange.bind(this));
34+
if (child.obj instanceof THREE.Object3D) {
35+
// Since we use clone for objects, we need to rebuild for
36+
// any nested change instead of just rerendering.
37+
this.listenTo(child, 'childchange', this.onChildChange.bind(this));
38+
}
39+
},
40+
41+
onChildChange: function() {
42+
this._rebuildNeeded = true;
43+
},
44+
2245
constructScene: function() {
2346

2447
var obj = this.model.get('child').obj;
2548

2649
this.clearScene();
50+
// cameras need to be added to scene
2751
this.scene.add(this.camera);
2852

2953
if (obj instanceof THREE.Object3D) {
3054

3155
this.log('render Object3D');
32-
this.scene.add(obj);
56+
// Use a clone to not change `parent` attribute
57+
this.scene.add(obj.clone());
3358

3459
} else if (obj instanceof THREE.Geometry || obj instanceof THREE.BufferGeometry) {
3560

@@ -81,6 +106,9 @@ var PreviewView = RenderableView.extend({
81106

82107
}
83108

109+
// Clear at end to ensure that any changes to obj does not
110+
// cause infinite rebuild chain.
111+
this._rebuildNeeded = false;
84112
},
85113

86114
clearScene: function() {
@@ -90,37 +118,6 @@ var PreviewView = RenderableView.extend({
90118
}, this);
91119
},
92120

93-
update: function() {
94-
95-
RenderableView.prototype.update.apply(this, arguments);
96-
97-
if (this.model.get('child').obj) {
98-
this.constructScene();
99-
}
100-
this.renderScene();
101-
102-
},
103-
104-
renderScene: function() {
105-
this.log('renderScene');
106-
107-
// TODO: check renderer.domElement.isContextLost()
108-
109-
if (this.isFrozen) {
110-
this.log('renderScene->isFrozen');
111-
112-
this.acquireRenderer();
113-
this.updateSize();
114-
this.enableControls();
115-
116-
if (this.model.get('child').obj) {
117-
this.constructScene();
118-
}
119-
}
120-
121-
this.renderer.render(this.scene, this.camera);
122-
},
123-
124121
setupControls: function() {
125122
// Allow user to inspect object with mouse/scrollwheel
126123
this.log('setting up controls');
@@ -135,13 +132,12 @@ var PreviewView = RenderableView.extend({
135132
this.camera.position.set(-40, 40, 40);
136133
this.camera.lookAt(new THREE.Vector3(0,0,0));
137134

135+
// Update aspect ratio of camera:
138136
this.updateSize();
139137

140138
this.scene = new THREE.Scene();
141-
// cameras need to be added to scene
142-
this.scene.add(this.camera);
143139

144-
// Overrides clear color on renderer:
140+
// Overrides clear color of renderer:
145141
this.scene.background = BLACK;
146142

147143
// Lights
@@ -155,18 +151,32 @@ var PreviewView = RenderableView.extend({
155151
this.setupControls();
156152
this.enableControls();
157153

158-
if (this.model.get('child').obj) {
154+
this.renderScene();
155+
},
156+
157+
renderScene: function() {
158+
this.log('renderScene');
159+
160+
// TODO: check renderer.domElement.isContextLost()
161+
162+
if (this.isFrozen) {
163+
this.unfreeze();
164+
}
165+
if (this._rebuildNeeded) {
159166
this.constructScene();
160-
this.renderScene();
161167
}
168+
169+
this.renderer.render(this.scene, this.camera);
162170
},
163171

164172
updateSize: function() {
165173
RenderableView.prototype.updateSize.call(this);
166-
var width = this.model.get('_width');
167-
var height = this.model.get('_height');
168-
this.camera.aspect = width / height;
169-
this.camera.updateProjectionMatrix();
174+
if (this.camera) {
175+
var width = this.model.get('_width');
176+
var height = this.model.get('_height');
177+
this.camera.aspect = width / height;
178+
this.camera.updateProjectionMatrix();
179+
}
170180
},
171181

172182
});
@@ -188,19 +198,19 @@ var PreviewModel = RenderableModel.extend({
188198
initialize: function(attributes, options) {
189199
RenderableModel.prototype.initialize.apply(this, arguments);
190200

201+
// Don't listen to child until it is finished it's setup
191202
this.initPromise = this.get('child').initPromise.bind(this).then(function() {
192203
this.setupListeners();
193204
});
194205
},
195206

196207
setupListeners: function() {
197208
var child = this.get('child');
198-
this.listenTo(child, 'change', this.onChange.bind(this));
199-
this.listenTo(child, 'childchange', this.onChange.bind(this));
200-
209+
this.listenTo(child, 'change', this.onChildChange.bind(this));
210+
this.listenTo(child, 'childchange', this.onChildChange.bind(this));
201211
},
202212

203-
onChange: function(model, options) {
213+
onChildChange: function(model, options) {
204214
this.trigger('rerender', this, {});
205215
},
206216

‎js/src/_base/Renderable.js

Lines changed: 60 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,9 @@ var RenderableView = widgets.DOMWidgetView.extend({
3636
remove: function() {
3737
widgets.DOMWidgetView.prototype.remove.apply(this, arguments);
3838

39+
this.$el.empty();
3940
if (!this.isFrozen) {
40-
this.isFrozen = true;
41-
RendererPool.release(this.renderer);
42-
this.renderer = null;
41+
this.teardownViewer();
4342
}
4443
},
4544

@@ -50,23 +49,19 @@ var RenderableView = widgets.DOMWidgetView.extend({
5049
doRender: function() {
5150
this.el.className = "jupyter-widget jupyter-threejs";
5251

53-
this.acquireRenderer();
52+
this.unfreeze();
5453

5554
this.lazyRendererSetup();
5655

5756
this.setupEventListeners();
5857
},
5958

6059
setupEventListeners: function() {
61-
this.on('destroy', this.destroy, this);
62-
this.listenTo(this.model, 'rerender', this.tick.bind(this));
60+
this.listenTo(this.model, 'rerender', this.update.bind(this));
6361
this.listenTo(this.model, 'msg:custom', this.onCustomMessage.bind(this));
6462

6563
this.listenTo(this.model, 'change:_width', this.updateSize.bind(this));
6664
this.listenTo(this.model, 'change:_height', this.updateSize.bind(this));
67-
// Redraw after a size change. This is bound after updateSize (significant).
68-
this.listenTo(this.model, 'change:_width', this.tick.bind(this));
69-
this.listenTo(this.model, 'change:_height', this.tick.bind(this));
7065
},
7166

7267
tick: function() {
@@ -84,77 +79,65 @@ var RenderableView = widgets.DOMWidgetView.extend({
8479
}
8580
},
8681

87-
destroy: function() {
88-
this.$el.empty();
89-
if (!this.isFrozen) {
90-
this.teardownViewer();
91-
}
92-
},
93-
9482
updateSize: function() {
9583
var width = this.model.get('_width');
9684
var height = this.model.get('_height');
97-
this.renderer.setSize(width, height);
85+
if (this.isFrozen) {
86+
// Set size of frozen element
87+
this.$frozenRenderer.width(width).height(height);
88+
} else {
89+
this.renderer.setSize(width, height);
90+
}
9891
},
9992

100-
renderScene: function() {
93+
renderScene: function(scene, camera) {
10194
this.log('renderScene');
10295

96+
scene = scene || this.scene;
97+
camera = camera || this.camera;
98+
10399
// TODO: check renderer.domElement.isContextLost()
104100

105101
if (this.isFrozen) {
106-
this.log('renderScene->isFrozen');
107-
108-
this.acquireRenderer();
109-
this.updateSize();
102+
this.unfreeze();
110103
}
111104

112-
this.renderer.render(this.scene, this.camera);
113-
},
114-
115-
teardownViewer: function() {
116-
117-
this.$renderer.off('mouseenter');
118-
this.$renderer.off('mouseleave');
119-
120-
this.isFrozen = true;
121-
RendererPool.release(this.renderer);
122-
123-
this.$renderer = null;
124-
this.renderer = null;
125-
126-
this.disableControls();
127-
128-
this.$el.css('margin-bottom', 'auto');
129-
105+
this.renderer.render(scene, camera);
130106
},
131107

132-
acquireRenderer: function() {
108+
unfreeze: function() {
133109
if (!this.isFrozen) {
134110
return;
135111
}
136-
this.isFrozen = false;
112+
this.log('unfreeze');
137113

138-
this.log('ThreeView.acquiring...');
114+
this.isFrozen = false;
139115

140116
if(this.$frozenRenderer) {
141117
this.$frozenRenderer.off('mouseenter');
142118
this.$frozenRenderer = null;
143119
}
144120

121+
this.acquireRenderer();
122+
this.updateSize();
123+
124+
if (this.controls) {
125+
this.enableControls();
126+
}
127+
},
128+
129+
acquireRenderer: function() {
130+
131+
this.log('ThreeView.acquiring...');
132+
145133
this.renderer = RendererPool.acquire(this.onRendererReclaimed.bind(this));
146-
this.renderer.setSize(this.model.get('_width'), this.model.get('_height'));
147134

148135
this.$renderer = $(this.renderer.domElement);
149136
this.$el.empty().append(this.$renderer);
150137

151138
this.$el.css('margin-bottom', '-5px');
152139

153140
this.log('ThreeView.acquireRenderer(' + this.renderer.poolId + ')');
154-
155-
if (this.controls) {
156-
this.enableControls();
157-
}
158141
},
159142

160143
freeze: function() {
@@ -169,12 +152,36 @@ var RenderableView = widgets.DOMWidgetView.extend({
169152
this.$el.empty().append('<img src="' + this.renderer.domElement.toDataURL() + '" />');
170153

171154
this.teardownViewer();
172-
173155
this.$frozenRenderer = this.$el.find('img');
174-
this.$frozenRenderer.on('mouseenter', _.bind(function() {
175-
this.log('frozenRenderer.mouseenter');
176-
this.tick(); // renderer will be acquired by renderScene
177-
}, this));
156+
157+
// Ensure the image gets set the right size:
158+
this.updateSize();
159+
160+
if (this.controls) {
161+
this.$frozenRenderer.on('mouseenter', _.bind(function() {
162+
this.log('frozenRenderer.mouseenter');
163+
this.tick(); // renderer will be acquired by renderScene
164+
}, this));
165+
}
166+
167+
},
168+
169+
teardownViewer: function() {
170+
171+
this.$renderer.off('mouseenter');
172+
this.$renderer.off('mouseleave');
173+
174+
this.isFrozen = true;
175+
RendererPool.release(this.renderer);
176+
177+
this.$renderer = null;
178+
this.renderer = null;
179+
180+
if (this.controls) {
181+
this.disableControls();
182+
}
183+
184+
this.$el.css('margin-bottom', 'auto');
178185

179186
},
180187

‎js/src/_base/Three.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ var ThreeModel = widgets.WidgetModel.extend({
299299

300300
onChildChanged: function(model, options) {
301301
console.log('child changed: ' + model.id);
302+
// Propagate up hierarchy:
302303
this.trigger('childchange', this);
303304
},
304305

0 commit comments

Comments
 (0)