Skip to content

Commit

Permalink
Increase MAX_DEFINES in DefineList (re-do) (#2116)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@stephengold
stephengold authored Oct 18, 2023
1 parent 5975b3f commit 58365c4
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 68 deletions.
12 changes: 1 addition & 11 deletions jme3-core/src/main/java/com/jme3/material/TechniqueDef.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -422,11 +422,6 @@ public VarType getDefineIdType(int defineId) {
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);
Expand All @@ -445,11 +440,6 @@ public void addShaderParamDefine(String paramName, VarType paramType, String def
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;
Expand Down
51 changes: 30 additions & 21 deletions jme3-core/src/main/java/com/jme3/shader/DefineList.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -32,6 +32,7 @@
package com.jme3.shader;

import java.util.Arrays;
import java.util.BitSet;
import java.util.List;

/**
Expand All @@ -41,20 +42,19 @@
*/
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);
}
Expand All @@ -65,18 +65,18 @@ private void rangeCheck(int id) {

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;
}

Expand Down Expand Up @@ -124,7 +124,7 @@ public void setAll(DefineList other) {
}

public void clear() {
isSet = 0;
isSet.clear();
Arrays.fill(values, 0);
}

Expand All @@ -142,21 +142,30 @@ public int getInt(int id) {

@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() {
Expand Down
10 changes: 5 additions & 5 deletions jme3-core/src/test/java/com/jme3/renderer/OpaqueComparatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ public void testSortByShaderDefines() {
lightingMatTCVColorLight.setBoolean("VertexLighting", true);
lightingMatTCVColorLight.setBoolean("SeparateTexCoord", true);

testSort(lightingMat, lightingMatVColor, lightingMatVLight,
lightingMatVColorLight, lightingMatTC, lightingMatTCVColorLight);
testSort(lightingMatVColor, lightingMat, lightingMatVColorLight,
lightingMatVLight, lightingMatTC, lightingMatTCVColorLight);
}

@Test
Expand Down Expand Up @@ -332,8 +332,8 @@ public void testSortByAll() {
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);
}
}
81 changes: 50 additions & 31 deletions jme3-core/src/test/java/com/jme3/shader/DefineListTest.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -91,7 +91,9 @@ private String generateSource(DefineList dl) {
@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("");
}

Expand All @@ -100,46 +102,68 @@ public void testSourceBooleanDefine() {
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("");
}

@Test
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("");
}

Expand All @@ -148,11 +172,17 @@ public void testSourceFloatDefine() {
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);
Expand Down Expand Up @@ -191,49 +221,38 @@ public void testEqualsAndHashCode() {
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);
}

Expand Down

0 comments on commit 58365c4

Please sign in to comment.