Skip to content

Commit 78e38b9

Browse files
committed
refactor(dataarray): change setData
Rename values to data in extend function and accept null as array param
1 parent 34a6681 commit 78e38b9

File tree

6 files changed

+36
-39
lines changed

6 files changed

+36
-39
lines changed

Sources/Common/Core/DataArray/index.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ export function extend(publicAPI: object, model: object, initialValues?: object)
209209
* Method use to create a new instance of vtkDataArray
210210
* @param {object} [initialValues] for pre-setting some of its content
211211
* initialValues can have a property "empty: true" to be able to create a
212-
* model without giving values. This property will not be stored in the model
212+
* model without giving data. This property will not be stored in the model
213213
*/
214214
export function newInstance(initialValues?: object): vtkDataArray;
215215

Sources/Common/Core/DataArray/index.js

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ function vtkDataArray(publicAPI, model) {
264264
};
265265

266266
publicAPI.setData = (typedArray, numberOfComponents) => {
267-
if (!typedArray) {
267+
if (typedArray === undefined) {
268268
vtkErrorMacro(`Cannot call setData with given array : ${typedArray}`);
269269
return;
270270
}
@@ -273,14 +273,15 @@ function vtkDataArray(publicAPI, model) {
273273
// eslint-disable-next-line no-param-reassign
274274
typedArray = macro.newTypedArrayFrom(model.dataType, typedArray);
275275
}
276+
276277
model.values = typedArray;
277-
model.size = typedArray.length;
278-
model.dataType = getDataType(typedArray);
278+
model.size = typedArray ? typedArray.length : 0;
279+
model.dataType = typedArray ? getDataType(typedArray) : model.dataType;
279280

280281
if (numberOfComponents) {
281282
model.numberOfComponents = numberOfComponents;
282283
}
283-
if (model.size % model.numberOfComponents !== 0) {
284+
if (model.size % model.numberOfComponents !== 0 || model.size === 0) {
284285
model.numberOfComponents = 1;
285286
}
286287
dataChange();
@@ -332,7 +333,7 @@ function defaultValues(initialValues) {
332333
size: 0,
333334
dataType: DefaultDataType,
334335
rangeTuple: [0, 0],
335-
// values: macro.newTypedArray(DefaultValues),
336+
// data: macro.newTypedArray(DefaultValues),
336337
// ranges: null,
337338
...initialValues,
338339
};
@@ -346,6 +347,9 @@ export function extend(publicAPI, model, initialValues = {}) {
346347
'Cannot create vtkDataArray object without: size > 0, values'
347348
);
348349
}
350+
let initialData = initialValues.values;
351+
delete initialValues.empty;
352+
delete initialValues.values;
349353
Object.assign(initialValues, defaultValues(initialValues));
350354

351355
// Object methods
@@ -355,28 +359,17 @@ export function extend(publicAPI, model, initialValues = {}) {
355359
model.dataType = initialValues.dataType
356360
? initialValues.dataType
357361
: DefaultDataType;
358-
if (!initialValues.values) {
359-
initialValues.values = macro.newTypedArray(
360-
model.dataType,
361-
initialValues.size
362-
);
363-
} else if (Array.isArray(initialValues.values)) {
364-
initialValues.values = macro.newTypedArrayFrom(
365-
model.dataType,
366-
initialValues.values
367-
);
362+
delete initialValues.dataType;
363+
if (!initialData) {
364+
initialData = macro.newTypedArray(model.dataType, initialValues.size);
365+
} else if (Array.isArray(initialData)) {
366+
initialData = macro.newTypedArrayFrom(model.dataType, initialData);
368367
}
369368

369+
initialValues.data = initialData;
370+
370371
// Object specific methods
371372
vtkDataArray(publicAPI, model);
372-
// We need to call setData manually here because it is not a setter for a property
373-
// in itself but calling it is still the proper way to set the datas of the model
374-
publicAPI.setData(initialValues.values, initialValues.numberOfComponents);
375-
delete initialValues.values;
376-
delete initialValues.empty;
377-
delete initialValues.numberOfComponents;
378-
delete initialValues.size;
379-
delete initialValues.dataType;
380373
}
381374

382375
// ----------------------------------------------------------------------------

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,22 @@ test('Test vtkDataArray instance', (t) => {
1919

2020
t.throws(
2121
() => vtkDataArray.newInstance({}),
22-
'Create instance without values'
22+
'Not allowed to create instance without initialValues'
2323
);
2424

25-
t.doesNotThrow(() => vtkDataArray.newInstance({ empty: true }));
25+
t.doesNotThrow(
26+
() => vtkDataArray.newInstance({ empty: true }),
27+
'Allowed to create instance with empty true, no values'
28+
);
2629

2730
t.throws(
2831
() => vtkDataArray.newInstance({ empty: false }),
29-
'Create instance with empty false, no values'
32+
'Not allowed to create instance with empty false, no values'
3033
);
3134

3235
t.doesNotThrow(
3336
() => vtkDataArray.newInstance({ size: 256 }),
34-
'Create instance with only size'
37+
'Allowed to create instance with only size'
3538
);
3639

3740
const dataArray0 = vtkDataArray.newInstance({
@@ -137,7 +140,7 @@ test('Test vtkDataArray setData', (t) => {
137140
values: macro.newTypedArrayFrom(DefaultDataType, [4, 5, 6, 7]),
138141
},
139142
getDataArrayProperties(dataArray),
140-
'Change data of existing instance'
143+
'Change values of existing instance'
141144
);
142145

143146
dataArray.setData([1, 2, 3, 4, 5, 6], 2);
@@ -176,28 +179,29 @@ test('Test vtkDataArray setData', (t) => {
176179
'Empty an instance (pass [] array)'
177180
);
178181

179-
// For both of those cases, check it does not change the DataArray
180-
// Not supposed to call setData with typedArray = null
182+
// Fill the DataArray before so that we are sure the size and the numberOfComponents is updated
183+
dataArray.setData([1, 2, 3], 3);
181184
dataArray.setData(null);
182185
t.deepEqual(
183186
{
184187
dataType: DefaultDataType,
185188
size: 0,
186189
numberOfComponents: 1,
187-
values: macro.newTypedArray(DefaultDataType),
190+
values: null,
188191
},
189192
getDataArrayProperties(dataArray),
190193
'Call setData with typedArray = null'
191194
);
192195

193-
// Not supposed to call setData without parameters
196+
// Not supposed to call setData without parameters : check it does nothing on the DataArray
197+
dataArray.setData([1, 2, 3]);
194198
dataArray.setData();
195199
t.deepEqual(
196200
{
197201
dataType: DefaultDataType,
198-
size: 0,
202+
size: 3,
199203
numberOfComponents: 1,
200-
values: macro.newTypedArray(DefaultDataType),
204+
values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
201205
},
202206
getDataArrayProperties(dataArray),
203207
'Call setData with typedArray = undefined'
@@ -212,7 +216,7 @@ test('Test vtkDataArray setData', (t) => {
212216
values: macro.newTypedArrayFrom('Uint32Array', [4, 5, 6, 7]),
213217
},
214218
getDataArrayProperties(dataArray),
215-
'Change data of existing instance with another type'
219+
'Change values of existing instance with another type'
216220
);
217221
});
218222

Sources/Common/Core/Points/index.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ function vtkPoints(publicAPI, model) {
8282

8383
function defaultValues(initialValues) {
8484
return {
85+
// empty is only here to be passed to the DataArray extend function in order
86+
// to create Points without giving values
8587
empty: true,
8688
numberOfComponents: 3,
8789
dataType: VtkDataTypes.FLOAT,

Sources/Rendering/Misc/FullScreenRenderWindow/index.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export function extend(publicAPI: object, model: object, initialValues?: IFullSc
128128
/**
129129
* Method used to create a new instance of vtkFullScreenRenderWindow
130130
* @param {IFullScreenRenderWindowInitialValues} [initialValues] for pre-setting some of its content
131+
* a background property can be given in the initialValues to set the background of the renderer
131132
*/
132133
export function newInstance(initialValues?: IFullScreenRenderWindowInitialValues): vtkFullScreenRenderWindow;
133134

Sources/Rendering/Misc/FullScreenRenderWindow/index.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,6 @@ function defaultValues(initialValues) {
200200
// ----------------------------------------------------------------------------
201201

202202
export function extend(publicAPI, model, initialValues = {}) {
203-
// a background property can be set in the initialValues but we do not want
204-
// to "store" background, only forward it to the renderer
205-
206203
Object.assign(initialValues, defaultValues(initialValues));
207204

208205
// Object methods

0 commit comments

Comments
 (0)