ODB: cuboid shape for dbMarker#9516
ODB: cuboid shape for dbMarker#9516osamahammad21 wants to merge 3 commits intoThe-OpenROAD-Project:masterfrom
Conversation
Signed-off-by: Osama <osama21@aucegypt.edu>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully introduces the Cuboid shape type for dbMarker, which is essential for representing 3D violations in the 3D viewer. The implementation covers geometry definitions, database serialization, and GUI rendering. However, I identified a critical issue regarding the JSON/PTree serialization of markers, where the new Cuboid shape is not handled, which will lead to crashes during report generation or loading. I have also suggested a minor optimization in the 3Dblox checker, aligning with best practices for loop efficiency.
| case _dbMarker::ShapeType::kCuboid: { | ||
| Cuboid c; | ||
| stream >> c; | ||
| obj.shapes_.emplace_back(c); | ||
| break; | ||
| } |
There was a problem hiding this comment.
The populatePTree and fromPTree methods in this file (lines 275 and 382 in the full file) also need to be updated to handle the Cuboid shape type. Currently, populatePTree has a default else block that assumes a Polygon, which will cause a std::bad_variant_access exception when encountering a Cuboid. Similarly, fromPTree will fail to reconstruct markers containing cuboids from JSON/TR reports.
There was a problem hiding this comment.
True but 3dblox violations are not yet handled in that scope altogether. I intent to address this in a separate PR.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: Osama <osama21@aucegypt.edu>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
After approving I realized you need to rev the db schema to prevent an old version from loading an odb with a marker it can't understand. |
How would this change affect backward compatibility? |
Signed-off-by: Osama <osama21@aucegypt.edu>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@maliberty _dbMarker::ShapeType is serialized as a full int (same numeric values 0–3 for existing shapes). Old dbs only ever contain 0–3; the changed dbIStream still handles those and adds 4 (kCuboid). No format or size change, so no backward compatibility needed. |
|
Not so much backward compatibility but as catching
A schema rev should give an error on the second step. |
2 participants
Introduce Cuboid as a shape type for dbMarker to represent 3d violations needed by the 3dviewer.