Trapezoidal Decomposition Prototype for Discussion #658

tigercosmos · 2025-12-26T14:12:35Z

The PR is a prototype of trapezoidal decomposition for the further support of polygon operations.

tigercosmos

@yungyuc Could you take a look and discuss if the current PR approach is correct? I want to add trapezoidal decomposition with further support of polygon boolean operations. However, after studying a while, I am still confusing what to do next.

tigercosmos · 2025-12-26T14:13:26Z

cpp/modmesh/universe/polygon.hpp

+    struct Trapezoid
+    {
+        value_type top_y; ///< Y-coordinate of the top horizontal edge
+        value_type bottom_y; ///< Y-coordinate of the bottom horizontal edge
+        segment_type left_edge; ///< Left boundary segment of the trapezoid
+        segment_type right_edge; ///< Right boundary segment of the trapezoid
+        size_t source_polygon; ///< ID of the polygon this trapezoid came from
+    };


Struct for trapazoid

Trapezoid should be taken outside to be a normal class (with the name Trapezoid3d, class declaration, private data with prefix, and accessors). It will be so useful that you may just create TrapezoidPad using this PR.

Got it. Thanks.

tigercosmos · 2025-12-26T14:16:36Z

cpp/modmesh/universe/polygon.hpp

+     * @throws std::out_of_range if polygon_id is invalid
+     * @throws std::invalid_argument if polygon has fewer than 3 nodes
+     */
+    std::vector<Trapezoid> const & decompose_polygon(size_t polygon_id);


The API for trapazoidal decomposition.

Create a helper class TrapezoidalDecomposition (or a more accurate name you like), and rename PolygonPad::decompose_polygon() to PolygonPad::decompose_to_trapezoid() for accurate semantics. Call the helper class from decompose_to_trapezoid().

tigercosmos · 2025-12-26T14:18:01Z

cpp/modmesh/universe/polygon.hpp

+     * @param p2 Second polygon
+     * @return Vector of polygons forming the union
+     */
+    std::vector<polygon_type> boolean_union(polygon_type const & p1, polygon_type const & p2);


The part I am not sure. Do we want to have operation APIs in the PolygonPad? The operation is between polygon and polygon, or polygon pad or polygon pad?

Good point. Area Boolean is set Boolean, so it operates between sets, so PolygonPad and PolygonPad.

Our goal is volumetric Boolean, but we need to make the area Boolean very clear before moving forward.

Create helper classes AreaBooleanUnion, AreaBooleanIntersection, and AreaBooleanDifference for the 3 operations you are prototyping here.

Use naive implementation at this point. Add unit tests to make sure they have Python interface and do not crash. Leave TODO comments in the tests saying results are not confirmed to be correct and the correctness is left for a follow-up PR.

This is good progress.

I see. Thanks!

yungyuc

Did you use any AI tool to help? I am curious about it. If you did please specify.

This is good progress. Points to address:

Make a normal (non-nested) Trapezoid3d and TrapezoidPad.
Do not use hash table and do not use hash table to vectors.
Create a helper class TrapezoidalDecomposition (or a more accurate name you like).
Rename PolygonPad::decompose_polygon() to PolygonPad::decompose_to_trapezoid() for accurate semantics.
Create helper classes AreaBooleanUnion, AreaBooleanIntersection, and AreaBooleanDifference for the 3 operations.
For the area Boolean, add unit tests to make sure they have Python interface and do not crash. Leave TODO comments in the tests saying results are not confirmed to be correct and the correctness is left for a follow-up PR.

yungyuc · 2025-12-27T01:38:43Z

cpp/modmesh/universe/polygon.hpp

+    struct Trapezoid
+    {
+        value_type top_y; ///< Y-coordinate of the top horizontal edge
+        value_type bottom_y; ///< Y-coordinate of the bottom horizontal edge
+        segment_type left_edge; ///< Left boundary segment of the trapezoid
+        segment_type right_edge; ///< Right boundary segment of the trapezoid
+        size_t source_polygon; ///< ID of the polygon this trapezoid came from
+    };


Trapezoid should be taken outside to be a normal class (with the name Trapezoid3d, class declaration, private data with prefix, and accessors). It will be so useful that you may just create TrapezoidPad using this PR.

yungyuc · 2025-12-27T01:40:55Z

cpp/modmesh/universe/polygon.hpp

-    //  - Point location query structure
-    //  - Integration with node lists for synchronization
+    /// Cache of trapezoidal decompositions, keyed by polygon ID. Computed lazily on first access.
+    std::unordered_map<size_t, std::vector<Trapezoid>> m_decompositions;


