Skip to content

Commit e4d538d

Browse files
joannacirillojoannacirillo
committed
refactor(fullscreenrenderwindow, dataarray, remoteview, macro): refactor constructors
Refactor of fullscreenrenderwindow constructor so that the setXXX() methods of correesponding properties are called when the fullScreenRenderWindow is instanciated (calling newInstance). In the macro.js newInstance() method, the set() method is called in order that the setXXX() methods for each entry of initial values is called if such methods exist. This is in order that side effects of setters are taken into account when creating new objects. Setters of both default and initial values are called Refactor of DataArray and child classes. The method "setData" is called so that the parameters are synchronised. Tests have been added and updates. A new test for macro protected properties have been added. BREAKING CHANGE: If a custom handling is done on the parameters in the extend function, the changes made might be overwritten by the call of the set function in newInstace. If it is not what is intended, those properties have to be removed from initialValues.
1 parent 230092f commit e4d538d

File tree

9 files changed

+198
-56
lines changed

9 files changed

+198
-56
lines changed

‎Sources/Common/Core/CellArray/index.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ function vtkCellArray(publicAPI, model) {
8888

8989
function defaultValues(initialValues) {
9090
return {
91+
// empty is only here to be passed to the DataArray extend function in order
92+
// to create a cellArray without giving values
9193
empty: true,
9294
numberOfComponents: 1,
9395
dataType: VtkDataTypes.UNSIGNED_INT,
@@ -98,7 +100,8 @@ function defaultValues(initialValues) {
98100
// ----------------------------------------------------------------------------
99101

100102
export function extend(publicAPI, model, initialValues = {}) {
101-
vtkDataArray.extend(publicAPI, model, defaultValues(initialValues));
103+
Object.assign(initialValues, defaultValues(initialValues));
104+
vtkDataArray.extend(publicAPI, model, initialValues);
102105
vtkCellArray(publicAPI, model);
103106
}
104107

‎Sources/Common/Core/DataArray/index.js‎

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,14 @@ function vtkDataArray(publicAPI, model) {
262262
};
263263

264264
publicAPI.setData = (typedArray, numberOfComponents) => {
265+
if (Array.isArray(typedArray)) {
266+
// eslint-disable-next-line no-param-reassign
267+
typedArray = macro.newTypedArrayFrom(model.dataType, typedArray);
268+
}
265269
model.values = typedArray;
266270
model.size = typedArray.length;
267271
model.dataType = getDataType(typedArray);
272+
268273
if (numberOfComponents) {
269274
model.numberOfComponents = numberOfComponents;
270275
}
@@ -313,44 +318,55 @@ function vtkDataArray(publicAPI, model) {
313318
// Object factory
314319
// ----------------------------------------------------------------------------
315320

316-
const DEFAULT_VALUES = {
317-
name: '',
318-
numberOfComponents: 1,
319-
size: 0,
320-
dataType: DefaultDataType,
321-
rangeTuple: [0, 0],
322-
// values: null,
323-
// ranges: null,
324-
};
321+
function defaultValues(initialValues) {
322+
return {
323+
name: '',
324+
numberOfComponents: 1,
325+
size: 0,
326+
dataType: DefaultDataType,
327+
rangeTuple: [0, 0],
328+
// values: macro.newTypedArray(DefaultValues),
329+
// ranges: null,
330+
...initialValues,
331+
};
332+
}
325333

326334
// ----------------------------------------------------------------------------
327335

328336
export function extend(publicAPI, model, initialValues = {}) {
329-
Object.assign(model, DEFAULT_VALUES, initialValues);
330-
331-
if (!model.empty && !model.values && !model.size) {
337+
if (initialValues.empty === false && !initialValues.values) {
332338
throw new TypeError(
333339
'Cannot create vtkDataArray object without: size > 0, values'
334340
);
335341
}
336-
337-
if (!model.values) {
338-
model.values = macro.newTypedArray(model.dataType, model.size);
339-
} else if (Array.isArray(model.values)) {
340-
model.values = macro.newTypedArrayFrom(model.dataType, model.values);
341-
}
342-
343-
if (model.values) {
344-
model.size = model.values.length;
345-
model.dataType = getDataType(model.values);
346-
}
342+
Object.assign(initialValues, defaultValues(initialValues));
347343

348344
// Object methods
349345
macro.obj(publicAPI, model);
350346
macro.set(publicAPI, model, ['name', 'numberOfComponents']);
351347

348+
model.dataType = initialValues.dataType;
349+
if (!initialValues.values) {
350+
initialValues.values = macro.newTypedArray(
351+
model.dataType,
352+
initialValues.size
353+
);
354+
} else if (Array.isArray(initialValues.values)) {
355+
initialValues.values = macro.newTypedArrayFrom(
356+
model.dataType,
357+
initialValues.values
358+
);
359+
}
360+
352361
// Object specific methods
353362
vtkDataArray(publicAPI, model);
363+
publicAPI.setData(initialValues.values, initialValues.numberOfComponents);
364+
delete model.empty;
365+
delete initialValues.values;
366+
delete initialValues.empty;
367+
delete initialValues.numberOfComponents;
368+
delete initialValues.size;
369+
delete initialValues.dataType;
354370
}
355371

356372
// ----------------------------------------------------------------------------

‎Sources/Common/Core/DataArray/test/testDataArray.js‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,113 @@
11
import test from 'tape-catch';
22
import vtkDataArray from 'vtk.js/Sources/Common/Core/DataArray';
3+
import Constants from 'vtk.js/Sources/Common/Core/DataArray/Constants';
4+
import * as macro from 'vtk.js/Sources/macros';
5+
6+
const { DefaultDataType } = Constants;
7+
8+
function getDataArrayProperties(dataArray) {
9+
return {
10+
size: dataArray.get().size,
11+
numberOfComponents: dataArray.get().numberOfComponents,
12+
dataType: dataArray.get().dataType,
13+
values: dataArray.get().values,
14+
};
15+
}
316

417
test('Test vtkDataArray instance', (t) => {
518
t.ok(vtkDataArray, 'Make sure the class definition exists');
619
const instance = vtkDataArray.newInstance({ size: 256 });
720
t.ok(instance);
21+
22+
const dataArray1 = vtkDataArray.newInstance({
23+
values: Uint32Array.from([1, 2, 3]),
24+
});
25+
t.deepEqual(
26+
{
27+
dataType: 'Uint32Array',
28+
size: 3,
29+
numberOfComponents: 1,
30+
values: Uint32Array.from([1, 2, 3]),
31+
},
32+
getDataArrayProperties(dataArray1),
33+
'Create instance with values (typed array)'
34+
);
35+
36+
const dataArray2 = vtkDataArray.newInstance({});
37+
t.deepEqual(
38+
{
39+
dataType: DefaultDataType,
40+
size: 0,
41+
numberOfComponents: 1,
42+
values: macro.newTypedArray(DefaultDataType),
43+
},
44+
getDataArrayProperties(dataArray2),
45+
'Create instance without values'
46+
);
47+
48+
const dataArray3 = vtkDataArray.newInstance({
49+
values: [1, 2, 3],
50+
});
51+
t.deepEqual(
52+
{
53+
dataType: DefaultDataType,
54+
size: 3,
55+
numberOfComponents: 1,
56+
values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
57+
},
58+
getDataArrayProperties(dataArray3),
59+
'Create instance with values (untyped array)'
60+
);
61+
62+
dataArray2.setData([4, 5, 6]);
63+
t.deepEqual(
64+
{
65+
dataType: DefaultDataType,
66+
size: 3,
67+
numberOfComponents: 1,
68+
values: macro.newTypedArrayFrom(DefaultDataType, [4, 5, 6]),
69+
},
70+
getDataArrayProperties(dataArray2),
71+
'Add values to empty instance (untyped array)'
72+
);
73+
74+
// Not supposed to call setData without parameters
75+
t.throws(
76+
() => dataArray2.setData(),
77+
'Empty an instance (pass undefined array)'
78+
);
79+
80+
dataArray3.setData([]);
81+
t.deepEqual(
82+
{
83+
dataType: DefaultDataType,
84+
size: 0,
85+
numberOfComponents: 1,
86+
values: macro.newTypedArray(DefaultDataType),
87+
},
88+
getDataArrayProperties(dataArray3),
89+
'Empty an instance (pass [] array)'
90+
);
91+
92+
const dataArray4 = vtkDataArray.newInstance({
93+
size: 3,
94+
});
95+
t.deepEqual(
96+
{
97+
dataType: DefaultDataType,
98+
size: 3,
99+
numberOfComponents: 1,
100+
values: macro.newTypedArray(DefaultDataType, 3),
101+
},
102+
getDataArrayProperties(dataArray4),
103+
'Create instance giving only size = 3'
104+
);
105+
106+
t.throws(
107+
() => vtkDataArray.newInstance({ empty: false }),
108+
'Create instance with empty false, no values'
109+
);
110+
8111
t.end();
9112
});
10113

‎Sources/Common/Core/Points/index.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,20 @@ function vtkPoints(publicAPI, model) {
8080
// Object factory
8181
// ----------------------------------------------------------------------------
8282

83-
const DEFAULT_VALUES = {
84-
empty: true,
85-
numberOfComponents: 3,
86-
dataType: VtkDataTypes.FLOAT,
87-
bounds: [1, -1, 1, -1, 1, -1],
88-
};
83+
function defaultValues(initialValues) {
84+
return {
85+
empty: true,
86+
numberOfComponents: 3,
87+
dataType: VtkDataTypes.FLOAT,
88+
bounds: [1, -1, 1, -1, 1, -1],
89+
...initialValues,
90+
};
91+
}
8992

9093
// ----------------------------------------------------------------------------
9194

9295
export function extend(publicAPI, model, initialValues = {}) {
93-
Object.assign(model, DEFAULT_VALUES, initialValues);
94-
96+
Object.assign(initialValues, defaultValues(initialValues));
9597
vtkDataArray.extend(publicAPI, model, initialValues);
9698
vtkPoints(publicAPI, model);
9799
}

‎Sources/Common/DataModel/Cell/test/testCell.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ test('Test vtkCell deepCopy', (t) => {
5353
cell.initialize(points);
5454

5555
const cell2 = vtkCell.newInstance();
56-
cell2.deepCopy(cell);
56+
cell.deepCopy(cell2);
5757
t.notEqual(cell2.getPoints(), points);
5858
t.deepEqual(cell2.getPoints().getData(), points.getData());
5959

‎Sources/Rendering/Misc/FullScreenRenderWindow/index.js‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,11 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
9494
model.interactor.initialize();
9595
model.interactor.bindEvents(model.container);
9696

97-
// Expose background
9897
publicAPI.setBackground = model.renderer.setBackground;
98+
publicAPI.getBackground = model.renderer.getBackground;
99+
100+
// Update BG color
101+
publicAPI.setBackground(...model.background);
99102

100103
publicAPI.removeController = () => {
101104
const el = model.controlContainer;
@@ -137,9 +140,6 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
137140
});
138141
};
139142

140-
// Update BG color
141-
publicAPI.setBackground(...model.background);
142-
143143
// Representation API
144144
publicAPI.addRepresentation = (representation) => {
145145
representation.getActors().forEach((actor) => {
@@ -188,19 +188,24 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
188188
// Object factory
189189
// ----------------------------------------------------------------------------
190190

191-
const DEFAULT_VALUES = {
192-
background: [0.32, 0.34, 0.43],
193-
containerStyle: null,
194-
controlPanelStyle: null,
195-
listenWindowResize: true,
196-
resizeCallback: null,
197-
controllerVisibility: true,
198-
};
191+
function defaultValues(initialValues) {
192+
return {
193+
background: [0.32, 0.34, 0.43],
194+
containerStyle: null,
195+
controlPanelStyle: null,
196+
listenWindowResize: true,
197+
resizeCallback: null,
198+
controllerVisibility: true,
199+
...initialValues,
200+
};
201+
}
199202

200203
// ----------------------------------------------------------------------------
201204

202205
export function extend(publicAPI, model, initialValues = {}) {
203-
Object.assign(model, DEFAULT_VALUES, initialValues);
206+
Object.assign(initialValues, defaultValues(initialValues));
207+
// we do not want to "store" background, only forward it to renderer
208+
model.background = initialValues.background;
204209

205210
// Object methods
206211
macro.obj(publicAPI, model);
@@ -213,9 +218,10 @@ export function extend(publicAPI, model, initialValues = {}) {
213218
'container',
214219
'controlContainer',
215220
]);
216-
221+
macro.setArray(publicAPI, model, ['background'], 3, 1.0);
217222
// Object specific methods
218223
vtkFullScreenRenderWindow(publicAPI, model);
224+
delete model.background;
219225
}
220226

221227
// ----------------------------------------------------------------------------

‎Sources/Rendering/Misc/RemoteView/index.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,9 @@ function vtkRemoteView(publicAPI, model) {
183183
};
184184

185185
// Initialize viewStream if available
186-
if (model.viewStream) {
187-
publicAPI.setViewStream(model.viewStream);
188-
}
186+
// if (model.viewStream) {
187+
// publicAPI.setViewStream(model.viewStream);
188+
// }
189189
}
190190

191191
// ----------------------------------------------------------------------------

‎Sources/Testing/testMacro.js‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const DEFAULT_VALUES = {
3535
myProp11: 11,
3636
_myProp12: [12],
3737
_myProp13: 13,
38+
myProp14: 14,
3839
};
3940

4041
// ----------------------------------------------------------------------------
@@ -72,7 +73,12 @@ function extend(publicAPI, model, initialValues = {}) {
7273
// Protected variables
7374
macro.setGet(publicAPI, model, ['_myProp11']);
7475
macro.setGetArray(publicAPI, model, ['_myProp12'], 1);
75-
macro.moveToProtected(publicAPI, model, ['myProp11', 'myProp12', 'myProp13']);
76+
macro.moveToProtected(publicAPI, model, [
77+
'myProp11',
78+
'myProp12',
79+
'myProp13',
80+
'myProp14',
81+
]);
7682

7783
// Object specific methods
7884
myClass(publicAPI, model);
@@ -337,12 +343,17 @@ test('Macro protected variables tests', (t) => {
337343
_myProp11: 111,
338344
_myProp12: [112],
339345
_myProp13: 113,
346+
_myProp14: 114,
340347
});
341-
t.deepEqual(overridenInstance2.get('_myProp11', '_myProp12', '_myProp13'), {
342-
_myProp11: DEFAULT_VALUES.myProp11, // TBD
343-
_myProp12: [112],
344-
_myProp13: 113,
345-
});
348+
t.deepEqual(
349+
overridenInstance2.get('_myProp11', '_myProp12', '_myProp13', '_myProp14'),
350+
{
351+
_myProp11: 111,
352+
_myProp12: [112],
353+
_myProp13: 113,
354+
_myProp14: 114,
355+
}
356+
);
346357
t.end();
347358
});
348359

‎Sources/macros.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,7 @@ export function newInstance(extend, className) {
972972
const model = {};
973973
const publicAPI = {};
974974
extend(publicAPI, model, initialValues);
975+
publicAPI.set(initialValues, true);
975976

976977
return Object.freeze(publicAPI);
977978
};

0 commit comments

Comments
 (0)