Add drawing shape interface to World#712
Conversation
tigercosmos
left a comment
There was a problem hiding this comment.
Key design points highlighted inline.
09422f7 to
a7742d6
Compare
|
@yungyuc Please take a look. Thank you. |
There was a problem hiding this comment.
This does not look right. World is the canvas. It is not necessary to have another class inside World for creating geometry objects.
And it is called World so that the geometry objects do not have to be in a surface, but can be in a space. The World is part of the universe module. While we do not have a Universe container yet, it may be a future extension to hold multiple World.
And it is not correct to postfix the names to the geometry entities with 2D (although d should be lower-cased per the naming convention, it is not the point). Polygons, rectangles, triangles, circles, polylines, ellipses are all 2D entities.
The 3d postfix we have for Point, Segment, etc., meant the entities are in 3D space.
I am moving this PR to draft for you to follow up.
a7742d6 to
a56c683
Compare
4e8f208 to
4a5b559
Compare
| * | ||
| * All concrete shapes must implement the pure-virtual methods below. Shapes | ||
| * live on the z = 0 plane; Point3d is used for compatibility with the | ||
| * existing rendering pipeline. |
There was a problem hiding this comment.
Shape base class: Abstract base for all shapes managed by World. Concrete shapes implement draw(World&) to emit segments, plus translate/scale/rotate for the Update part of CRUD (transforms are called on the shape object, not through World).
| // -- shapes ------------------------------------------------------------- | ||
|
|
||
| /// Add a pre-built shape. Returns the assigned ID. | ||
| int32_t add_shape(shape_ptr shape) |
There was a problem hiding this comment.
World shape CRUD: Create — add_shape() and typed helpers (add_rectangle, add_circle, etc.) return an int32_t ID. Read — get_shape(id), nshape. Update — via shape methods (translate/scale/rotate). Delete — remove_shape(id), clear_shapes(). Storage is unordered_map<int32_t, shape_ptr> with sequential IDs for O(1) average lookup.
|
|
||
| template <typename T> | ||
| class MODMESH_PYTHON_WRAPPER_VISIBILITY WrapPolygon | ||
| : public WrapBase<WrapPolygon<T>, Polygon<T>, std::shared_ptr<Polygon<T>>, Shape<T>> |
There was a problem hiding this comment.
pybind11 inheritance: Concrete shape wrappers pass Shape<T> as the 4th WrapBase template arg so pybind11 registers the C++ inheritance hierarchy. This enables polymorphic returns from get_shape() and correct isinstance() checks in Python.
| 'WorldFp32', | ||
| 'WorldFp64', | ||
| 'ShapeFp32', | ||
| 'ShapeFp64', |
There was a problem hiding this comment.
Shape types exported here so they are accessible as modmesh.ShapeFp64, modmesh.RectangleFp64, etc.
4a5b559 to
516884a
Compare
Integrate shape management directly into World instead of a separate Canvas2D wrapper, per review feedback. Shape classes (Polygon, Rectangle, Triangle, Circle, Line, Polyline, Ellipse) are defined in World.hpp without the "2D" suffix — these entities are inherently 2D. World gains add_*/get_shape/remove_shape/ clear_shapes methods for shape CRUD. All method bodies are stubs (throw runtime_error) — this defines the interface only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
516884a to
1778bd5
Compare
|
@yungyuc I have update the interface design based on the review. Please take a look, thanks! |
Summary
Add shape CRUD management directly into
World. All method bodies are stubs (throw runtime_error) — this PR defines the interface only.Shape hierarchy (C++ templates in
World.hpp)Shape<T>is the abstract base. All shapes live on z=0 usingPoint3d<T>for rendering pipeline compatibility. Concrete shapes:CRUD on World
add_shape(),add_rectangle(),add_circle(),add_line(),add_triangle(),add_polygon(),add_polyline(),add_ellipse()— returnsint32_tshape IDget_shape(id)— by ID;shapes— all shapes;nshape— counttranslate(dx, dy),scale(factor, cx, cy),rotate(angle_deg, cx, cy)remove_shape(id),clear_shapes()Usage
Files changed
World.hppwrap_World.cppcore.pytest_pilot_drawing.pyTest plan
makebuilds successfullypre-commitpasses (clang-format, flake8, ascii, trailing whitespace)isinstance(RectangleFp64(...), ShapeFp64)isTrue)tests/test_pilot_drawing.py— 4 tests, allxfail(stubs throw, as expected)🤖 Generated with Claude Code