Skip to content

Commit 0761730

Browse files
joannacirillojoannacirillo
committed
refactor(fullscreenrenderwindow, dataarray, remoteview, macro): refactor constructors
Refactor of (some classes) constructors so that the setXXX() methods of coresponding 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 as it is not a setter of one property in itself but has that purpose. Tests have been added and updated. 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. If a setter of a property is called in the function creating an object (vtkObject(publicAPI, model)) such call should be removed to avoid witnessing twice the same side effects. fix(dataarray, test): undefined or null parameters for newInstance undefined or null parameters for empty models creation and dataType can be null for newInstance test(dataarray): setData with number of components setData with number of components not default tests style(remoteview): remove useless comments remove useless commented part
1 parent 230092f commit 0761730

File tree

9 files changed

+261
-62
lines changed

9 files changed

+261
-62
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: 43 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,59 @@ 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 && !initialValues.values && !initialValues.size) {
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+
? initialValues.dataType
350+
: DefaultDataType;
351+
if (!initialValues.values) {
352+
initialValues.values = macro.newTypedArray(
353+
model.dataType,
354+
initialValues.size
355+
);
356+
} else if (Array.isArray(initialValues.values)) {
357+
initialValues.values = macro.newTypedArrayFrom(
358+
model.dataType,
359+
initialValues.values
360+
);
361+
}
362+
352363
// Object specific methods
353364
vtkDataArray(publicAPI, model);
365+
// We need to call setData manually here because it is not a setter for a property
366+
// in itself but calling it is still the proper way to set the datas of the model
367+
publicAPI.setData(initialValues.values, initialValues.numberOfComponents);
368+
delete model.empty;
369+
delete initialValues.values;
370+
delete initialValues.empty;
371+
delete initialValues.numberOfComponents;
372+
delete initialValues.size;
373+
delete initialValues.dataType;
354374
}
355375

356376
// ----------------------------------------------------------------------------

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

Lines changed: 161 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,168 @@
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';
35

4-
test('Test vtkDataArray instance', (t) => {
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+
}
16+
17+
test.only('Test vtkDataArray instance', (t) => {
518
t.ok(vtkDataArray, 'Make sure the class definition exists');
6-
const instance = vtkDataArray.newInstance({ size: 256 });
7-
t.ok(instance);
19+
20+
t.throws(
21+
() => vtkDataArray.newInstance({ empty: false }),
22+
'Create instance with empty false, no values'
23+
);
24+
25+
t.throws(
26+
() => vtkDataArray.newInstance({}),
27+
'Create instance without values'
28+
);
29+
30+
t.doesNotThrow(
31+
() => vtkDataArray.newInstance({ size: 256 }),
32+
'Create instance with only size'
33+
);
34+
35+
const dataArray1 = vtkDataArray.newInstance({ size: 256 });
36+
t.deepEqual(
37+
{
38+
dataType: DefaultDataType,
39+
size: 256,
40+
numberOfComponents: 1,
41+
values: macro.newTypedArray(DefaultDataType, 256),
42+
},
43+
getDataArrayProperties(dataArray1),
44+
'Give only size to create instance'
45+
);
46+
47+
const dataArray2 = vtkDataArray.newInstance({
48+
values: Uint32Array.from([1, 2, 3]),
49+
});
50+
t.deepEqual(
51+
{
52+
dataType: 'Uint32Array',
53+
size: 3,
54+
numberOfComponents: 1,
55+
values: Uint32Array.from([1, 2, 3]),
56+
},
57+
getDataArrayProperties(dataArray2),
58+
'Create instance with values (typed array)'
59+
);
60+
61+
const dataArray3 = vtkDataArray.newInstance({
62+
values: [1, 2, 3],
63+
});
64+
t.deepEqual(
65+
{
66+
dataType: DefaultDataType,
67+
size: 3,
68+
numberOfComponents: 1,
69+
values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
70+
},
71+
getDataArrayProperties(dataArray3),
72+
'Create instance with values (untyped array)'
73+
);
74+
75+
dataArray3.setData([4, 5, 6, 7]);
76+
t.deepEqual(
77+
{
78+
dataType: DefaultDataType,
79+
size: 4,
80+
numberOfComponents: 1,
81+
values: macro.newTypedArrayFrom(DefaultDataType, [4, 5, 6, 7]),
82+
},
83+
getDataArrayProperties(dataArray3),
84+
'Change data of existing instance'
85+
);
86+
87+
// Not supposed to call setData without parameters
88+
t.throws(
89+
() => dataArray2.setData(),
90+
'Empty an instance (pass undefined array)'
91+
);
92+
93+
dataArray3.setData([]);
94+
t.deepEqual(
95+
{
96+
dataType: DefaultDataType,
97+
size: 0,
98+
numberOfComponents: 1,
99+
values: macro.newTypedArray(DefaultDataType),
100+
},
101+
getDataArrayProperties(dataArray3),
102+
'Empty an instance (pass [] array)'
103+
);
104+
105+
const dataArray4 = vtkDataArray.newInstance({
106+
values: [1, 2, 3, 4, 5, 6, 7, 8, 9],
107+
numberOfComponents: 3,
108+
});
109+
t.deepEqual(
110+
{
111+
dataType: DefaultDataType,
112+
size: 9,
113+
numberOfComponents: 3,
114+
values: macro.newTypedArrayFrom(
115+
DefaultDataType,
116+
[1, 2, 3, 4, 5, 6, 7, 8, 9]
117+
),
118+
},
119+
getDataArrayProperties(dataArray4),
120+
'Change number of components at instanciation'
121+
);
122+
123+
const dataArray5 = vtkDataArray.newInstance({
124+
values: [],
125+
numberOfComponents: 1,
126+
});
127+
dataArray5.setData([1, 2, 3, 4, 5, 6], 2),
128+
t.deepEqual(
129+
{
130+
dataType: DefaultDataType,
131+
size: 6,
132+
numberOfComponents: 2,
133+
values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3, 4, 5, 6]),
134+
},
135+
getDataArrayProperties(dataArray5),
136+
'Change number of components at with setData'
137+
);
138+
139+
dataArray5.setData([1, 2, 3, 4], 3);
140+
t.deepEqual(
141+
{
142+
dataType: DefaultDataType,
143+
size: 4,
144+
numberOfComponents: 1,
145+
values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3, 4]),
146+
},
147+
getDataArrayProperties(dataArray5),
148+
'Change number of components at with setData but wrong numberOfComponents'
149+
);
150+
151+
const dataArray6 = vtkDataArray.newInstance({
152+
values: [1, 2, 3],
153+
dataType: null,
154+
});
155+
t.deepEqual(
156+
{
157+
dataType: DefaultDataType,
158+
size: 3,
159+
numberOfComponents: 1,
160+
values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
161+
},
162+
getDataArrayProperties(dataArray6),
163+
'Give null as dataType'
164+
);
165+
8166
t.end();
9167
});
10168

