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 initialValues 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.

Author: joannacirillo

Sources/Common/Core/CellArray/index.js

@@ -88,6 +88,8 @@ function vtkCellArray(publicAPI, model) {
 
 function defaultValues(initialValues) {
   return {
+    // empty is only here to be passed to the DataArray extend function in order
+    // to create a cellArray without giving values
     empty: true,
     numberOfComponents: 1,
     dataType: VtkDataTypes.UNSIGNED_INT,
@@ -98,7 +100,8 @@ function defaultValues(initialValues) {
 // ----------------------------------------------------------------------------
 
 export function extend(publicAPI, model, initialValues = {}) {
-  vtkDataArray.extend(publicAPI, model, defaultValues(initialValues));
+  Object.assign(initialValues, defaultValues(initialValues));
+  vtkDataArray.extend(publicAPI, model, initialValues);
   vtkCellArray(publicAPI, model);
 }
 

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

@@ -208,6 +208,8 @@ export function extend(publicAPI: object, model: object, initialValues?: object)
 /**
  * Method use to create a new instance of vtkDataArray
  * @param {object} [initialValues] for pre-setting some of its content
+ * initialValues can have a property "empty: true" to be able to create a
+ * model without giving values. This property will not be stored in the model
  */
 export function newInstance(initialValues?: object): vtkDataArray;
 

Sources/Common/Core/DataArray/index.js

@@ -1,6 +1,7 @@
 import Constants from 'vtk.js/Sources/Common/Core/DataArray/Constants';
 import * as macro from 'vtk.js/Sources/macros';
 import * as vtkMath from 'vtk.js/Sources/Common/Core/Math';
+const { vtkErrorMacro } = macro;
 
 const { DefaultDataType } = Constants;
 const TUPLE_HOLDER = [];
@@ -262,9 +263,19 @@ function vtkDataArray(publicAPI, model) {
   };
 
   publicAPI.setData = (typedArray, numberOfComponents) => {
+    if (!typedArray) {
+      vtkErrorMacro(`Cannot call setData with given array : ${typedArray}`);
+      return;
+    }
+
+    if (Array.isArray(typedArray)) {
+      // eslint-disable-next-line no-param-reassign
+      typedArray = macro.newTypedArrayFrom(model.dataType, typedArray);
+    }
     model.values = typedArray;
     model.size = typedArray.length;
     model.dataType = getDataType(typedArray);
+
     if (numberOfComponents) {
       model.numberOfComponents = numberOfComponents;
     }
@@ -313,44 +324,58 @@ function vtkDataArray(publicAPI, model) {
 // Object factory
 // ----------------------------------------------------------------------------
 
-const DEFAULT_VALUES = {
-  name: '',
-  numberOfComponents: 1,
-  size: 0,
-  dataType: DefaultDataType,
-  rangeTuple: [0, 0],
-  // values: null,
-  // ranges: null,
-};
+function defaultValues(initialValues) {
+  return {
+    name: '',
+    numberOfComponents: 1,
+    size: 0,
+    dataType: DefaultDataType,
+    rangeTuple: [0, 0],
+    // values: macro.newTypedArray(DefaultValues),
+    // ranges: null,
+    ...initialValues,
+  };
+}
 
 // ----------------------------------------------------------------------------
 
 export function extend(publicAPI, model, initialValues = {}) {
-  Object.assign(model, DEFAULT_VALUES, initialValues);
-
-  if (!model.empty && !model.values && !model.size) {
+  if (!initialValues.empty && !initialValues.values && !initialValues.size) {
     throw new TypeError(
       'Cannot create vtkDataArray object without: size > 0, values'
     );
   }
-
-  if (!model.values) {
-    model.values = macro.newTypedArray(model.dataType, model.size);
-  } else if (Array.isArray(model.values)) {
-    model.values = macro.newTypedArrayFrom(model.dataType, model.values);
-  }
-
-  if (model.values) {
-    model.size = model.values.length;
-    model.dataType = getDataType(model.values);
-  }
+  Object.assign(initialValues, defaultValues(initialValues));
 
   // Object methods
   macro.obj(publicAPI, model);
   macro.set(publicAPI, model, ['name', 'numberOfComponents']);
 
+  model.dataType = initialValues.dataType
+    ? initialValues.dataType
+    : DefaultDataType;
+  if (!initialValues.values) {
+    initialValues.values = macro.newTypedArray(
+      model.dataType,
+      initialValues.size
+    );
+  } else if (Array.isArray(initialValues.values)) {
+    initialValues.values = macro.newTypedArrayFrom(
+      model.dataType,
+      initialValues.values
+    );
+  }
+
   // Object specific methods
   vtkDataArray(publicAPI, model);
+  // We need to call setData manually here because it is not a setter for a property
+  // in itself but calling it is still the proper way to set the datas of the model
+  publicAPI.setData(initialValues.values, initialValues.numberOfComponents);
+  delete initialValues.values;
+  delete initialValues.empty;
+  delete initialValues.numberOfComponents;
+  delete initialValues.size;
+  delete initialValues.dataType;
 }
 
 // ----------------------------------------------------------------------------

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

@@ -1,13 +1,221 @@
 import test from 'tape-catch';
 import vtkDataArray from 'vtk.js/Sources/Common/Core/DataArray';
+import Constants from 'vtk.js/Sources/Common/Core/DataArray/Constants';
+import * as macro from 'vtk.js/Sources/macros';
+
+const { DefaultDataType } = Constants;
+
+function getDataArrayProperties(dataArray) {
+  return {
+    size: dataArray.get().size,
+    numberOfComponents: dataArray.get().numberOfComponents,
+    dataType: dataArray.get().dataType,
+    values: dataArray.get().values,
+  };
+}
 
 test('Test vtkDataArray instance', (t) => {
   t.ok(vtkDataArray, 'Make sure the class definition exists');
-  const instance = vtkDataArray.newInstance({ size: 256 });
-  t.ok(instance);
+
+  t.throws(
+    () => vtkDataArray.newInstance({}),
+    'Create instance without values'
+  );
+
+  t.doesNotThrow(() => vtkDataArray.newInstance({ empty: true }));
+
+  t.throws(
+    () => vtkDataArray.newInstance({ empty: false }),
+    'Create instance with empty false, no values'
+  );
+
+  t.doesNotThrow(
+    () => vtkDataArray.newInstance({ size: 256 }),
+    'Create instance with only size'
+  );
+
+  const dataArray0 = vtkDataArray.newInstance({
+    empty: true,
+    values: null,
+  });
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 0,
+      numberOfComponents: 1,
+      values: macro.newTypedArray(DefaultDataType, 0),
+    },
+    getDataArrayProperties(dataArray0),
+    'initialValues.values = null'
+  );
+
+  const dataArray1 = vtkDataArray.newInstance({ size: 256 });
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 256,
+      numberOfComponents: 1,
+      values: macro.newTypedArray(DefaultDataType, 256),
+    },
+    getDataArrayProperties(dataArray1),
+    'Give only size to create instance'
+  );
+
+  const dataArray2 = vtkDataArray.newInstance({
+    values: Uint32Array.from([1, 2, 3]),
+  });
+  t.deepEqual(
+    {
+      dataType: 'Uint32Array',
+      size: 3,
+      numberOfComponents: 1,
+      values: Uint32Array.from([1, 2, 3]),
+    },
+    getDataArrayProperties(dataArray2),
+    'Create instance with values (typed array)'
+  );
+
+  const dataArray3 = vtkDataArray.newInstance({
+    values: [1, 2, 3],
+  });
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 3,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
+    },
+    getDataArrayProperties(dataArray3),
+    'Create instance with values (untyped array)'
+  );
+
+  const dataArray4 = vtkDataArray.newInstance({
+    values: [1, 2, 3, 4, 5, 6, 7, 8, 9],
+    numberOfComponents: 3,
+  });
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 9,
+      numberOfComponents: 3,
+      values: macro.newTypedArrayFrom(
+        DefaultDataType,
+        [1, 2, 3, 4, 5, 6, 7, 8, 9]
+      ),
+    },
+    getDataArrayProperties(dataArray4),
+    'Change number of components at instanciation'
+  );
+
+  const dataArray5 = vtkDataArray.newInstance({
+    values: [1, 2, 3],
+    dataType: null,
+  });
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 3,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
+    },
+    getDataArrayProperties(dataArray5),
+    'Give null as dataType'
+  );
+
   t.end();
 });
 
