159 lines
6.7 KiB
Diff
159 lines
6.7 KiB
Diff
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);
|
||
}
|
||
}
|