bugfix: unique rgb for regions when rgb is absent

common/util.js

@@ -47,14 +47,13 @@
    *
    * https://stackoverflow.com/a/16348977/6059235
    */
-  exports.intToRgb = int => {
-    if (int >= 65500) {
-      return [255, 255, 255]
-    }
-    const str = String(int * 65535)
+  exports.strToRgb = str => {
+    if (typeof str !== 'string') throw new Error(`strToRgb input must be typeof string !`)
+
     let hash = 0
-    for (let i = 0; i < str.length; i++) {
-      hash = str.charCodeAt(i) + ((hash << 5) - hash);
+    // run at least 2 cycles, or else, len 1 string does not get hashed well
+    for (let i = 0; i < str.length || i < 5; i++) {
+      hash = str.charCodeAt(i % str.length) + ((hash << 5) - hash);
     }
     const returnV = []
     for (let i = 0; i < 3; i++) {

common/util.spec.js

-import { getIdFromFullId } from './util'
+import { getIdFromFullId, strToRgb } from './util'
 
 describe('common/util.js', () => {
   describe('getIdFromFullId', () => {
@@ -17,4 +17,63 @@ describe('common/util.js', () => {
       expect(getIdFromFullId(fullId)).toBe(`minds/core/parcellationregion/v1.0.0/a844d80f-1d94-41a0-901a-14ae257519db`)
     })
   })
+
+  describe('strToRgb', () => {
+    const str1 = 'hello world'
+    const str2 = 'foo bar'
+    const str3 = 'a'
+    const str4 = 'b'
+    const strArr = [
+      str1,
+      str2,
+      str3,
+      str4,
+    ]
+    it('should return rgb', () => {
+      const outs = strArr.map(strToRgb)
+      for (const out of outs) {
+        expect(
+          out instanceof Array
+        ).toBeTruthy()
+
+        expect(out.length).toEqual(3)
+
+        for (const n of out) {
+          expect(n).toBeGreaterThanOrEqual(0)
+          expect(n).toBeLessThanOrEqual(255)
+        }
+      }
+      
+    })
+
+    it('rgb returned should be disinct', () => {
+
+      const outs = strArr.map(strToRgb)
+      for (let i = 0; i < outs.length; i++) {
+        const compareA = outs[i]
+        for (let j = i + 1; j < outs.length; j++) {
+          const compareB = outs[j]
+          // compare all generated rgb, expect at least 1 of rgb to be of greater than 5 units out
+          expect(
+            compareA.some((n, idx) => Math.abs( n - compareB[idx] ) > 5)
+          ).toBeTruthy()
+        }
+      }
+    })
+
+    it ('should throw if not providing stirng', () => {
+      expect(() => {
+        strToRgb(12)
+      }).toThrow()
+      
+      expect(() => {
+        strToRgb(['hello world'])
+      }).toThrow()
+
+      expect(() => {
+        strToRgb({foo: 'baz'})
+      }).toThrow()
+    })
+  
+  })
 })

docs/releases/v2.4.0.md

@@ -3,6 +3,7 @@
 ## Bugfixes
 
 - fixes UI issue, where chips wraps on smaller screen (#740)
+- regions with the same labelIndex without colour will no longer have the same colour (#750)
 
 ## New features
 
@@ -12,4 +13,4 @@
 
 - refactored code, added additional test coverage
 - reworked screenshot component, removed html2canvas dependency
-- deprecated `getToastHandler` and `geModalHandler` in plugin API
\ No newline at end of file
+- deprecated `getToastHandler` and `geModalHandler` in plugin API

src/atlasComponents/parcellationRegion/region.base.ts

@@ -7,7 +7,7 @@ import { ARIA_LABELS } from 'common/constants'
 import { flattenRegions, getIdFromFullId, rgbToHsl } from 'common/util'
 import { viewerStateSetConnectivityRegion, viewerStateNavigateToRegion, viewerStateToggleRegionSelect, viewerStateNewViewer, isNewerThan } from "src/services/state/viewerState.store.helper";
 import { viewerStateFetchedTemplatesSelector, viewerStateGetSelectedAtlas, viewerStateSelectedTemplateFullInfoSelector, viewerStateSelectedTemplateSelector } from "src/services/state/viewerState/selectors";
-import { intToRgb, verifyPositionArg, getRegionHemisphere } from 'common/util'
+import { strToRgb, verifyPositionArg, getRegionHemisphere } from 'common/util'
 
 export class RegionBase {
 
@@ -36,7 +36,11 @@ export class RegionBase {
     this.position = val && val.position
     if (!this._region) return
 
-    const rgb = this._region.rgb || (this._region.labelIndex && intToRgb(Number(this._region.labelIndex))) || [255, 200, 200]
+    let rgb = this._region.rgb
+    rgb = rgb || this._region.labelIndex > 65500 ? [255, 255, 255] : null
+    rgb = rgb || strToRgb(`${this._region.ngId || this._region.name}${this._region.labelIndex}`)
+    rgb = rgb || [255, 200, 200]
+    
     this.rgbString = `rgb(${rgb.join(',')})`
     const [_h, _s, l] = rgbToHsl(...rgb)
     this.rgbDarkmode = l < 0.4

src/viewerModule/nehuba/nehubaViewer/nehubaViewer.component.ts

@@ -10,7 +10,7 @@ import { getExportNehuba, getViewer, setNehubaViewer } from "src/util/fn";
 import '!!file-loader?context=third_party&name=main.bundle.js!export-nehuba/dist/min/main.bundle.js'
 import '!!file-loader?context=third_party&name=chunk_worker.bundle.js!export-nehuba/dist/min/chunk_worker.bundle.js'
 import { scanSliceViewRenderFn } from "../util";
-import { intToRgb as intToColour, deserialiseParcRegionId } from 'common/util'
+import { strToRgb, deserialiseParcRegionId } from 'common/util'
 
 const NG_LANDMARK_LAYER_NAME = 'spatial landmark layer'
 const NG_USER_LANDMARK_LAYER_NAME = 'user landmark layer'
@@ -944,7 +944,7 @@ export class NehubaViewerUnit implements OnInit, OnDestroy {
               return [val, {}]
             }),
           ],
-        ).map((val: [number, any]) => ([val[0], this.getRgb(val[0], val[1].rgb)])) as any),
+        ).map((val: [number, any]) => ([val[0], this.getRgb(val[0], { ngId, rgb: val[1].rgb})])) as any),
       ]
     }) as Array<[string, Map<number, {red: number, green: number, blue: number}>]>
 
@@ -987,9 +987,17 @@ export class NehubaViewerUnit implements OnInit, OnDestroy {
     })
   }
 
-  private getRgb(labelIndex: number, rgb?: number[]): {red: number, green: number, blue: number} {
+  private getRgb(labelIndex: number, region: { rgb: number[], ngId: string }): {red: number, green: number, blue: number} {
+    const { rgb, ngId } = region
     if (typeof rgb === 'undefined' || rgb === null) {
-      const arr = intToColour(Number(labelIndex))
+      if (labelIndex > 65500) {
+        return {
+          red: 255,
+          green: 255,
+          blue: 255
+        }
+      }
+      const arr = strToRgb(`${ngId}${labelIndex}`)
       return {
         red : arr[0],
         green: arr[1],