Maniphest T96310

Geometry Nodes: Collection Info alphabetic sorting of instances not correct
Closed, DuplicateBUG
Actions

Assigned To
None
Authored By
Simen Musæus (simen)
Mar 10 2022, 8:06 PM
Tags
  • BF Blender
  • Geometry Nodes
  • Nodes & Physics
  • Platform: macOS
Subscribers
Germano Cavalcante (mano-wii)
Omar Emara (OmarSquircleArt)
Philipp Oeser (lichtwerk)
Simen Musæus (simen)

Description

System Information
Operating system: macOS-11.6-x86_64-i386-64bit 64 Bits
Graphics card: NVIDIA GeForce GT 750M OpenGL Engine NVIDIA Corporation 4.1 NVIDIA-16.0.13

Blender Version
Broken: version: 3.1.0, branch: master, commit date: 2022-03-08 18:16, hash: rBc77597cd0e15
Worked: (newest version of Blender that worked as expected)

Short description of error
The Collection Info node does not sort instances alphabetically when Separate Children is checked.

Exact steps for others to reproduce the error

  1. Open the attached blender file.
  2. Select the "Boards/Ke Jie Match 3 27-05-2017" object with Geometry Nodes mode open.
  3. In the spreadsheet viewer select "Instances" and scroll to index 27-28. There the object move.27 an move.28 are swapped (not the expected alphabetic order).

The attach file is simplified, in the actual projectfile this the occured several times throughout the instance sequence. In the Collection pane they are ordered as expect alphabetically.

The issue seems to be local in that it only swaps neighbouring sequence positions, there was no positions being completely off.

sorting bug.blend3 MBDownload

Related Objects

Event Timeline

Simen Musæus (simen) renamed this task from Geometry Nodes: Collection Info to Geometry Nodes: Collection Info alphabetic sorting of instances not correct.Mar 10 2022, 8:06 PM
Simen Musæus (simen) created this task.
Omar Emara (OmarSquircleArt) changed the task status from Needs Triage to Needs Information from User.Mar 11 2022, 8:53 AM
Omar Emara (OmarSquircleArt) added a subscriber: Omar Emara (OmarSquircleArt).

I can't replicate this issue. Is the wrong sorting you get stable? Meaning, if you reopen the file multiple times, change couple of things, are the elements that are switched always the same, or do they change?

Simen Musæus (simen) added a comment.Mar 13 2022, 11:56 AM

Yes it's stable, even when changing the name pattern of the instance sequence to include another prefix.

Omar Emara (OmarSquircleArt) changed the task status from Needs Information from User to Needs Triage.Mar 13 2022, 4:26 PM
Philipp Oeser (lichtwerk) added subscribers: Germano Cavalcante (mano-wii), Philipp Oeser (lichtwerk).Mar 21 2022, 11:32 AM

I also cannot reproduce this, @Germano Cavalcante (mano-wii) can you on MAC?

Germano Cavalcante (mano-wii) changed the task status from Needs Triage to Confirmed.Mar 21 2022, 9:51 PM
Germano Cavalcante (mano-wii) changed the subtype of this task from "Report" to "Bug".
Germano Cavalcante (mano-wii) added projects: Geometry Nodes, Nodes & Physics, Platform: macOS.

I can confirm on a Mac.
The problem seems to be with std::sort, but I couldn't pinpoint the exact cause.
Replacing sdt::sort with qsort seems to solve the problem:

diff --git a/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc b/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc
index 68d42444afd..4c7a4a32148 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc
@@ -48,6 +48,11 @@ struct InstanceListEntry {
   float4x4 transform;
 };
 
+static int cmp_fn(const void *a, const void *b)
+{
+  return BLI_strcasecmp_natural(((InstanceListEntry *)a)->name, ((InstanceListEntry *)b)->name);
+};
+
 static void node_geo_exec(GeoNodeExecParams params)
 {
   Collection *collection = params.get_input<Collection *>("Collection");
@@ -117,11 +122,8 @@ static void node_geo_exec(GeoNodeExecParams params)
       entries.append({handle, &(child_object->id.name[2]), transform});
     }
 
-    std::sort(entries.begin(),
-              entries.end(),
-              [](const InstanceListEntry &a, const InstanceListEntry &b) {
-                return BLI_strcasecmp_natural(a.name, b.name) <= 0;
-              });
+    qsort(entries.data(), entries.size(), sizeof(InstanceListEntry), cmp_fn);
+
     for (const InstanceListEntry &entry : entries) {
       instances.add_instance(entry.handle, entry.transform);
     }
Germano Cavalcante (mano-wii) added a comment.Mar 21 2022, 9:58 PM

After a little more testing the problem seems to be in using less than or equal instead of less than:

diff --git a/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc b/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc
index 68d42444afd..54a061993a3 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_collection_info.cc
@@ -120,7 +120,7 @@ static void node_geo_exec(GeoNodeExecParams params)
     std::sort(entries.begin(),
               entries.end(),
               [](const InstanceListEntry &a, const InstanceListEntry &b) {
-                return BLI_strcasecmp_natural(a.name, b.name) <= 0;
+                return BLI_strcasecmp_natural(a.name, b.name) < 0;
               });
     for (const InstanceListEntry &entry : entries) {
       instances.add_instance(entry.handle, entry.transform);
Ankit Meel (ankitm) closed this task as a duplicate of T93285: Geometry nodes: Wrong instance order when instancing a collection with more than 30 objects.Mar 22 2022, 6:33 AM
Germano Cavalcante (mano-wii) mentioned this in T93285: Geometry nodes: Wrong instance order when instancing a collection with more than 30 objects.Apr 27 2022, 3:52 PM