Increase MAX_DEFINES in DefineList (re-do) (#2116)

* DefineList:  re-implement using a BitSet to remove the size limit

* TechniqueDef:  DefineList size is no longer limited, so remove 2 checks

* OpaqueComparatorTest: update expected sort order affected by DefineList

* DefineListTest:  rewrite tests to accept any valid hashCode() function

jme3-core/src/main/java/com/jme3/material/TechniqueDef.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009-2021 jMonkeyEngine
+ * Copyright (c) 2009-2023 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -422,11 +422,6 @@ public class TechniqueDef implements Savable, Cloneable {
     public void addShaderParamDefine(String paramName, VarType paramType, String defineName) {
         int defineId = defineNames.size();
 
-        if (defineId >= DefineList.MAX_DEFINES) {
-            throw new IllegalStateException("Cannot have more than " +
-                    DefineList.MAX_DEFINES + " defines on a technique.");
-        }
-
         paramToDefineId.put(paramName, defineId);
         defineNames.add(defineName);
         defineTypes.add(paramType);
@@ -445,11 +440,6 @@ public class TechniqueDef implements Savable, Cloneable {
     public int addShaderUnmappedDefine(String defineName, VarType defineType) {
         int defineId = defineNames.size();
 
-        if (defineId >= DefineList.MAX_DEFINES) {
-            throw new IllegalStateException("Cannot have more than " +
-                    DefineList.MAX_DEFINES + " defines on a technique.");
-        }
-
         defineNames.add(defineName);
         defineTypes.add(defineType);
         return defineId;

jme3-core/src/main/java/com/jme3/shader/DefineList.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009-2015 jMonkeyEngine
+ * Copyright (c) 2009-2023 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -32,6 +32,7 @@
 package com.jme3.shader;
 
 import java.util.Arrays;
+import java.util.BitSet;
 import java.util.List;
 
 /**
@@ -41,20 +42,19 @@ import java.util.List;
  */
 public final class DefineList {
 
-    public static final int MAX_DEFINES = 64;
-
-    private long isSet;
+    private final BitSet isSet;
     private final int[] values;
 
     public DefineList(int numValues) {
-        if (numValues < 0 || numValues > MAX_DEFINES) {
-            throw new IllegalArgumentException("numValues must be between 0 and 64");
+        if (numValues < 0) {
+            throw new IllegalArgumentException("numValues must be >= 0");
         }
         values = new int[numValues];
+        isSet = new BitSet(numValues);
     }
 
     private DefineList(DefineList original) {
-        this.isSet = original.isSet;
+        this.isSet = (BitSet) original.isSet.clone();
         this.values = new int[original.values.length];
         System.arraycopy(original.values, 0, values, 0, values.length);
     }
@@ -65,18 +65,18 @@ public final class DefineList {
 
     public boolean isSet(int id) {
         rangeCheck(id);
-        return (isSet & (1L << id)) != 0;
+        return isSet.get(id);
     }
 
     public void unset(int id) {
         rangeCheck(id);
-        isSet &= ~(1L << id);
+        isSet.clear(id);
         values[id] = 0;
     }
 
     public void set(int id, int val) {
         rangeCheck(id);
-        isSet |= (1L << id);
+        isSet.set(id, true);
         values[id] = val;
     }
 
@@ -124,7 +124,7 @@ public final class DefineList {
     }
 
     public void clear() {
-        isSet = 0;
+        isSet.clear();
         Arrays.fill(values, 0);
     }
 
@@ -142,21 +142,30 @@ public final class DefineList {
 
     @Override
     public int hashCode() {
-        return (int) ((isSet >> 32) ^ isSet);
+        return isSet.hashCode();
     }
 
     @Override
-    public boolean equals(Object other) {
-        DefineList o = (DefineList) other;
-        if (isSet == o.isSet) {
-            for (int i = 0; i < values.length; i++) {
-                if (values[i] != o.values[i]) {
-                    return false;
-                }
-            }
+    public boolean equals(Object object) {
+        if (this == object) {
             return true;
         }
-        return false;
+        if (object == null || object.getClass() != getClass()) {
+            return false;
+        }
+        DefineList otherDefineList = (DefineList) object;
+        if (values.length != otherDefineList.values.length) {
+            return false;
+        }
+        if (!isSet.equals(otherDefineList.isSet)) {
+            return false;
+        }
+        for (int i = 0; i < values.length; i++) {
+            if (values[i] != otherDefineList.values[i]) {
+                return false;
+            }
+        }
+        return true;
     }
 
     public DefineList deepClone() {

jme3-core/src/test/java/com/jme3/renderer/OpaqueComparatorTest.java

@@ -252,8 +252,8 @@ public class OpaqueComparatorTest {
         lightingMatTCVColorLight.setBoolean("VertexLighting", true);
         lightingMatTCVColorLight.setBoolean("SeparateTexCoord", true);
         
-        testSort(lightingMat, lightingMatVColor, lightingMatVLight,
-                 lightingMatVColorLight, lightingMatTC, lightingMatTCVColorLight);
+        testSort(lightingMatVColor, lightingMat, lightingMatVColorLight,
+                lightingMatVLight, lightingMatTC, lightingMatTCVColorLight);
     }
     
     @Test
@@ -332,8 +332,8 @@ public class OpaqueComparatorTest {
         Material mat2000 = matBase2.clone();
         mat2000.setName("2000");
         
-        testSort(mat1100, mat1101, mat1102, mat1110, 
-                 mat1120, mat1121, mat1122, mat1140, 
-                 mat1200, mat1210, mat1220, mat2000);
+        testSort(mat1110, mat1100, mat1101, mat1102, 
+                 mat1140, mat1120, mat1121, mat1122, 
+                 mat1220, mat1210, mat1200, mat2000);
     }
 }

jme3-core/src/test/java/com/jme3/shader/DefineListTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009-2021 jMonkeyEngine
+ * Copyright (c) 2009-2023 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -91,7 +91,9 @@ public class DefineListTest {
     @Test
     public void testSourceInitial() {
         DefineList dl = new DefineList(NUM_DEFINES);
-        assert dl.hashCode() == 0;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            assert !dl.isSet(id);
+        }
         assert generateSource(dl).equals("");
     }
 
@@ -100,19 +102,29 @@ public class DefineListTest {
         DefineList dl = new DefineList(NUM_DEFINES);
 
         dl.set(BOOL_VAR, true);
-        assert dl.hashCode() == 1;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isBoolVar = (id == BOOL_VAR);
+            assert dl.isSet(id) == isBoolVar;
+        }
         assert generateSource(dl).equals("#define BOOL_VAR 1\n");
 
         dl.set(BOOL_VAR, false);
-        assert dl.hashCode() == 0;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            assert !dl.isSet(id);
+        }
         assert generateSource(dl).equals("");
 
         dl.set(BOOL_VAR, true);
-        assert dl.hashCode() == 1;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isBoolVar = (id == BOOL_VAR);
+            assert dl.isSet(id) == isBoolVar;
+        }
         assert generateSource(dl).equals("#define BOOL_VAR 1\n");
 
         dl.unset(BOOL_VAR);
-        assert dl.hashCode() == 0;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            assert !dl.isSet(id);
+        }
         assert generateSource(dl).equals("");
     }
 
@@ -120,26 +132,38 @@ public class DefineListTest {
     public void testSourceIntDefine() {
         DefineList dl = new DefineList(NUM_DEFINES);
 
-        int hashCodeWithInt = 1 << INT_VAR;
-
         dl.set(INT_VAR, 123);
-        assert dl.hashCode() == hashCodeWithInt;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isIntVar = (id == INT_VAR);
+            assert dl.isSet(id) == isIntVar;
+        }
         assert generateSource(dl).equals("#define INT_VAR 123\n");
 
         dl.set(INT_VAR, 0);
-        assert dl.hashCode() == hashCodeWithInt;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isIntVar = (id == INT_VAR);
+            assert dl.isSet(id) == isIntVar;
+        }
         assert generateSource(dl).equals("#define INT_VAR 0\n");
 
         dl.set(INT_VAR, -99);
-        assert dl.hashCode() == hashCodeWithInt;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isIntVar = (id == INT_VAR);
+            assert dl.isSet(id) == isIntVar;
+        }
         assert generateSource(dl).equals("#define INT_VAR -99\n");
 
         dl.set(INT_VAR, Integer.MAX_VALUE);
-        assert dl.hashCode() == hashCodeWithInt;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isIntVar = (id == INT_VAR);
+            assert dl.isSet(id) == isIntVar;
+        }
         assert generateSource(dl).equals("#define INT_VAR 2147483647\n");
 
         dl.unset(INT_VAR);
-        assert dl.hashCode() == 0;
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            assert !dl.isSet(id);
+        }
         assert generateSource(dl).equals("");
     }
 
@@ -148,11 +172,17 @@ public class DefineListTest {
         DefineList dl = new DefineList(NUM_DEFINES);
 
         dl.set(FLOAT_VAR, 1f);
-        assert dl.hashCode() == (1 << FLOAT_VAR);
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isFloatVar = (id == FLOAT_VAR);
+            assert dl.isSet(id) == isFloatVar;
+        }
         assert generateSource(dl).equals("#define FLOAT_VAR 1.0\n");
 
         dl.set(FLOAT_VAR, 0f);
-        assert dl.hashCode() == (1 << FLOAT_VAR);
+        for (int id = 0; id < NUM_DEFINES; ++id) {
+            boolean isFloatVar = (id == FLOAT_VAR);
+            assert dl.isSet(id) == isFloatVar;
+        }
         assert generateSource(dl).equals("#define FLOAT_VAR 0.0\n");
 
         dl.set(FLOAT_VAR, -1f);
@@ -191,49 +221,38 @@ public class DefineListTest {
         DefineList dl1 = new DefineList(NUM_DEFINES);
         DefineList dl2 = new DefineList(NUM_DEFINES);
 
-        assertEquals(0, dl1.hashCode());
-        assertEquals(0, dl2.hashCode());
+        assertEquals(dl1.hashCode(), dl2.hashCode());
         assertEquals(dl1, dl2);
 
         dl1.set(BOOL_VAR, true);
 
-        assertEquals(1, dl1.hashCode());
-        assertEquals(0, dl2.hashCode());
         assertNotEquals(dl1, dl2);
 
         dl2.set(BOOL_VAR, true);
 
-        assertEquals(1, dl1.hashCode());
-        assertEquals(1, dl2.hashCode());
+        assertEquals(dl1.hashCode(), dl2.hashCode());
         assertEquals(dl1, dl2);
 
         dl1.set(INT_VAR, 2);
 
-        assertEquals(1 | 2, dl1.hashCode());
-        assertEquals(1, dl2.hashCode());
         assertNotEquals(dl1, dl2);
 
         dl2.set(INT_VAR, 2);
 
-        assertEquals(1 | 2, dl1.hashCode());
-        assertEquals(1 | 2, dl2.hashCode());
+        assertEquals(dl1.hashCode(), dl2.hashCode());
         assertEquals(dl1, dl2);
 
         dl1.set(BOOL_VAR, false);
 
-        assertEquals(2, dl1.hashCode());
-        assertEquals(1 | 2, dl2.hashCode());
         assertNotEquals(dl1, dl2);
 
         dl2.unset(BOOL_VAR);
 
-        assertEquals(2, dl1.hashCode());
-        assertEquals(2, dl2.hashCode());
+        assertEquals(dl1.hashCode(), dl2.hashCode());
         assertEquals(dl1, dl2); // unset is the same as false
 
         dl1.unset(BOOL_VAR);
-        assertEquals(2, dl1.hashCode());
-        assertEquals(2, dl2.hashCode());
+        assertEquals(dl1.hashCode(), dl2.hashCode());
         assertEquals(dl1, dl2);
     }