diff --git a/jme3-core/src/main/java/com/jme3/material/MatParam.java b/jme3-core/src/main/java/com/jme3/material/MatParam.java index 8e4168133e..f7ff011ea6 100644 --- a/jme3-core/src/main/java/com/jme3/material/MatParam.java +++ b/jme3-core/src/main/java/com/jme3/material/MatParam.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009-2024 jMonkeyEngine + * Copyright (c) 2009-2025 jMonkeyEngine * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -31,9 +31,6 @@ */ package com.jme3.material; -import java.io.IOException; -import java.util.Arrays; - import com.jme3.asset.TextureKey; import com.jme3.export.InputCapsule; import com.jme3.export.JmeExporter; @@ -51,6 +48,10 @@ import com.jme3.texture.Texture; import com.jme3.texture.Texture.WrapMode; +import java.io.IOException; +import java.util.Arrays; +import java.util.Objects; + /** * Describes a material parameter. This is used for both defining a name and type * as well as a material parameter value. @@ -85,6 +86,11 @@ public MatParam(VarType type, String name, Object value) { protected MatParam() { } + /** + * Checks if type checking is enabled for this material parameter. + * + * @return true if type checking is enabled, false otherwise. + */ public boolean isTypeCheckEnabled() { return typeCheck; } @@ -103,7 +109,7 @@ public void setTypeCheckEnabled(boolean typeCheck) { /** * Returns the material parameter type. * - * @return the material parameter type. + * @return The {@link VarType} of this material parameter. */ public VarType getVarType() { return type; @@ -112,24 +118,27 @@ public VarType getVarType() { /** * Returns the name of the material parameter. * - * @return the name of the material parameter. + * @return The name of the material parameter. */ public String getName() { return name; } /** - * Returns the name with "m_" prefixed to it. + * Returns the name of the material parameter prefixed with "m_". + * This prefixed name is commonly used for internal uniform naming + * in shaders (e.g., "m_Color" for a parameter named "Color"). * - * @return the name with "m_" prefixed to it + * @return The prefixed name of the material parameter. */ public String getPrefixedName() { return prefixedName; } /** - * Used internally - * @param name + * Internal use only. + * + * @param name The name for the parameter. Must not be null. */ void setName(String name) { this.name = name; @@ -152,32 +161,47 @@ public Object getValue() { /** * Sets the value of this material parameter. *

- * It is assumed the value is of the same {@link MatParam#getVarType() type} - * as this material parameter. + * It is generally assumed that the value's type + * corresponds to the {@link MatParam#getVarType() type} of this parameter. * - * @param value the value of this material parameter. + * @param value The new value for this material parameter. Can be null. + * @throws RuntimeException if type checking is enabled and the provided + * value's type is incompatible with the parameter's + * defined variable type. */ public void setValue(Object value) { if (isTypeCheckEnabled()) { - if (value != null && this.type != null && this.type.getJavaType().length != 0) { - boolean valid = false; - for (Class jtype : this.type.getJavaType()) { - if (jtype.isAssignableFrom(value.getClass())) { - valid = true; - break; - } - } - if (!valid) { - throw new RuntimeException("Trying to assign a value of type " + value.getClass() - + " to " + this.getName() - + " of type " + type.name() - + ". Valid types are " + Arrays.deepToString(type.getJavaType())); - } + if (value != null) { + validateValueType(value); } } this.value = value; } + /** + * Validates if the provided value is compatible with the parameter's type. + * + * @param value The value to validate. + * @throws RuntimeException if the value's type is not compatible. + */ + private void validateValueType(Object value) { + if (type.getJavaType().length != 0) { + boolean valid = false; + for (Class javaType : type.getJavaType()) { + if (javaType.isAssignableFrom(value.getClass())) { + valid = true; + break; + } + } + if (!valid) { + throw new RuntimeException("Trying to assign a value of type " + value.getClass() + + " to " + this.getName() + + " of type " + type.name() + + ". Valid types are " + Arrays.deepToString(type.getJavaType())); + } + } + } + /** * Returns the material parameter value as it would appear in a J3M * file. E.g. @@ -199,82 +223,21 @@ public String getValueAsString() { return value.toString(); case Vector2: Vector2f v2 = (Vector2f) value; - return v2.getX() + " " + v2.getY(); -/* -This may get used at a later point of time -When arrays can be inserted in J3M files - - case Vector2Array: - Vector2f[] v2Arr = (Vector2f[]) value; - String v2str = ""; - for (int i = 0; i < v2Arr.length ; i++) { - v2str += v2Arr[i].getX() + " " + v2Arr[i].getY() + "\n"; - } - return v2str; -*/ + return v2.x + " " + v2.y; case Vector3: Vector3f v3 = (Vector3f) value; - return v3.getX() + " " + v3.getY() + " " + v3.getZ(); -/* - case Vector3Array: - Vector3f[] v3Arr = (Vector3f[]) value; - String v3str = ""; - for (int i = 0; i < v3Arr.length ; i++) { - v3str += v3Arr[i].getX() + " " - + v3Arr[i].getY() + " " - + v3Arr[i].getZ() + "\n"; - } - return v3str; - case Vector4Array: - // can be either ColorRGBA, Vector4f or Quaternion - if (value instanceof Vector4f) { - Vector4f[] v4arr = (Vector4f[]) value; - String v4str = ""; - for (int i = 0; i < v4arr.length ; i++) { - v4str += v4arr[i].getX() + " " - + v4arr[i].getY() + " " - + v4arr[i].getZ() + " " - + v4arr[i].getW() + "\n"; - } - return v4str; - } else if (value instanceof ColorRGBA) { - ColorRGBA[] colorArr = (ColorRGBA[]) value; - String colStr = ""; - for (int i = 0; i < colorArr.length ; i++) { - colStr += colorArr[i].getRed() + " " - + colorArr[i].getGreen() + " " - + colorArr[i].getBlue() + " " - + colorArr[i].getAlpha() + "\n"; - } - return colStr; - } else if (value instanceof Quaternion) { - Quaternion[] quatArr = (Quaternion[]) value; - String quatStr = ""; - for (int i = 0; i < quatArr.length ; i++) { - quatStr += quatArr[i].getX() + " " - + quatArr[i].getY() + " " - + quatArr[i].getZ() + " " - + quatArr[i].getW() + "\n"; - } - return quatStr; - } else { - throw new UnsupportedOperationException("Unexpected Vector4Array type: " + value); - } -*/ + return v3.x + " " + v3.y + " " + v3.z; case Vector4: // can be either ColorRGBA, Vector4f or Quaternion if (value instanceof Vector4f) { Vector4f v4 = (Vector4f) value; - return v4.getX() + " " + v4.getY() + " " - + v4.getZ() + " " + v4.getW(); + return v4.x + " " + v4.y + " " + v4.z + " " + v4.w; } else if (value instanceof ColorRGBA) { ColorRGBA color = (ColorRGBA) value; - return color.getRed() + " " + color.getGreen() + " " - + color.getBlue() + " " + color.getAlpha(); + return color.r + " " + color.g + " " + color.b + " " + color.a; } else if (value instanceof Quaternion) { - Quaternion quat = (Quaternion) value; - return quat.getX() + " " + quat.getY() + " " - + quat.getZ() + " " + quat.getW(); + Quaternion q = (Quaternion) value; + return q.getX() + " " + q.getY() + " " + q.getZ() + " " + q.getW(); } else { throw new UnsupportedOperationException("Unexpected Vector4 type: " + value); } @@ -286,10 +249,6 @@ public String getValueAsString() { Texture texVal = (Texture) value; TextureKey texKey = (TextureKey) texVal.getKey(); if (texKey == null) { - // throw new UnsupportedOperationException("The specified MatParam cannot be represented in J3M"); - // this is used in toString and the above line causes blender materials to throw this exception. - // toStrings should be very robust IMO as even debuggers often invoke toString and logging code - // often does as well, even implicitly. return texVal + ":returned null key"; } @@ -323,16 +282,15 @@ public String getValueAsString() { } private String getWrapMode(Texture texVal, Texture.WrapAxis axis) { - WrapMode mode = WrapMode.EdgeClamp; try { - mode = texVal.getWrap(axis); + WrapMode mode = texVal.getWrap(axis); + if (mode != WrapMode.EdgeClamp) { + return "Wrap" + mode.name() + "_" + axis.name() + " "; + } } catch (IllegalArgumentException ex) { // this axis doesn't exist on the texture - return ""; - } - if (mode != WrapMode.EdgeClamp) { - return "Wrap" + mode.name() + "_" + axis.name() + " "; } + return ""; } @@ -377,6 +335,7 @@ public void read(JmeImporter im) throws IOException { type = ic.readEnum("varType", VarType.class, null); name = ic.readString("name", null); prefixedName = "m_" + name; + switch (getVarType()) { case Boolean: value = ic.readBoolean("value_bool", false); @@ -388,38 +347,38 @@ public void read(JmeImporter im) throws IOException { value = ic.readInt("value_int", 0); break; case Vector2Array: - Savable[] savableArray = ic.readSavableArray("value_savable_array", null); - if (savableArray != null) { - value = new Vector2f[savableArray.length]; - System.arraycopy(savableArray, 0, value, 0, savableArray.length); + Savable[] vec2Array = ic.readSavableArray("value_savable_array", null); + if (vec2Array != null) { + value = new Vector2f[vec2Array.length]; + System.arraycopy(vec2Array, 0, value, 0, vec2Array.length); } break; case Vector3Array: - savableArray = ic.readSavableArray("value_savable_array", null); - if (savableArray != null) { - value = new Vector3f[savableArray.length]; - System.arraycopy(savableArray, 0, value, 0, savableArray.length); + Savable[] vec3Array = ic.readSavableArray("value_savable_array", null); + if (vec3Array != null) { + value = new Vector3f[vec3Array.length]; + System.arraycopy(vec3Array, 0, value, 0, vec3Array.length); } break; case Vector4Array: - savableArray = ic.readSavableArray("value_savable_array", null); - if (savableArray != null) { - value = new Vector4f[savableArray.length]; - System.arraycopy(savableArray, 0, value, 0, savableArray.length); + Savable[] vec4Array = ic.readSavableArray("value_savable_array", null); + if (vec4Array != null) { + value = new Vector4f[vec4Array.length]; + System.arraycopy(vec4Array, 0, value, 0, vec4Array.length); } break; case Matrix3Array: - savableArray = ic.readSavableArray("value_savable_array", null); - if (savableArray != null) { - value = new Matrix3f[savableArray.length]; - System.arraycopy(savableArray, 0, value, 0, savableArray.length); + Savable[] mat3Array = ic.readSavableArray("value_savable_array", null); + if (mat3Array != null) { + value = new Matrix3f[mat3Array.length]; + System.arraycopy(mat3Array, 0, value, 0, mat3Array.length); } break; case Matrix4Array: - savableArray = ic.readSavableArray("value_savable_array", null); - if (savableArray != null) { - value = new Matrix4f[savableArray.length]; - System.arraycopy(savableArray, 0, value, 0, savableArray.length); + Savable[] mat4Array = ic.readSavableArray("value_savable_array", null); + if (mat4Array != null) { + value = new Matrix4f[mat4Array.length]; + System.arraycopy(mat4Array, 0, value, 0, mat4Array.length); } break; default: @@ -432,20 +391,22 @@ public void read(JmeImporter im) throws IOException { @Override public boolean equals(Object obj) { - if (obj == null) { + if (!(obj instanceof MatParam)) { return false; } - if (getClass() != obj.getClass()) { - return false; + + if (this == obj) { + return true; } + final MatParam other = (MatParam) obj; if (this.type != other.type) { return false; } - if ((this.name == null) ? (other.name != null) : !this.name.equals(other.name)) { + if (!Objects.equals(this.name, other.name)) { return false; } - if (this.value != other.value && (this.value == null || !this.value.equals(other.value))) { + if (!Objects.equals(this.value, other.value)) { return false; } return true; @@ -453,19 +414,15 @@ public boolean equals(Object obj) { @Override public int hashCode() { - int hash = 7; - hash = 59 * hash + (this.type != null ? this.type.hashCode() : 0); - hash = 59 * hash + (this.name != null ? this.name.hashCode() : 0); - hash = 59 * hash + (this.value != null ? this.value.hashCode() : 0); - return hash; + return Objects.hash(type, name, value); } @Override public String toString() { if (value != null) { - return type.name() + " " + name + " : " + getValueAsString(); - } else { - return type.name() + " " + name; + String sValue = getValueAsString(); + return type.name() + " " + name + " : " + ((sValue != null) ? sValue : value); } + return type.name() + " " + name; } } diff --git a/jme3-core/src/test/java/com/jme3/material/MaterialCompareTest.java b/jme3-core/src/test/java/com/jme3/material/MaterialCompareTest.java new file mode 100644 index 0000000000..453cbe4382 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/material/MaterialCompareTest.java @@ -0,0 +1,141 @@ +package com.jme3.material; + +import com.jme3.asset.AssetManager; +import com.jme3.asset.TextureKey; +import com.jme3.export.binary.BinaryExporter; +import com.jme3.math.ColorRGBA; +import com.jme3.system.JmeSystem; +import com.jme3.texture.Texture; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.net.URL; +import java.util.Arrays; +import java.util.List; + +/** + * Test cases for verifying the `contentEquals` and `contentHashCode` methods of the + * {@link Material} class. These tests ensure that material comparison and hashing + * behave as expected under various scenarios, including cloning, loading, + * and modifying material parameters and textures. + * + * @author capdevon + */ +public class MaterialCompareTest { + + private static AssetManager assetManager; + + /** + * Initializes the asset manager before any tests are run. + * This method loads a desktop configuration for the asset manager. + */ + @BeforeClass + public static void init() { + URL config = MaterialCompareTest.class.getResource("/com/jme3/asset/Desktop.cfg"); + assetManager = JmeSystem.newAssetManager(config); + } + + @Test + public void testMaterialCompare() { + // Test case 1: Cloned materials should be content-equal. + Material mat1 = new Material(assetManager, "Common/MatDefs/Misc/Unshaded.j3md"); + mat1.setName("mat1"); + mat1.setColor("Color", ColorRGBA.Blue); + + Material mat2 = mat1.clone(); + mat2.setName("mat2"); + Assert.assertTrue(mat1.contentEquals(mat2)); + + // Test case 2: Cloned material with a different render state should not be content-equal. + Material mat3 = mat1.clone(); + mat3.setName("mat3"); + mat3.getAdditionalRenderState().setBlendMode(RenderState.BlendMode.ModulateX2); + Assert.assertFalse(mat1.contentEquals(mat3)); + + // Test case 3: Two separately loaded materials with identical content should be content-equal. + Material mat4 = assetManager.loadMaterial("Models/Sign Post/Sign Post.j3m"); + mat4.setName("mat4"); + Material mat5 = assetManager.loadMaterial("Models/Sign Post/Sign Post.j3m"); + mat5.setName("mat5"); + Assert.assertTrue(mat4.contentEquals(mat5)); + + // Test case 4: Comparing textures - ensuring texture keys are identical for content equality. + TextureKey originalKey = (TextureKey) mat4.getTextureParam("DiffuseMap").getTextureValue().getKey(); + TextureKey tex1key = new TextureKey("Models/Sign Post/Sign Post.jpg", false); + tex1key.setGenerateMips(true); + + // The texture keys from the original and the newly loaded texture must be identical + // for their corresponding textures, and thus the materials, to be considered identical. + Assert.assertEquals(originalKey, tex1key); + + Texture tex1 = assetManager.loadTexture(tex1key); + mat4.setTexture("DiffuseMap", tex1); + Assert.assertTrue(mat4.contentEquals(mat5)); + + // Test case 5: Changing texture properties should make materials no longer content-equal. + tex1.setWrap(Texture.WrapMode.MirroredRepeat); + Assert.assertFalse(mat4.contentEquals(mat5)); + + // Test case 6: Comparing materials with different textures should result in non-equality. + Texture tex2 = assetManager.loadTexture("Interface/Logo/Monkey.jpg"); + mat4.setTexture("DiffuseMap", tex2); + Assert.assertFalse(mat4.contentEquals(mat5)); + + // Test case 7: Two materials created with the same initial properties should be content-equal. + Material mat6 = new Material(assetManager, "Common/MatDefs/Misc/Unshaded.j3md"); + mat6.setName("mat6"); + mat6.setColor("Color", ColorRGBA.Blue); + Assert.assertTrue(mat1.contentEquals(mat6)); + + // Test case 8: Changing a material parameter should make materials no longer content-equal. + mat6.setColor("Color", ColorRGBA.Green); + Assert.assertFalse(mat1.contentEquals(mat6)); + } + + @Test + public void testMaterialCloneAndSave() { + + List matDefs = Arrays.asList(Materials.UNSHADED, Materials.LIGHTING, Materials.PBR); + + Texture tex = assetManager.loadTexture("Common/Textures/MissingTexture.png"); + tex.setWrap(Texture.WrapMode.Repeat); + + int index = 0; + for (String matDef : matDefs) { + Material mat = new Material(assetManager, matDef); + + String name = index == 0 ? "ColorMap" : index == 1 ? "DiffuseMap" : "BaseColorMap"; + mat.setTexture(name, tex); + + Material clone = mat.clone(); + Assert.assertTrue(mat.contentEquals(clone)); + Assert.assertEquals(mat.contentHashCode(), clone.contentHashCode()); + test(mat, clone); + + Material copy = BinaryExporter.saveAndLoad(assetManager, mat); + Assert.assertTrue(mat.contentEquals(copy)); + Assert.assertEquals(mat.contentHashCode(), copy.contentHashCode()); + test(mat, copy); + + index++; + } + } + + private void test(Material mat, Material copy) { + for (MatParam def : mat.getMaterialDef().getMaterialParams()) { + MatParam param = mat.getParam(def.getName()); + + if (param != null) { + MatParam mCopy = copy.getParam(def.getName()); + Assert.assertEquals(param, mCopy); + Assert.assertEquals(param.hashCode(), mCopy.hashCode()); + + } else { + MatParam mCopy = copy.getMaterialDef().getMaterialParam(def.getName()); + Assert.assertEquals(def, mCopy); + Assert.assertEquals(def.hashCode(), mCopy.hashCode()); + } + } + } +}