Do not use hash table. Hash table is the last resort when (1) you run out of ideas or (2) you run out of time.

A has table to vector is even worth: tons of dynamic memory allocation. It is bound to be slow.

This is why you need TrapezoidPad.

yungyuc · 2025-12-27T01:45:02Z

cpp/modmesh/universe/polygon.hpp

+     * @throws std::out_of_range if polygon_id is invalid
+     * @throws std::invalid_argument if polygon has fewer than 3 nodes
+     */
+    std::vector<Trapezoid> const & decompose_polygon(size_t polygon_id);


Create a helper class TrapezoidalDecomposition (or a more accurate name you like), and rename PolygonPad::decompose_polygon() to PolygonPad::decompose_to_trapezoid() for accurate semantics. Call the helper class from decompose_to_trapezoid().

yungyuc · 2025-12-27T01:48:36Z

cpp/modmesh/universe/polygon.hpp

+     * @param p2 Second polygon
+     * @return Vector of polygons forming the union
+     */
+    std::vector<polygon_type> boolean_union(polygon_type const & p1, polygon_type const & p2);


Good point. Area Boolean is set Boolean, so it operates between sets, so PolygonPad and PolygonPad.

Our goal is volumetric Boolean, but we need to make the area Boolean very clear before moving forward.

yungyuc · 2025-12-27T01:51:28Z

cpp/modmesh/universe/polygon.hpp

+     * @param p2 Second polygon
+     * @return Vector of polygons forming the union
+     */
+    std::vector<polygon_type> boolean_union(polygon_type const & p1, polygon_type const & p2);


Create helper classes AreaBooleanUnion, AreaBooleanIntersection, and AreaBooleanDifference for the 3 operations you are prototyping here.

Use naive implementation at this point. Add unit tests to make sure they have Python interface and do not crash. Leave TODO comments in the tests saying results are not confirmed to be correct and the correctness is left for a follow-up PR.

This is good progress.

tigercosmos · 2025-12-31T13:41:40Z

@yungyuc Yes, I used AI and spent around 5–6 hours exploring how to implement the trapezoidal decomposition. In the end, I wasn’t happy with the outcome and wasn’t confident about the design direction. I removed most of the implementation, kept what seemed to be the most reasonable interface, and opened this PR for discussion.

yungyuc · 2026-01-01T03:24:19Z

@yungyuc Yes, I used AI and spent around 5–6 hours exploring how to implement the trapezoidal decomposition. In the end, I wasn’t happy with the outcome and wasn’t confident about the design direction. I removed most of the implementation, kept what seemed to be the most reasonable interface, and opened this PR for discussion.

6 hours on the trapezoidal algorithm are not too many, but spending time to study it and then prototype incrementally will be more productive. Dividing tasks is an art that we cannot count on tools already.

tigercosmos · 2026-01-07T14:57:32Z

cpp/modmesh/universe/polygon.hpp

    }

-    TrianglePad(
+    TrianglePad(


GitHub's diff is weird here. There is new added Trapezoid3d and TrapezoidPad.

Since this is a large diff, the diff issue needs to be fixed. The messy diff stops reasonable tracking in the future.

Please reorder the new code to make diff looks good. Use a follow-up refactoring PR to reorder after this one.

tigercosmos · 2026-01-07T15:05:27Z

tests/test_polygon3d.py

        self.assertFalse(polygon1.is_same(polygon2))
        self.assertTrue(polygon1.is_not_same(polygon2))

+    # TODO: Verify and implement boolean operations once the C++ side is complete. # noqa: E501


Put TODO for now and only verify the interface.

Do not waive flake8 E501 for comments other than URL. Fix the comment length instead.

tigercosmos · 2026-01-07T15:08:20Z

cpp/modmesh/universe/polygon.hpp

+    std::shared_ptr<trapezoid_pad_type> m_trapezoids;
+    std::vector<ssize_type> m_begins;
+    std::vector<ssize_type> m_ends;


No hash table now.

tigercosmos · 2026-01-07T15:08:41Z

cpp/modmesh/universe/polygon.hpp

+ * @tparam T floating-point type
+ */
+template <typename T>
+class AreaBooleanUnion


Helper classes as requsted.

tigercosmos · 2026-01-07T15:09:27Z

cpp/modmesh/universe/polygon.hpp

+     * @throws std::out_of_range if polygon_id is invalid
+     * @throws std::invalid_argument if polygon has fewer than 3 nodes
+     */
+    std::pair<size_t, size_t> decompose_to_trapezoid(size_t polygon_id);


Fixed the naming.

tigercosmos · 2026-01-07T15:15:15Z

cpp/modmesh/universe/polygon.hpp

 }

+template <typename T>
+std::pair<size_t, size_t> PolygonPad<T>::decompose_to_trapezoid(size_t polygon_id)


Get the index range of the trapazoids in the pad after decomposition.

tigercosmos · 2026-01-07T15:16:05Z

@yungyuc the aforementioned issues are addressed. Please help take a look, thanks!

yungyuc

Good progress. Points to address:

Reorder the new code to make diff looks good.
Rename TrapezoidalDecomposer::Trapezoid to something very different from Trapezoid3d or Trapezoid and make it private.
Reduce memory footprint for coordinates for TrapezoidalDecomposer::YTrap.
Return PolygonPad from AreaBooleanUnion::compute, AreaBooleanIntersection::compute, AreaBooleanDifference::compute.

yungyuc · 2026-01-08T10:46:02Z

cpp/modmesh/universe/polygon.hpp

    }

