From fe94e43bc4a6c65d9e9f3b7e394ac04e3cd6ea61 Mon Sep 17 00:00:00 2001
From: rdb <git@rdb.name>
Date: Sun, 13 Nov 2022 11:08:05 +0100
Subject: [PATCH] pgraph: Fix issues with flattening TexMatrixAttrib

If multiple texture stages reference the same texcoord set, Panda wouldn't flatten the tex matrices even if they were the exact same

Fixes #1392
---
 panda/src/pgraph/geomNode.cxx | 56 +++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/panda/src/pgraph/geomNode.cxx b/panda/src/pgraph/geomNode.cxx
index a485851448..aad9420639 100644
--- a/panda/src/pgraph/geomNode.cxx
+++ b/panda/src/pgraph/geomNode.cxx
@@ -203,38 +203,56 @@ apply_attribs_to_vertices(const AccumulatedAttribs &attribs, int attrib_types,
           // effect on the GeomNode; this may not be true if there is a
           // texture that has been applied at a node above that from which we
           // started the flatten operation, but caveat programmer.
-          NameCount name_count;
-
-          if (geom_attribs._texture != nullptr) {
-            const TextureAttrib *ta = DCAST(TextureAttrib, geom_attribs._texture);
-            int num_on_stages = ta->get_num_on_stages();
-            for (int si = 0; si < num_on_stages; si++) {
-              TextureStage *stage = ta->get_on_stage(si);
-              const InternalName *name = stage->get_texcoord_name();
-              count_name(name_count, name);
-            }
-          }
+          pmap<InternalName *, pset<CPT(TransformState)> > name_transforms;
 
           const TexMatrixAttrib *tma =
             DCAST(TexMatrixAttrib, geom_attribs._tex_matrix);
-
-          CPT(TexMatrixAttrib) new_tma = DCAST(TexMatrixAttrib, TexMatrixAttrib::make());
-
           int num_stages = tma->get_num_stages();
           for (int i = 0; i < num_stages; i++) {
             TextureStage *stage = tma->get_stage(i);
             InternalName *name = stage->get_texcoord_name();
-            if (get_name_count(name_count, name) > 1) {
+            name_transforms[name].insert(tma->get_transform(stage)->get_unique());
+          }
+
+          if (geom_attribs._texture != nullptr) {
+            // There may be stages without a TexMatrixAttrib, which implicitly
+            // use the identity transform.
+            const TextureAttrib *ta = DCAST(TextureAttrib, geom_attribs._texture);
+            int num_on_stages = ta->get_num_on_stages();
+            for (int si = 0; si < num_on_stages; si++) {
+              TextureStage *stage = ta->get_on_stage(si);
+              InternalName *name = stage->get_texcoord_name();
+              if (!tma->has_stage(stage)) {
+                name_transforms[name].insert(TransformState::make_identity());
+              }
+            }
+          }
+
+          CPT(TexMatrixAttrib) new_tma = DCAST(TexMatrixAttrib, TexMatrixAttrib::make());
+
+          for (int i = 0; i < num_stages; i++) {
+            TextureStage *stage = tma->get_stage(i);
+            InternalName *name = stage->get_texcoord_name();
+            auto it = name_transforms.find(name);
+            if (it == name_transforms.end()) {
+              // Already processed this set.
+            }
+            else if (it->second.size() != 1) {
               // We can't transform these texcoords, since the name is used by
               // more than one active stage.
               new_tma = DCAST(TexMatrixAttrib, new_tma->add_stage(stage, tma->get_transform(stage)));
-
-            } else {
+            }
+            else {
               // It's safe to transform these texcoords; the name is used by
-              // no more than one active stage.
-              if (transformer.transform_texcoords(new_geom, name, name, tma->get_mat(stage))) {
+              // no more than one active stage, or all these stages have the
+              // same transform.
+              const TransformState *transform = *(it->second.begin());
+              if (!transform->is_identity() &&
+                  transformer.transform_texcoords(new_geom, name, name, transform->get_mat())) {
                 any_changed = true;
               }
+              // Now make sure we don't transform this set more than once.
+              name_transforms.erase(it);
             }
           }