+test('Test vtkDataArray setData', (t) => {
+  const dataArray = vtkDataArray.newInstance({ empty: true });
+
+  dataArray.setData([4, 5, 6, 7]);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 4,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [4, 5, 6, 7]),
+    },
+    getDataArrayProperties(dataArray),
+    'Change data of existing instance'
+  );
+
+  dataArray.setData([1, 2, 3, 4, 5, 6], 2);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 6,
+      numberOfComponents: 2,
+      values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3, 4, 5, 6]),
+    },
+    getDataArrayProperties(dataArray),
+    'Change number of components with setData'
+  );
+
+  dataArray.setData([1, 2, 3, 4], 3);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 4,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3, 4]),
+    },
+    getDataArrayProperties(dataArray),
+    'Change number of components with setData but wrong numberOfComponents'
+  );
+
+  dataArray.setData([]);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 0,
+      numberOfComponents: 1,
+      values: macro.newTypedArray(DefaultDataType),
+    },
+    getDataArrayProperties(dataArray),
+    'Empty an instance (pass [] array)'
+  );
+
+  // For both of those cases, check it does not change the DataArray
+  // Not supposed to call setData with typedArray = null
+  dataArray.setData(null);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 0,
+      numberOfComponents: 1,
+      values: macro.newTypedArray(DefaultDataType),
+    },
+    getDataArrayProperties(dataArray),
+    'Call setData with typedArray = null'
+  );
+
+  // Not supposed to call setData without parameters
+  dataArray.setData();
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 0,
+      numberOfComponents: 1,
+      values: macro.newTypedArray(DefaultDataType),
+    },
+    getDataArrayProperties(dataArray),
+    'Call setData with typedArray = undefined'
+  );
+
+  dataArray.setData(macro.newTypedArrayFrom('Uint32Array', [4, 5, 6, 7]));
+  t.deepEqual(
+    {
+      dataType: 'Uint32Array',
+      size: 4,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom('Uint32Array', [4, 5, 6, 7]),
+    },
+    getDataArrayProperties(dataArray),
+    'Change data of existing instance with another type'
+  );
+});
+
 test('Test vtkDataArray getRange function with single-channel data.', (t) => {
   // create a data array with a single channel.
   const newArray = new Uint16Array(256 * 3);

Sources/Common/Core/Points/index.js

@@ -80,18 +80,20 @@ function vtkPoints(publicAPI, model) {
 // Object factory
 // ----------------------------------------------------------------------------
 
-const DEFAULT_VALUES = {
-  empty: true,
-  numberOfComponents: 3,
-  dataType: VtkDataTypes.FLOAT,
-  bounds: [1, -1, 1, -1, 1, -1],
-};
+function defaultValues(initialValues) {
+  return {
+    empty: true,
+    numberOfComponents: 3,
+    dataType: VtkDataTypes.FLOAT,
+    bounds: [1, -1, 1, -1, 1, -1],
+    ...initialValues,
+  };
+}
 
 // ----------------------------------------------------------------------------
 
 export function extend(publicAPI, model, initialValues = {}) {
-  Object.assign(model, DEFAULT_VALUES, initialValues);
-
+  Object.assign(initialValues, defaultValues(initialValues));
   vtkDataArray.extend(publicAPI, model, initialValues);
   vtkPoints(publicAPI, model);
 }

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

@@ -53,7 +53,7 @@ test('Test vtkCell deepCopy', (t) => {
   cell.initialize(points);
 
   const cell2 = vtkCell.newInstance();
-  cell2.deepCopy(cell);
+  cell.deepCopy(cell2);
   t.notEqual(cell2.getPoints(), points);
   t.deepEqual(cell2.getPoints().getData(), points.getData());