-    TrianglePad(
+    TrianglePad(


Since this is a large diff, the diff issue needs to be fixed. The messy diff stops reasonable tracking in the future.

Please reorder the new code to make diff looks good. Use a follow-up refactoring PR to reorder after this one.

yungyuc · 2026-01-08T11:05:41Z

cpp/modmesh/universe/polygon.hpp

+ * @tparam T floating-point type
+ */
+template <typename T>
+class Trapezoid3d


This (line 813) is the new Trapezoid3d. Add inline annotation close to the line to make discussions clear.

yungyuc · 2026-01-08T11:07:16Z

cpp/modmesh/universe/polygon.hpp

        SimpleArray<T> const & y1,
        SimpleArray<T> const & x2,
        SimpleArray<T> const & y2,
+        SimpleArray<T> const & x3,


Diff algorithm has a hard time knowing that you are inserting a block of new code. So we need to choose a better location for the new code to make diff happy. Things like this happen, though inconvenience.

yungyuc · 2026-01-08T11:09:54Z

cpp/modmesh/universe/polygon.hpp

+     * In polygon decomposition, vertical lines are extended from each vertex to partition
+     * the polygon into trapezoids.
+     */
+    struct Trapezoid


It's confusing to have the inner class Trapezoid while there is a standalone Trapezoid3d. Rename this internal class name and make it private.

I suggest a name like YTrap because your scanlines are perpendicular to the Y-axis.

yungyuc · 2026-01-08T11:18:01Z

cpp/modmesh/universe/polygon.hpp

+     */
+    struct Trapezoid
+    {
+        value_type top_y;


You are using scanlines perpendicular to the Y-axis and should take advantage of that constraint. You do not need flexibility at the moment. (You will in the future, but do not over-engineer.)

Use real number for member data. Don't use points.

value_type lower_y, upper_y; value_type lower_x0, lower_x1, upper_x0, upper_x1; value_type lower_z0, lower_z1, upper_z0, upper_z1;

It takes only 10 real numbers for the trapezoid whose lower and upper edges are perpendicular to the Y-axis. Your old struct takes 14 real numbers. Don't be so wasteful.

value_type top_y; value_type bottom_y; segment_type left_edge; segment_type right_edge;

I usually see lower-left, lower-right, upper-left, and upper-right in geometry code. Let's use the convention and avoid top and bottom.

yungyuc · 2026-01-08T11:28:11Z

cpp/modmesh/universe/polygon.hpp

+    /**
+     * Trapezoid structure for decomposition.
+     *
+     * A trapezoid is a quadrilateral with two parallel horizontal edges (top and bottom).


Remove the trivial statement to explain trapezoid. People not understanding trapezoid cannot contribute.

Add descriptions for the fact that the struct is for traps whose bottom and top lines perpendicular to Y-axis. It's the most important point to understand the algorithm.

yungyuc · 2026-01-08T11:34:10Z

cpp/modmesh/universe/polygon.hpp

+     * @param polygon_id2 ID of second polygon
+     * @return Vector of polygons forming the union
+     */
+    std::vector<Polygon3d<T>> compute(polygon_pad_type & pad, size_t polygon_id1, size_t polygon_id2);


Return PolygonPad from AreaBoolean*::compute.

tigercosmos · 2026-01-16T16:36:46Z

@yungyuc I addressed the aforementioned issues. Cloud you take a look again?

yungyuc

Only cosmetic issues:

Some overload signature of TrapezoidPad::set() and TrapezoidPad::set_at() is too long. Break them with proper indentation. (Leave an inline annotation at the improved line to remind me to review.)
Do not waive flake8 E501 for comments other than URL. Fix the comment length instead.

yungyuc · 2026-01-19T01:04:28Z

cpp/modmesh/universe/polygon.hpp

+        x3_at(i) = x3;
+        y3_at(i) = y3;
+    }
+    void set_at(size_t i, value_type x0, value_type y0, value_type z0, value_type x1, value_type y1, value_type z1, value_type x2, value_type y2, value_type z2, value_type x3, value_type y3, value_type z3)


This line is too long. Break it with proper indent.

yungyuc · 2026-01-19T01:05:58Z

cpp/modmesh/universe/polygon.hpp

+            z3(i) = p3.z();
+        }
+    }
+    void set(size_t i, value_type x0_value, value_type y0_value, value_type x1_value, value_type y1_value, value_type x2_value, value_type y2_value, value_type x3_value, value_type y3_value)


