java-topology/defects/libgdx/patch/libgdx-0001-model-loadnode-hashmap-lookup.patch

160 lines
6.7 KiB
Diff
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# UNDF: UNDF-2026-000000139
Fixes libgdx-0001/0002/0003: Model.loadNode / ModelBuilder.rebuildReferences /
ModelInstance.invalidate — O(N²) Array linear scans replaced with HashMap/IdentityHashMap.
--- a/gdx/src/com/badlogic/gdx/graphics/g3d/Model.java
+++ b/gdx/src/com/badlogic/gdx/graphics/g3d/Model.java
@@ DEFECT libgdx-0001: loadNode() — nested for-loop string-ID scan, O(parts × meshes + parts × materials)
@@ FIXME comment on line 188 already calls this out: "create temporary maps for faster lookup?"
protected void loadNodes (Iterable<ModelNode> modelNodes) {
nodePartBones.clear();
+ // FIX libgdx-0001: build lookup maps once before processing all nodes
+ Map<String, MeshPart> meshPartById = new java.util.HashMap<>();
+ for (int i = 0; i < meshParts.size; i++) {
+ MeshPart part = meshParts.get(i);
+ meshPartById.put(part.id, part);
+ }
+ Map<String, Material> materialById = new java.util.HashMap<>();
+ for (int i = 0; i < materials.size; i++) {
+ Material mat = materials.get(i);
+ materialById.put(mat.id, mat);
+ }
for (ModelNode node : modelNodes) {
- nodes.add(loadNode(node));
+ nodes.add(loadNode(node, meshPartById, materialById));
}
// ...bone transforms loop unchanged...
}
-protected Node loadNode (ModelNode modelNode) {
+protected Node loadNode (ModelNode modelNode,
+ Map<String, MeshPart> meshPartById,
+ Map<String, Material> materialById) {
Node node = new Node();
node.id = modelNode.id;
// ...translation/rotation/scale unchanged...
- // FIXME create temporary maps for faster lookup?
+ // FIX libgdx-0001: maps provided by caller — O(1) lookups below
if (modelNode.parts != null) {
for (ModelNodePart modelNodePart : modelNode.parts) {
MeshPart meshPart = null;
Material meshMaterial = null;
if (modelNodePart.meshPartId != null) {
- for (MeshPart part : meshParts) { // O(M) linear scan — CWE-407
- if (modelNodePart.meshPartId.equals(part.id)) {
- meshPart = part;
- break;
- }
- }
+ meshPart = meshPartById.get(modelNodePart.meshPartId); // O(1)
}
if (modelNodePart.materialId != null) {
- for (Material material : materials) { // O(T) linear scan — CWE-407
- if (modelNodePart.materialId.equals(material.id)) {
- meshMaterial = material;
- break;
- }
- }
+ meshMaterial = materialById.get(modelNodePart.materialId); // O(1)
}
// ...rest unchanged...
}
}
if (modelNode.children != null) {
for (ModelNode child : modelNode.children) {
- node.addChild(loadNode(child));
+ node.addChild(loadNode(child, meshPartById, materialById));
}
}
return node;
}
--- a/gdx/src/com/badlogic/gdx/graphics/g3d/utils/ModelBuilder.java
+++ b/gdx/src/com/badlogic/gdx/graphics/g3d/utils/ModelBuilder.java
@@ DEFECT libgdx-0002: rebuildReferences() — Array.contains() in node-part loop, O(parts × materials)
+import java.util.IdentityHashMap;
public static void rebuildReferences (final Model model) {
model.materials.clear();
model.meshes.clear();
model.meshParts.clear();
+ // FIX libgdx-0002: identity sets for O(1) dedup instead of O(N) Array.contains()
+ IdentityHashMap<Material, Boolean> matSeen = new IdentityHashMap<>();
+ IdentityHashMap<MeshPart, Boolean> partSeen = new IdentityHashMap<>();
+ IdentityHashMap<Mesh, Boolean> meshSeen = new IdentityHashMap<>();
for (final Node node : model.nodes)
- rebuildReferences(model, node);
+ rebuildReferences(model, node, matSeen, partSeen, meshSeen);
}
-private static void rebuildReferences (final Model model, final Node node) {
+private static void rebuildReferences (final Model model, final Node node,
+ IdentityHashMap<Material, Boolean> matSeen,
+ IdentityHashMap<MeshPart, Boolean> partSeen,
+ IdentityHashMap<Mesh, Boolean> meshSeen) {
for (final NodePart mpm : node.parts) {
- if (!model.materials.contains(mpm.material, true)) // O(M) CWE-407
+ if (matSeen.put(mpm.material, Boolean.TRUE) == null) // O(1)
model.materials.add(mpm.material);
- if (!model.meshParts.contains(mpm.meshPart, true)) { // O(P) CWE-407
+ if (partSeen.put(mpm.meshPart, Boolean.TRUE) == null) { // O(1)
model.meshParts.add(mpm.meshPart);
- if (!model.meshes.contains(mpm.meshPart.mesh, true)) // O(X) CWE-407
+ if (meshSeen.put(mpm.meshPart.mesh, Boolean.TRUE) == null) // O(1)
model.meshes.add(mpm.meshPart.mesh);
model.manageDisposable(mpm.meshPart.mesh);
}
}
for (final Node child : node.getChildren())
- rebuildReferences(model, child);
+ rebuildReferences(model, child, matSeen, partSeen, meshSeen);
}
--- a/gdx/src/com/badlogic/gdx/graphics/g3d/ModelInstance.java
+++ b/gdx/src/com/badlogic/gdx/graphics/g3d/ModelInstance.java
@@ DEFECT libgdx-0003: invalidate() — Array.contains() in node-part loop, O(parts × materials)
+import java.util.IdentityHashMap;
-private void invalidate () {
+private void invalidate () {
+ // FIX libgdx-0003: identity set for O(1) dedup, avoids O(N²) Array.contains()
+ IdentityHashMap<Material, Boolean> seen = new IdentityHashMap<>();
for (int i = 0, n = nodes.size; i < n; ++i) {
- invalidate(nodes.get(i));
+ invalidate(nodes.get(i), seen);
}
}
-private void invalidate (Node node) {
+private void invalidate (Node node, IdentityHashMap<Material, Boolean> seen) {
for (int i = 0, n = node.parts.size; i < n; ++i) {
NodePart part = node.parts.get(i);
ArrayMap<Node, Matrix4> bindPose = part.invBoneBindTransforms;
if (bindPose != null) {
for (int j = 0; j < bindPose.size; ++j) {
bindPose.keys[j] = getNode(bindPose.keys[j].id);
}
}
- if (!materials.contains(part.material, true)) { // O(T) CWE-407
+ if (!seen.containsKey(part.material)) { // O(1)
final int midx = materials.indexOf(part.material, false);
if (midx < 0)
materials.add(part.material = part.material.copy());
else
part.material = materials.get(midx);
+ seen.put(part.material, Boolean.TRUE);
}
}
for (int i = 0, n = node.getChildCount(); i < n; ++i) {
- invalidate(node.getChild(i));
+ invalidate(node.getChild(i), seen);
}
}