Merge pull request #2604 from PaulHax/color-map-fix

fix(ColorTransferFunction): applyColorMap calls modified in more cases

Author: floryst

Sources/Rendering/Core/ColorTransferFunction/index.js

@@ -1209,6 +1209,7 @@ function vtkColorTransferFunction(publicAPI, model) {
 
   //----------------------------------------------------------------------------
   publicAPI.applyColorMap = (colorMap) => {
+    const oldColorSpace = JSON.stringify(model.colorSpace);
     if (colorMap.ColorSpace) {
       model.colorSpace = ColorSpace[colorMap.ColorSpace.toUpperCase()];
       if (model.colorSpace === undefined) {
@@ -1218,12 +1219,18 @@ function vtkColorTransferFunction(publicAPI, model) {
         model.colorSpace = ColorSpace.RGB;
       }
     }
+    let isModified = oldColorSpace !== JSON.stringify(model.colorSpace);
+
+    const oldNanColor = isModified || JSON.stringify(model.nanColor);
     if (colorMap.NanColor) {
       model.nanColor = [].concat(colorMap.NanColor);
       while (model.nanColor.length < 4) {
         model.nanColor.push(1.0);
       }
     }
+    isModified = isModified || oldNanColor !== JSON.stringify(model.nanColor);
+
+    const oldNodes = isModified || JSON.stringify(model.nodes);
     if (colorMap.RGBPoints) {
       const size = colorMap.RGBPoints.length;
       model.nodes = [];
@@ -1240,13 +1247,15 @@ function vtkColorTransferFunction(publicAPI, model) {
         });
       }
     }
-    // FIXME: not supported ?
-    // if (colorMap.IndexedColors) {
-    // }
-    // if (colorMap.Annotations) {
-    // }
 
-    publicAPI.sortAndUpdateRange();
+    const modifiedInvoked = publicAPI.sortAndUpdateRange();
+
+    const callModified =
+      !modifiedInvoked &&
+      (isModified || oldNodes !== JSON.stringify(model.nodes));
+    if (callModified) publicAPI.modified();
+
+    return modifiedInvoked || callModified;
   };
 }
 

Sources/Rendering/Core/ColorTransferFunction/test/testColorTransferFunction.js

@@ -159,3 +159,61 @@ test('Test discretized color transfer function', (t) => {
 
   t.end();
 });
+
+test('Test applyColorMap calls modified', (t) => {
+  const ctf = vtkColorTransferFunction.newInstance();
+
+  const colorMapA = {
+    ColorSpace: 'RGB',
+    RGBPoints: [0, 0, 0, 0, 1, 0, 0, 0],
+  };
+  const colorMapB = {
+    ColorSpace: 'RGB',
+    RGBPoints: [0, 0, 0, 0, 1, 1, 1, 1],
+  };
+
+  ctf.applyColorMap(colorMapA);
+
+  let isModified = false;
+  ctf.onModified(() => {
+    isModified = true;
+  });
+
+  ctf.applyColorMap(colorMapA);
+  t.notOk(
+    isModified,
+    `Expect applyColorMap does not call modified with same color map`
+  );
+
+  ctf.applyColorMap(colorMapB);
+  t.ok(
+    isModified,
+    `Expect applyColorMap calls modified with different color map`
+  );
+
+  colorMapB.ColorSpace = 'LAB';
+  isModified = false;
+  let modifiedReturn = ctf.applyColorMap(colorMapB);
+  t.ok(
+    isModified && modifiedReturn,
+    `Expect applyColorMap calls modified with different ColorSpace`
+  );
+
+  colorMapB.NanColor = [0, 0, 0, 1];
+  isModified = false;
+  modifiedReturn = ctf.applyColorMap(colorMapB);
+  t.ok(
+    isModified && modifiedReturn,
+    `Expect applyColorMap calls modified with different NanColor`
+  );
+
+  colorMapB.RGBPoints = [0, 0, 0, 0, 0.5, 1, 1, 1, 1, 1, 1, 1];
+  isModified = false;
+  modifiedReturn = ctf.applyColorMap(colorMapB);
+  t.ok(
+    isModified && modifiedReturn,
+    `Expect applyColorMap calls modified with different RGBPoints`
+  );
+
+  t.end();
+});