This line is too long. Break it with proper indent.

yungyuc · 2026-01-19T01:06:02Z

cpp/modmesh/universe/polygon.hpp

+        x3(i) = x3_value;
+        y3(i) = y3_value;
+    }
+    void set(size_t i, value_type x0_value, value_type y0_value, value_type z0_value, value_type x1_value, value_type y1_value, value_type z1_value, value_type x2_value, value_type y2_value, value_type z2_value, value_type x3_value, value_type y3_value, value_type z3_value)


This line is too long. Break it with proper indent.

yungyuc · 2026-01-19T01:11:07Z

tests/test_polygon3d.py

        self.assertFalse(polygon1.is_same(polygon2))
        self.assertTrue(polygon1.is_not_same(polygon2))

+    # TODO: Verify and implement boolean operations once the C++ side is complete. # noqa: E501


Do not waive flake8 E501 for comments other than URL. Fix the comment length instead.

tigercosmos · 2026-01-19T16:20:39Z

@yungyuc the issues are fixed in ac56bae, please take a look.

yungyuc

The PR is a prototype of trapezoidal decomposition for the further support of polygon operations.

Write informative summary in the PR first comment and commit log. This PR added more than 1.4k lines of code and deserve a good summaries.

And consider to review the code formatting again. There is at least one defect but could be more.

yungyuc · 2026-01-20T14:24:14Z

tests/test_polygon3d.py

+            pass
+    # TODO: Verify and implement boolean operations
+    # once the C++ side is complete.
+


The blank line is misplaced. You can fix it in this PR or in a later one.

tigercosmos marked this pull request as draft December 26, 2025 14:12

tigercosmos commented Dec 26, 2025

View reviewed changes

yungyuc assigned tigercosmos Dec 27, 2025

yungyuc added the geometry Geometry entities and manipulation label Dec 27, 2025

yungyuc requested changes Dec 27, 2025

View reviewed changes

tigercosmos commented Jan 7, 2026

View reviewed changes

tigercosmos changed the title ~~[WIP] Trapezoidal Decomposition Prototype for Discussion~~ Trapezoidal Decomposition Prototype for Discussion Jan 7, 2026

tigercosmos marked this pull request as ready for review January 7, 2026 15:15

yungyuc requested changes Jan 8, 2026

View reviewed changes

tigercosmos force-pushed the trapoziod_1226 branch from 41a1e4c to a4317a4 Compare January 16, 2026 16:27

The interface defination of polygon boolean operation

8e85e92

tigercosmos force-pushed the trapoziod_1226 branch from a4317a4 to 8e85e92 Compare January 16, 2026 16:27

tigercosmos requested a review from yungyuc January 18, 2026 09:57

yungyuc requested changes Jan 19, 2026

View reviewed changes

tigercosmos force-pushed the trapoziod_1226 branch from 469d230 to d8fb45d Compare January 19, 2026 16:38

Fix the format issues

ac56bae

tigercosmos force-pushed the trapoziod_1226 branch from d8fb45d to ac56bae Compare January 19, 2026 16:43

yungyuc requested changes Jan 20, 2026

View reviewed changes

Trapezoidal Decomposition Prototype for Discussion #658

Are you sure you want to change the base?

Trapezoidal Decomposition Prototype for Discussion #658

Uh oh!

Conversation

tigercosmos commented Dec 26, 2025

Uh oh!

tigercosmos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungyuc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tigercosmos commented Dec 31, 2025

Uh oh!

yungyuc commented Jan 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tigercosmos commented Jan 7, 2026

Uh oh!

yungyuc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungyuc left a comment •

edited

Loading

yungyuc left a comment •

edited

Loading

yungyuc left a comment •

edited

Loading

tigercosmos commented Jan 19, 2026 •

edited

Loading