define 0/0.0 int/float values in shader

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

@@ -43,30 +43,41 @@ public final class DefineList {
 
     public static final int MAX_DEFINES = 64;
 
-    private long hash;
-    private final int[] vals;
+    private long 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");
         }
-        vals = new int[numValues];
+        values = new int[numValues];
     }
 
     private DefineList(DefineList original) {
-        this.hash = original.hash;
-        this.vals = new int[original.vals.length];
-        System.arraycopy(original.vals, 0, vals, 0, vals.length);
+        this.isSet = original.isSet;
+        this.values = new int[original.values.length];
+        System.arraycopy(original.values, 0, values, 0, values.length);
     }
 
-    public void set(int id, int val) {
-        assert 0 <= id && id < 64;
-        if (val != 0) {
-            hash |=  (1L << id);
-        } else {
-            hash &= ~(1L << id);
+    private void rangeCheck(int id) {
+        assert 0 <= id && id < values.length;
+    }
+
+    public boolean isSet(int id) {
+        rangeCheck(id);
+        return (isSet & (1L << id)) != 0;
+    }
+
+    public void unset(int id) {
+        rangeCheck(id);
+        isSet &= ~(1L << id);
+        values[id] = 0;
     }
-        vals[id] = val;
+
+    public void set(int id, int val) {
+        rangeCheck(id);
+        isSet |= (1L << id);
+        values[id] = val;
     }
 
     public void set(int id, float val) {
@@ -74,15 +85,17 @@ public final class DefineList {
     }
 
     public void set(int id, boolean val) {
-        set(id, val ? 1 : 0);
+        if (val) {
+            set(id, 1);
+        } else {
+            // Because #ifdef usage is very common in shaders, unset the define
+            // instead of setting it to 0 for booleans.
+            unset(id);
         }
-
-    public void set(int id, VarType type, Object value) {
-        if (value == null) {
-            set(id, 0);
-            return;
     }
 
+    public void set(int id, VarType type, Object value) {
+        if (value != null) {
             switch (type) {
                 case Int:
                     set(id, (Integer) value);
@@ -97,44 +110,49 @@ public final class DefineList {
                     set(id, 1);
                     break;
             }
+        } else {
+            unset(id);
+        }
     }
 
     public void setAll(DefineList other) {
-        for (int i = 0; i < other.vals.length; i++) {
-            if (other.vals[i] != 0) {
-                vals[i] = other.vals[i];
+        for (int i = 0; i < other.values.length; i++) {
+            if (other.isSet(i)) {
+                set(i, other.getInt(i));
             }
         }
     }
 
     public void clear() {
-        hash = 0;
-        Arrays.fill(vals, 0);
+        isSet = 0;
+        Arrays.fill(values, 0);
     }
 
     public boolean getBoolean(int id) {
-        return vals[id] != 0;
+        return values[id] != 0;
     }
 
     public float getFloat(int id) {
-        return Float.intBitsToFloat(vals[id]);
+        return Float.intBitsToFloat(values[id]);
     }
 
     public int getInt(int id) {
-        return vals[id];
+        return values[id];
     }
 
     @Override
     public int hashCode() {
-        return (int)((hash >> 32) ^ hash);
+        return (int) ((isSet >> 32) ^ isSet);
     }
 
     @Override
     public boolean equals(Object other) {
         DefineList o = (DefineList) other;
-         if (hash == o.hash) {
-             for (int i = 0; i < vals.length; i++) {
-                  if (vals[i] != o.vals[i]) return false;
+        if (isSet == o.isSet) {
+            for (int i = 0; i < values.length; i++) {
+                if (values[i] != o.values[i]) {
+                    return false;
+                }
             }
             return true;
         }
@@ -146,16 +164,15 @@ public final class DefineList {
     }
 
     public void generateSource(StringBuilder sb, List<String> defineNames, List<VarType> defineTypes) {
-        for (int i = 0; i < vals.length; i++) {
-            if (vals[i] != 0) {
-                String defineName = defineNames.get(i);
+        for (int i = 0; i < values.length; i++) {
+            if (!isSet(i)) {
+                continue;
+            }
 
-                sb.append("#define ");
-                sb.append(defineName);
-                sb.append(" ");
+            sb.append("#define ").append(defineNames.get(i)).append(' ');
 
             if (defineTypes != null && defineTypes.get(i) == VarType.Float) {
-                    float val = Float.intBitsToFloat(vals[i]);
+                float val = Float.intBitsToFloat(values[i]);
                 if (Float.isInfinite(val) || Float.isNaN(val)) {
                     throw new IllegalArgumentException(
                             "GLSL does not support NaN "
@@ -163,11 +180,10 @@ public final class DefineList {
                 }
                 sb.append(val);
             } else {
-                    sb.append(vals[i]);
+                sb.append(values[i]);
             }
 
-                sb.append("\n");
-            }
+            sb.append('\n');
         }
     }
 

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

@@ -107,6 +107,14 @@ public class DefineListTest {
         dl.set(BOOL_VAR, false);
         assert dl.hashCode() == 0;
         assert generateSource(dl).equals("");
+
+        dl.set(BOOL_VAR, true);
+        assert dl.hashCode() == 1;
+        assert generateSource(dl).equals("#define BOOL_VAR 1\n");
+
+        dl.unset(BOOL_VAR);
+        assert dl.hashCode() == 0;
+        assert generateSource(dl).equals("");
     }
 
     @Test
@@ -120,8 +128,8 @@ public class DefineListTest {
         assert generateSource(dl).equals("#define INT_VAR 123\n");
 
         dl.set(INT_VAR, 0);
-        assert dl.hashCode() == 0;
-        assert generateSource(dl).equals("");
+        assert dl.hashCode() == hashCodeWithInt;
+        assert generateSource(dl).equals("#define INT_VAR 0\n");
 
         dl.set(INT_VAR, -99);
         assert dl.hashCode() == hashCodeWithInt;
@@ -130,6 +138,10 @@ public class DefineListTest {
         dl.set(INT_VAR, Integer.MAX_VALUE);
         assert dl.hashCode() == hashCodeWithInt;
         assert generateSource(dl).equals("#define INT_VAR 2147483647\n");
+
+        dl.unset(INT_VAR);
+        assert dl.hashCode() == 0;
+        assert generateSource(dl).equals("");
     }
 
     @Test
@@ -141,8 +153,8 @@ public class DefineListTest {
         assert generateSource(dl).equals("#define FLOAT_VAR 1.0\n");
 
         dl.set(FLOAT_VAR, 0f);
-        assert dl.hashCode() == 0;
-        assert generateSource(dl).equals("");
+        assert dl.hashCode() == (1 << FLOAT_VAR);
+        assert generateSource(dl).equals("#define FLOAT_VAR 0.0\n");
 
         dl.set(FLOAT_VAR, -1f);
         assert generateSource(dl).equals("#define FLOAT_VAR -1.0\n");
@@ -157,19 +169,22 @@ public class DefineListTest {
             dl.set(FLOAT_VAR, Float.NaN);
             generateSource(dl);
             assert false;
-        } catch (IllegalArgumentException ex) { }
+        } catch (IllegalArgumentException ex) {
+        }
 
         try {
             dl.set(FLOAT_VAR, Float.POSITIVE_INFINITY);
             generateSource(dl);
             assert false;
-        } catch (IllegalArgumentException ex) { }
+        } catch (IllegalArgumentException ex) {
+        }
 
         try {
             dl.set(FLOAT_VAR, Float.NEGATIVE_INFINITY);
             generateSource(dl);
             assert false;
-        } catch (IllegalArgumentException ex) { }
+        } catch (IllegalArgumentException ex) {
+        }
     }
 
     @Test
@@ -177,31 +192,50 @@ public class DefineListTest {
         DefineList dl1 = new DefineList(NUM_DEFINES);
         DefineList dl2 = new DefineList(NUM_DEFINES);
 
-        assertTrue(dl1.hashCode() == 0);
+        assertEquals(0, dl1.hashCode());
+        assertEquals(0, dl2.hashCode());
         assertEquals(dl1, dl2);
 
         dl1.set(BOOL_VAR, true);
 
-        assertTrue(dl1.hashCode() == 1);
-        assertNotSame(dl1, dl2);
+        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, dl2);
 
         dl1.set(INT_VAR, 2);
 
-        assertTrue(dl1.hashCode() == (1|2));
-        assertNotSame(dl1, dl2);
+        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, dl2);
 
         dl1.set(BOOL_VAR, false);
 
-        assertTrue(dl1.hashCode() == 2);
-        assertNotSame(dl1, dl2);
+        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, dl2); // unset is the same as false
+
+        dl1.unset(BOOL_VAR);
+        assertEquals(2, dl1.hashCode());
+        assertEquals(2, dl2.hashCode());
+        assertEquals(dl1, dl2);
     }
 
     @Test
@@ -209,25 +243,31 @@ public class DefineListTest {
         DefineList dl1 = new DefineList(NUM_DEFINES);
         DefineList dl2 = dl1.deepClone();
 
-        assertFalse(dl1 == dl2);
-        assertTrue(dl1.equals(dl2));
-        assertTrue(dl1.hashCode() == dl2.hashCode());
+        assertNotSame(dl1, dl2);
+        assertEquals(dl1, dl2);
+        assertEquals(dl1.hashCode(), dl2.hashCode());
 
         dl1.set(BOOL_VAR, true);
         dl2 = dl1.deepClone();
 
-        assertTrue(dl1.equals(dl2));
-        assertTrue(dl1.hashCode() == dl2.hashCode());
+        assertEquals(dl1, dl2);
+        assertEquals(dl1.hashCode(), dl2.hashCode());
+
+        dl1.set(BOOL_VAR, false);
+        dl2 = dl1.deepClone();
+
+        assertEquals(dl1, dl2);
+        assertEquals(dl1.hashCode(), dl2.hashCode());
 
         dl1.set(INT_VAR, 123);
 
-        assertFalse(dl1.equals(dl2));
-        assertFalse(dl1.hashCode() == dl2.hashCode());
+        assertNotEquals(dl1, dl2);
+        assertNotEquals(dl1.hashCode(), dl2.hashCode());
 
         dl2 = dl1.deepClone();
 
-        assertTrue(dl1.equals(dl2));
-        assertTrue(dl1.hashCode() == dl2.hashCode());
+        assertEquals(dl1, dl2);
+        assertEquals(dl1.hashCode(), dl2.hashCode());
     }
 
     @Test
@@ -242,8 +282,8 @@ public class DefineListTest {
 
         dl.set(INT_VAR, 123);
 
-        assertEquals("#define BOOL_VAR 1\n" + 
-                     "#define INT_VAR 123\n", generateSource(dl));
+        assertEquals("#define BOOL_VAR 1\n"
+                + "#define INT_VAR 123\n", generateSource(dl));
 
         dl.set(BOOL_VAR, false);
 
@@ -252,15 +292,18 @@ public class DefineListTest {
         dl.set(BOOL_VAR, true);
 
         // should have predictable ordering based on defineId
-        assertEquals("#define BOOL_VAR 1\n" + 
-                     "#define INT_VAR 123\n", generateSource(dl));
+        assertEquals("#define BOOL_VAR 1\n"
+                + "#define INT_VAR 123\n", generateSource(dl));
+
+        dl.unset(BOOL_VAR);
+        assertEquals("#define INT_VAR 123\n", generateSource(dl));
     }
 
-    private static String doLookup(HashMap<DefineList, String> map, boolean boolVal, int intVal, float floatVal) {
+    private static String doLookup(HashMap<DefineList, String> map, Boolean boolVal, Integer intVal, Float floatVal) {
         DefineList dl = new DefineList(NUM_DEFINES);
-        dl.set(BOOL_VAR, boolVal);
-        dl.set(INT_VAR, intVal);
-        dl.set(FLOAT_VAR, floatVal);
+        dl.set(BOOL_VAR, VarType.Boolean, boolVal);
+        dl.set(INT_VAR, VarType.Int, intVal);
+        dl.set(FLOAT_VAR, VarType.Float, floatVal);
         return map.get(dl);
     }
 
@@ -272,7 +315,7 @@ public class DefineListTest {
         String STR_BOOL_INT = "This define list has both a boolean and int value";
         String STR_BOOL_INT_FLOAT = "This define list has a boolean, int, and float value";
 
-        HashMap<DefineList, String> map = new HashMap<DefineList, String>();
+        HashMap<DefineList, String> map = new HashMap<>();
 
         DefineList lookup = new DefineList(NUM_DEFINES);
 
@@ -291,10 +334,10 @@ public class DefineListTest {
         lookup.set(FLOAT_VAR, FastMath.PI);
         map.put(lookup.deepClone(), STR_BOOL_INT_FLOAT);
 
-        assertEquals(doLookup(map, false, 0, 0f), STR_EMPTY);
-        assertEquals(doLookup(map, false, 123, 0f), STR_INT);
-        assertEquals(doLookup(map, true, 0, 0f), STR_BOOL);
-        assertEquals(doLookup(map, true, 123, 0f), STR_BOOL_INT);
-        assertEquals(doLookup(map, true, 123, FastMath.PI), STR_BOOL_INT_FLOAT);
+        assertEquals(STR_EMPTY, doLookup(map, null, null, null));
+        assertEquals(STR_INT, doLookup(map, false, 123, null));
+        assertEquals(STR_BOOL, doLookup(map, true, null, null));
+        assertEquals(STR_BOOL_INT, doLookup(map, true, 123, null));
+        assertEquals(STR_BOOL_INT_FLOAT, doLookup(map, true, 123, FastMath.PI));
     }
 }