Selaa lähdekoodia

[cpp] Fix C++ clipping triangle skipping and control flow bugs

- Remove erroneous i+=3 increment in SkeletonClipping.cpp that caused triangle skipping
- Add missing continue statement to match Java reference implementation behavior
- Portal animation now renders correctly with proper triangular clipping
Mario Zechner 2 kuukautta sitten
vanhempi
commit
c7af1a3438

+ 1 - 1
spine-cpp/src/spine/SkeletonClipping.cpp

@@ -246,7 +246,6 @@ bool SkeletonClipping::clipTriangles(float *vertices, unsigned short *triangles,
 				clippedTriangles[s + 1] = (unsigned short) (index + 1);
 				clippedTriangles[s + 2] = (unsigned short) (index + 2);
 				index += 3;
-				i += 3;
 				break;
 			}
 		}
@@ -321,6 +320,7 @@ bool SkeletonClipping::clip(float x1, float y1, float x2, float y2, float x3, fl
 				} else {
 					output->add(inputX2);
 					output->add(inputY2);
+					continue;
 				}
 			} else if (s2) {// v1 outside, v2 inside
 				float ix = inputX2 - inputX, iy = inputY2 - inputY, t = s1 / (ix * ey - iy * ex);

+ 2 - 3
spine-glfw/example/main.cpp

@@ -87,9 +87,8 @@ int main() {
 	AnimationStateData animationStateData(skeletonData);
 	animationStateData.setDefaultMix(0.2f);
 	AnimationState animationState(&animationStateData);
-	// animationState.setAnimation(0, "portal", true);
-	// animationState.addAnimation(0, "run", true, 0);
-	animationState.setAnimation(0, "idle", true);
+	animationState.setAnimation(0, "portal", true);
+	animationState.addAnimation(0, "run", true, 0);
 
 	// Create the renderer and set the viewport size to match the window size. This sets up a
 	// pixel perfect orthogonal projection for 2D rendering.

+ 118 - 0
todos/done/2025-01-21-10-06-07-cpp-clipping-broken-analysis.md

@@ -0,0 +1,118 @@
+Now I'll perform a detailed comparison of the clipping implementations between the Java reference and C++ port, focusing on the `clipTriangles` methods and other critical functions.
+
+## Detailed Comparison: spine-libgdx vs spine-cpp SkeletonClipping
+
+After analyzing both implementations line by line, I've identified several significant differences and bugs in the C++ port. Here's my detailed analysis:
+
+### **CRITICAL BUG #1: Loop Index Increment Error**
+
+**Location**: C++ implementation, line 249 in `clipTriangles` with UVs
+```cpp
+// C++ (line 249) - INCORRECT
+i += 3;  // This increments i by an additional 3!
+break;
+```
+
+**Java reference** (lines 268, 358):
+```java
+// Java - CORRECT, no additional increment
+index += 3;
+break;
+```
+
+**Impact**: This bug causes the C++ implementation to skip triangles when clipping fails. The outer loop already increments `i += 3`, so this additional increment makes it skip every other triangle group, leading to incomplete clipping results.
+
+### **BUG #2: Missing Continue Statement**
+
+**Location**: C++ implementation, lines 322-324, 332-336
+```cpp
+// C++ - Missing 'continue' after adding vertices in valid intersection case
+if (t >= 0 && t <= 1) {
+    output->add(inputX + ix * t);
+    output->add(inputY + iy * t);
+} else {
+    output->add(inputX2);
+    output->add(inputY2);
+    // Missing continue here!
+}
+```
+
+**Java reference** (lines 414-418, 426-430):
+```java
+// Java - Has continue statements
+if (t >= 0 && t <= 1) {
+    output.add(inputX + ix * t);
+    output.add(inputY + iy * t);
+} else {
+    output.add(inputX2);
+    output.add(inputY2);
+    continue;  // Present in Java
+}
+```
+
+**Impact**: Without the `continue` statements, the C++ code will always set `clipped = true` even when the intersection calculation fails, potentially affecting the algorithm's correctness.
+
+### **DIFFERENCE #3: Return Value Logic**
+
+**Java implementation** (line 155):
+```java
+return clipOutputItems != null;
+```
+
+**C++ implementation** (line 158):
+```cpp
+return clipped;
+```
+
+**Analysis**: The Java version tracks whether any clipping output was generated (`clipOutputItems != null`), while C++ tracks whether any clipping occurred (`clipped`). These are subtly different conditions that could lead to different behavior.
+
+### **DIFFERENCE #4: Additional Safety Check**
+
+**C++ implementation** has an additional safety check (lines 366-369):
+```cpp
+if (originalOutput->size() < 6) {
+    originalOutput->clear();
+    return false;
+}
+```
+
+This check ensures a minimum polygon size, which is absent in the Java version.
+
+### **DIFFERENCE #5: Data Type Differences**
+
+- **Indices**: Java uses `short` vs C++ uses `unsigned short`
+- **Loop counters**: Java uses `int` vs C++ uses `size_t`
+- **Array access**: Java has bounds checking vs C++ direct array access
+
+### **DIFFERENCE #6: Method Signature Variations**
+
+The C++ implementation has additional overloaded methods:
+- `clipTriangles(Array<float> &vertices, Array<unsigned short> &triangles, Array<float> &uvs, size_t stride)`
+- Different parameter types and organization
+
+### **DIFFERENCE #7: UV Coordinate Handling**
+
+**Java implementation**: Has separate methods for different UV handling scenarios, with complex stride logic in the second `clipTriangles` method.
+
+**C++ implementation**: Simplified UV handling with dedicated `_clippedUVs` array that's always populated when UVs are provided.
+
+### **Minor Differences**
+
+1. **Variable naming**: Java uses camelCase (`clipOutput`) vs C++ uses underscore prefix (`_clipOutput`)
+2. **Memory management**: Java relies on GC vs C++ manual array management
+3. **Array operations**: Different API calls for array manipulation
+
+## Summary
+
+The most critical issues in the C++ port are:
+
+1. **Triangle skipping bug** (line 249) - causes incomplete processing
+2. **Missing continue statements** - affects clipping logic correctness
+3. **Different return value semantics** - may cause behavioral differences
+
+The C++ implementation appears to be a reasonable port overall, but these bugs need to be fixed to ensure compatibility with the Java reference implementation. The triangle skipping bug is particularly serious as it would cause visible rendering artifacts.
+
+**Files analyzed:**
+- `/Users/badlogic/workspaces/spine-runtimes/spine-libgdx/spine-libgdx/src/com/esotericsoftware/spine/utils/SkeletonClipping.java`
+- `/Users/badlogic/workspaces/spine-runtimes/spine-cpp/src/spine/SkeletonClipping.cpp`
+- `/Users/badlogic/workspaces/spine-runtimes/spine-cpp/include/spine/SkeletonClipping.h`

+ 21 - 0
todos/done/2025-01-21-10-06-07-cpp-clipping-broken.md

@@ -0,0 +1,21 @@
+# C++ Clipping Appears Broken
+**Status:** Done
+**Agent PID:** 7713
+
+## Original Todo
+c++ clipping appears broken, try with portal animation in spine-glfw
+
+## Description
+Fix critical bugs in the C++ clipping implementation that cause triangles to be skipped during clipping operations. The analysis reveals that the C++ port of the Java reference implementation has several bugs, most notably an erroneous loop increment that causes incomplete triangle processing during clipping.
+
+## Implementation Plan
+- [x] Fix triangle skipping bug: Remove line 249 `i += 3;` from spine-cpp/src/spine/SkeletonClipping.cpp in clipTriangles method (keep `index += 3; break;` only)
+- [x] Analyze Java labeled continue statements: Compare spine-libgdx/spine-libgdx/src/com/esotericsoftware/spine/utils/SkeletonClipping.java lines 414-430 with C++ lines 322-336 to determine correct control flow
+- [x] Fix polygon clipping control flow: Modify spine-cpp/src/spine/SkeletonClipping.cpp lines 322-336 to properly handle intersection cases (replace missing continue logic with appropriate break/goto)
+- [x] Build spine-cpp: Run `cd spine-cpp && cmake -B build && cmake --build build` to verify compilation
+- [x] Enable portal test: Uncomment line 90 in spine-glfw/example/main.cpp: `animationState.setAnimation(0, "portal", true);` (already enabled)
+- [x] Build spine-glfw: Run `cd spine-glfw && cmake -B build && cmake --build build` to compile test
+- [x] User test: Run spine-glfw example and verify portal animation shows correct triangular clipping (no missing/skipped triangles)
+
+## Notes
+[Implementation notes]

+ 0 - 1
todos/todos.md

@@ -1,4 +1,3 @@
-- c++ clipping appears broken, try with portal animation in spine-glfw
 - spine-c/codegen type extractor should also report typedefs like typedef long long PropertyId; so primitive type to some name, and we need to handle that in the codegen
 - Generate language bindings in spine-c/codegen
     - Use CClassOrStruct, CEnum that get generated from spine-cpp-types.json and generate