‎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: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,14 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
9494
model.interactor.initialize();
9595
model.interactor.bindEvents(model.container);
9696

97-
// Expose background
98-
publicAPI.setBackground = model.renderer.setBackground;
97+
publicAPI.setBackground = macro.chain(
98+
publicAPI.setBackground,
99+
model.renderer.setBackground
100+
);
101+
publicAPI.getBackground = model.renderer.getBackground;
102+
103+
// Update BG color
104+
publicAPI.setBackground(...model.background);
99105

100106
publicAPI.removeController = () => {
101107
const el = model.controlContainer;
@@ -137,9 +143,6 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
137143
});
138144
};
139145

140-
// Update BG color
141-
publicAPI.setBackground(...model.background);
142-
143146
// Representation API
144147
publicAPI.addRepresentation = (representation) => {
145148
representation.getActors().forEach((actor) => {
@@ -188,19 +191,24 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
188191
// Object factory
189192
// ----------------------------------------------------------------------------
190193

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-
};
194+
function defaultValues(initialValues) {
195+
return {
196+
background: [0.32, 0.34, 0.43],
197+
containerStyle: null,
198+
controlPanelStyle: null,
199+
listenWindowResize: true,
200+
resizeCallback: null,
201+
controllerVisibility: true,
202+
...initialValues,
203+
};
204+
}
199205

200206
// ----------------------------------------------------------------------------
201207

202208
export function extend(publicAPI, model, initialValues = {}) {
203-
Object.assign(model, DEFAULT_VALUES, initialValues);
209+
Object.assign(initialValues, defaultValues(initialValues));
210+
// we do not want to "store" background, only forward it to renderer
211+
model.background = initialValues.background;
204212

205213
// Object methods
206214
macro.obj(publicAPI, model);
@@ -213,9 +221,10 @@ export function extend(publicAPI, model, initialValues = {}) {
213221
'container',
214222
'controlContainer',
215223
]);
216-
224+
macro.setArray(publicAPI, model, ['background'], 3, 1.0);
217225
// Object specific methods
218226
vtkFullScreenRenderWindow(publicAPI, model);
227+
delete model.background;
219228
}
220229

221230
// ----------------------------------------------------------------------------

0 commit comments

Comments
 (0)