3D viewer violations by openroad-ci · Pull Request #9657 · The-OpenROAD-Project/OpenROAD

openroad-ci · 2026-03-05T17:27:00Z

Fix the issue :
#9412

gemini-code-assist

Code Review

This pull request introduces significant enhancements to the 3D chiplet viewer, transitioning it from a wireframe display to a more advanced solid-rendered view. Key improvements include lighting effects, back-face culling, and depth sorting using a painter's algorithm. The viewer is now more interactive, with features like zooming to selected objects and selection-focus animations. The implementation is robust, leveraging modern C++ and performance optimizations such as reusing buffers during painting. The integration with the main window's selection and highlight system is also well-executed. My review includes a couple of suggestions to replace a magic number with a named constant to improve code maintainability, aligning with repository guidelines.

gemini-code-assist · 2026-03-05T17:30:47Z

    // Color by Depth (proportional to Z)
    const QVector3D color = kColorPalette[index++ % kColorPalette.size()];

    const uint32_t base = vertices_.size();
    for (const auto& p : draw_cuboid.getPoints()) {
-      vertices_.push_back({QVector3D(p.x(), p.y(), p.z()), color});
+      vertices_.push_back({QVector3D(p.x(), p.y(), p.z() * 2.0f), color});


The Z-scaling factor 2.0f is used here as a magic number. To improve readability and maintainability, consider defining it as a named constant (e.g., constexpr float kZScale = 2.0f;) in the anonymous namespace at the top of the file. This magic number is also used on line 371.

Suggested change

vertices_.push_back({QVector3D(p.x(), p.y(), p.z() * 2.0f), color});

vertices_.push_back({QVector3D(p.x(), p.y(), p.z() * kZScale), color});

References

Define tunable parameters as named constants instead of using hardcoded magic numbers.

gemini-code-assist · 2026-03-05T17:30:47Z

+          if (c_rect.intersects(bbox)) {
+            odb::Cuboid draw_cuboid = cuboid;
+            center_transform_.apply(draw_cuboid);
+            target_z = draw_cuboid.zMax() * 2.0f + kHighlightZOffset;


The Z-scaling factor 2.0f is used here as a magic number. To improve readability and maintainability, consider using the named constant suggested for line 216.

Suggested change

target_z = draw_cuboid.zMax() * 2.0f + kHighlightZOffset;

target_z = draw_cuboid.zMax() * kZScale + kHighlightZOffset;

References

Define tunable parameters as named constants instead of using hardcoded magic numbers.

github-actions · 2026-03-05T17:52:47Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2026-03-05T18:37:33Z

clang-tidy review says "All clean, LGTM! 👍"

jorge-ferreira-pii · 2026-03-05T18:55:37Z

🎨 Visual Improvements & Rendering

Solid Chiplet Visualization: Transitioned from wireframe-only to solid-filled face rendering.

    Painter’s Algorithm: Implemented depth sorting (sorted_faces_) to ensure correct draw order (back-to-front).

    Back-Face Culling: Added logic to skip rendering faces not oriented toward the camera, preventing visual artifacts and overlapping during rotation.

Lighting & Shading: Added a simple facet-based lighting model to improve spatial perception:

    Top faces: Rendered brighter.

    Side/Bottom faces: Rendered darker to provide a clear sense of depth and volume.

Expanded Color Palette: Added Teal, Purple, and Pink to better differentiate between chiplets.

    Note: Yellow has been intentionally excluded from the random palette as it is reserved for DRC/Selection highlights.

Z-Axis Scaling: Introduced a configurable scale factor (kZScale = 2.0f) to visually exaggerate the height of chiplet structures, which are often too thin to inspect at a 1:1 ratio.

🖱️ Interaction & Navigation

Synchronized Selection & Highlighting:

    The 3D viewer is now fully synced with the Layout Viewer and DRC Viewer.

    Highlighted objects are rendered with thicker outlines and a yellow fill to match the main UI's visual language.

    Added a selectionFocus animation (pulse/flash effect) to help users quickly locate small components.

Smart Zoom & Auto-Focus:

    Implemented zoomTo(): Automatically centers the camera and adjusts the zoom level to fit the selected object.

    Top-Down Reset: When focusing on a selection, the camera rotation resets to a 2D-like top view for easier localization.

Enhanced Camera Controls:

    Improved panning logic and implemented strict zoom bounds (kMinDistanceBound, kMaxDistanceBound) to prevent the camera from clipping through geometry or "getting lost" in space.

🛠️ Technical Details & Optimization

MainWindow Integration:

    The Chiplet3DWidget now maintains active references to selected_ and highlighted_ sets from the main window.

    Connected new signals to trigger automatic updates whenever changes occur in the Inspector or DRC Viewer.

Performance Optimizations:

    Memory Management: Introduced reusable buffers (polygon_buffer_, face_view_points_) to eliminate heap allocations within the paintEvent loop, significantly reducing CPU overhead during high-FPS interaction.

osamahammad21

In general, when I tested this branch I found the following:

Zoom function is messed up because of the clamping values
The faces draw over each other in specific angles
Although hovering over the violation from the DRC widget animates one of the faces of the violating chiplets, it does not highlight the chiplet when the violation is selected.
Instead of highlighting the chiplet, you should highlight the violation cuboid (minor and could be addressed in a separate PR).
The drawn matrix changes its z coordinate with different distance_ values

osamahammad21 · 2026-03-08T22:20:47Z

@@ -303,6 +611,7 @@ void Chiplet3DWidget::wheelEvent(QWheelEvent* e)
  } else {
    distance_ *= kZoomOutFactor;
  }
+  distance_ = std::clamp(distance_, kMinDistanceBound, kMaxDistanceBound);


Is there a logical explanation behind the values of the constants kMinDistanceBound and kMaxDistanceBound? They cause a problem when the distance_ is out of these bounds.

github-actions · 2026-03-20T13:51:00Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2026-03-20T19:19:46Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

github-actions · 2026-04-08T03:18:47Z

clang-tidy review says "All clean, LGTM! 👍"

jorge-ferreira-pii · 2026-04-08T13:31:59Z

New DRC Widget (Web GUI)

Added DRCWidget panel (drc-widget.js) to the Web interface to enable loading DRC reports.

UI Features: Added support for tree-view listing by categories, item selection, and individual marker visibility control.

DB Synchronization: Implemented direct synchronization with the database (odb::dbMarkerCategory) via new WebSocket endpoints (drc_report, drc_load, drc_highlight, etc.). These are defined in request_handler.cpp and managed by DRCReport.

3D Viewer Enhancements (chiplet3DWidget.cpp/h & 3d-viewer-widget.js)

Full DRC Integration: The 3D viewer can now render violation geometries (cuboids, polygons, and rectangles) at their respective Z-axis heights.

Backend Rendering Improvements (C++): Added back-face culling and view-based depth sorting (Painter's algorithm with cycle resolution) to mitigate Z-fighting artifacts and incorrect chiplet overlapping.

UX / Camera Adjustments: >   * Added selection animations (selectionFocus) and dynamic camera focusing (zoomTo) when clicking on markers or components.

    Implemented camera rotation constraints to prevent unwanted bottom-up views.

Database Support (odb::dbDatabaseObserver)

Observer Trigger: Added the postMarkersChanged trigger to the DB observer to automatically update clients when new DRC markers are generated (e.g., after running check_3dblox).

Bug Fixes / Lifecycle

Safe Shutdown: Moved delete web_server_ up in the execution order. This ensures that the web server and its observer are safely shut down and unlinked while the database is still alive.

osamahammad21

You should remove the chiplet3DWidtget.* and keep only the web 3d widget.
Also, @maliberty, I am not the best one to review the web gui code, is there a code owner?

openroad-ci assigned jorge-ferreira-pii Mar 5, 2026

gemini-code-assist Bot reviewed Mar 5, 2026

View reviewed changes

jorge-ferreira-pii requested a review from osamahammad21 March 5, 2026 18:56

osamahammad21 requested changes Mar 8, 2026

View reviewed changes

openroad-ci force-pushed the test-3D-Viewer-Violations branch from 3f6fe76 to 127c781 Compare March 20, 2026 13:46

openroad-ci force-pushed the test-3D-Viewer-Violations branch from 127c781 to cc018cd Compare March 20, 2026 19:14

jorge-ferreira-pii added 3 commits April 7, 2026 23:12

feat(gui): integrate DRC report into 3D viewer

e52456e

Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

3D Viewer in the GUI

ca7d44f

Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

fix(web): resolve clang-tidy warnings

fe0f4de

Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

openroad-ci force-pushed the test-3D-Viewer-Violations branch from cc018cd to fe0f4de Compare April 8, 2026 03:02

jorge-ferreira-pii requested a review from osamahammad21 April 11, 2026 20:19

osamahammad21 requested changes Apr 13, 2026

View reviewed changes

openroad-ci closed this Apr 16, 2026

openroad-ci deleted the test-3D-Viewer-Violations branch April 16, 2026 01:41

	vertices_.push_back({QVector3D(p.x(), p.y(), p.z() * 2.0f), color});
	vertices_.push_back({QVector3D(p.x(), p.y(), p.z() * kZScale), color});

	target_z = draw_cuboid.zMax() * 2.0f + kHighlightZOffset;
	target_z = draw_cuboid.zMax() * kZScale + kHighlightZOffset;

Uh oh!

Conversation

openroad-ci commented Mar 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 5, 2026

Uh oh!

github-actions Bot commented Mar 5, 2026

Uh oh!

jorge-ferreira-pii commented Mar 5, 2026

Uh oh!

osamahammad21 left a comment

Choose a reason for hiding this comment

Uh oh!

osamahammad21 Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 20, 2026

Uh oh!

github-actions Bot commented Mar 20, 2026

Uh oh!

github-actions Bot commented Apr 8, 2026

Uh oh!

jorge-ferreira-pii commented Apr 8, 2026

Uh oh!

osamahammad21 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants