refactor(fullscreenrenderwindow, dataarray, remoteview, macro): refac…

…tor 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
Kitware · May 16, 2022 · 0761730 · 0761730
1 parent 230092f
commit 0761730
Show file tree

Hide file tree

Showing 9 changed files with 261 additions and 62 deletions.
diff --git a/Sources/Common/Core/CellArray/index.js b/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);
 }
 

diff --git a/Sources/Common/Core/DataArray/index.js b/Sources/Common/Core/DataArray/index.js
@@ -262,9 +262,14 @@ function vtkDataArray(publicAPI, model) {
   };
 
   publicAPI.setData = (typedArray, numberOfComponents) => {
+    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 +318,59 @@ 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 model.empty;
+  delete initialValues.values;
+  delete initialValues.empty;
+  delete initialValues.numberOfComponents;
+  delete initialValues.size;
+  delete initialValues.dataType;
 }
 
 // ----------------------------------------------------------------------------

diff --git a/Sources/Common/Core/DataArray/test/testDataArray.js b/Sources/Common/Core/DataArray/test/testDataArray.js
@@ -1,10 +1,168 @@
 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';
 
-test('Test vtkDataArray instance', (t) => {
+const { DefaultDataType } = Constants;
+
+function getDataArrayProperties(dataArray) {
+  return {
+    size: dataArray.get().size,
+    numberOfComponents: dataArray.get().numberOfComponents,
+    dataType: dataArray.get().dataType,
+    values: dataArray.get().values,
+  };
+}
+
+test.only('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({ empty: false }),
+    'Create instance with empty false, no values'
+  );
+
+  t.throws(
+    () => vtkDataArray.newInstance({}),
+    'Create instance without values'
+  );
+
+  t.doesNotThrow(
+    () => vtkDataArray.newInstance({ size: 256 }),
+    'Create instance with only size'
+  );
+
+  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)'
+  );
+
+  dataArray3.setData([4, 5, 6, 7]);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 4,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [4, 5, 6, 7]),
+    },
+    getDataArrayProperties(dataArray3),
+    'Change data of existing instance'
+  );
+
+  // Not supposed to call setData without parameters
+  t.throws(
+    () => dataArray2.setData(),
+    'Empty an instance (pass undefined array)'
+  );
+
+  dataArray3.setData([]);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 0,
+      numberOfComponents: 1,
+      values: macro.newTypedArray(DefaultDataType),
+    },
+    getDataArrayProperties(dataArray3),
+    'Empty an instance (pass [] 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: [],
+    numberOfComponents: 1,
+  });
+  dataArray5.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(dataArray5),
+      'Change number of components at with setData'
+    );
+
+  dataArray5.setData([1, 2, 3, 4], 3);
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 4,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3, 4]),
+    },
+    getDataArrayProperties(dataArray5),
+    'Change number of components at with setData but wrong numberOfComponents'
+  );
+
+  const dataArray6 = vtkDataArray.newInstance({
+    values: [1, 2, 3],
+    dataType: null,
+  });
+  t.deepEqual(
+    {
+      dataType: DefaultDataType,
+      size: 3,
+      numberOfComponents: 1,
+      values: macro.newTypedArrayFrom(DefaultDataType, [1, 2, 3]),
+    },
+    getDataArrayProperties(dataArray6),
+    'Give null as dataType'
+  );
+
   t.end();
 });
 

diff --git a/Sources/Common/Core/Points/index.js b/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);
 }

diff --git a/Sources/Common/DataModel/Cell/test/testCell.js b/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());
 

diff --git a/Sources/Rendering/Misc/FullScreenRenderWindow/index.js b/Sources/Rendering/Misc/FullScreenRenderWindow/index.js
@@ -94,8 +94,14 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
   model.interactor.initialize();
   model.interactor.bindEvents(model.container);
 
-  // Expose background
-  publicAPI.setBackground = model.renderer.setBackground;
+  publicAPI.setBackground = macro.chain(
+    publicAPI.setBackground,
+    model.renderer.setBackground
+  );
+  publicAPI.getBackground = model.renderer.getBackground;
+
+  // Update BG color
+  publicAPI.setBackground(...model.background);
 
   publicAPI.removeController = () => {
     const el = model.controlContainer;
@@ -137,9 +143,6 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
     });
   };
 
-  // Update BG color
-  publicAPI.setBackground(...model.background);
-
   // Representation API
   publicAPI.addRepresentation = (representation) => {
     representation.getActors().forEach((actor) => {
@@ -188,19 +191,24 @@ function vtkFullScreenRenderWindow(publicAPI, model) {
 // Object factory
 // ----------------------------------------------------------------------------
 
-const DEFAULT_VALUES = {
-  background: [0.32, 0.34, 0.43],
-  containerStyle: null,
-  controlPanelStyle: null,
-  listenWindowResize: true,
-  resizeCallback: null,
-  controllerVisibility: true,
-};
+function defaultValues(initialValues) {
+  return {
+    background: [0.32, 0.34, 0.43],
+    containerStyle: null,
+    controlPanelStyle: null,
+    listenWindowResize: true,
+    resizeCallback: null,
+    controllerVisibility: true,
+    ...initialValues,
+  };
+}
 
 // ----------------------------------------------------------------------------
 
 export function extend(publicAPI, model, initialValues = {}) {
-  Object.assign(model, DEFAULT_VALUES, initialValues);
+  Object.assign(initialValues, defaultValues(initialValues));
+  // we do not want to "store" background, only forward it to renderer
+  model.background = initialValues.background;
 
   // Object methods
   macro.obj(publicAPI, model);
@@ -213,9 +221,10 @@ export function extend(publicAPI, model, initialValues = {}) {
     'container',
     'controlContainer',
   ]);
-
+  macro.setArray(publicAPI, model, ['background'], 3, 1.0);
   // Object specific methods
   vtkFullScreenRenderWindow(publicAPI, model);
+  delete model.background;
 }
 
 // ----------------------------------------------------------------------------