Repository navigation
web: show bump footprints and logical inter-die links in 3D - #11897
naveenvenk17 wants to merge 7 commits into
Conversation
Carry forward the reviewed changes from The-OpenROAD-Project#11847 under the correct contributor account. The source tree matches f9a5e95. Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
There was a problem hiding this comment.
Code Review
This pull request introduces 3D interconnect visualization for the chiplet viewer, including bump footprints and logical net links. It adds new logic in the backend to extract and format this data, updates the frontend 3D viewer to render these meshes with visibility controls, and implements robust camera fitting logic. The reviewer identified several potential null pointer dereferences and type errors in the new implementation, suggesting necessary safety checks to prevent crashes.
| for (const ChipletNode& node : nodes) { | ||
| for (odb::dbChipNet* net : node.chip->getChipNets()) { |
There was a problem hiding this comment.
Potential null pointer dereference. node.chip can be nullptr (as checked in handleGet3DData on line 4326). If node.chip is null, calling node.chip->getChipNets() will result in a crash. A null check should be added before accessing getChipNets().
for (const ChipletNode& node : nodes) {
if (node.chip == nullptr) {
continue;
}
for (odb::dbChipNet* net : node.chip->getChipNets()) {| for (odb::dbChipBumpInst* bump_inst : region_inst->getChipBumpInsts()) { | ||
| odb::dbInst* inst = bump_inst->getChipBump()->getInst(); | ||
| if (inst == nullptr || !inst->isPlaced()) { | ||
| continue; | ||
| } | ||
| odb::Rect footprint = inst->getBBox()->getBox(); |
There was a problem hiding this comment.
Potential null pointer dereference. bump_inst->getChipBump() and inst->getBBox() can return nullptr in certain database states or incomplete designs. Accessing them without null checks can lead to server crashes. It is safer to add null checks before dereferencing them.
| for (odb::dbChipBumpInst* bump_inst : region_inst->getChipBumpInsts()) { | |
| odb::dbInst* inst = bump_inst->getChipBump()->getInst(); | |
| if (inst == nullptr || !inst->isPlaced()) { | |
| continue; | |
| } | |
| odb::Rect footprint = inst->getBBox()->getBox(); | |
| for (odb::dbChipBumpInst* bump_inst : region_inst->getChipBumpInsts()) { | |
| odb::dbChipBump* bump = bump_inst->getChipBump(); | |
| if (bump == nullptr) { | |
| continue; | |
| } | |
| odb::dbInst* inst = bump->getInst(); | |
| if (inst == nullptr || !inst->isPlaced()) { | |
| continue; | |
| } | |
| odb::dbBox* bbox = inst->getBBox(); | |
| if (bbox == nullptr) { | |
| continue; | |
| } | |
| odb::Rect footprint = bbox->getBox(); |
| _fitCameraDistance() { | ||
| const b = this._sceneBounds; |
There was a problem hiding this comment.
If this._sceneBounds is null or undefined (for example, during initialization or before a design is loaded), accessing properties on b will throw a TypeError. Adding a guard clause to return a default distance prevents potential crashes.
| _fitCameraDistance() { | |
| const b = this._sceneBounds; | |
| _fitCameraDistance() { | |
| const b = this._sceneBounds; | |
| if (!b) { | |
| return 1000; | |
| } |
Regenerated against the current upstream merge and checked for idempotence on the GCP VM. Only the missing Bison registry checksum is added; no dependency versions change. Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
|
I checked these against the traversal and ODB constructors. Grouping nodes can have no block, but the collected nodes still have a chip. Region and bump instances are created with their owning objects, and an instance's bounding box is allocated at creation. The existing nullable placement instance is already checked here. For the camera, scene bounds are set before the initial fit. Reset and resize both guard the unloaded state. The grouping-node and camera lifecycle cases are in the viewer tests, so I kept the existing contracts rather than silently skipping valid net data. |
Keep the feature tree unchanged and refresh pull-request CI after the missing registry checksum was added to master. Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
vvbandeira
left a comment
There was a problem hiding this comment.
approved from devops team side
Regenerate the browser bundles from the merged source. Keep the bump and logical-link viewer and its resize-loop regression. Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
Use the database scale when the top chip has no block. Leave flat-block responses and empty databases unchanged. Cover the native tech response for hierarchical, flat and empty designs, and require replay fixtures to carry the database scale. Signed-off-by: Naveen Venkat <81929600+naveenvenk17@users.noreply.github.com>
The 3D viewer shows chip bodies but leaves out their interconnects. This adds placed bump footprints from ODB geometry and logical links between bumps on each inter-die net.
Bumps, links and chip bodies have separate visibility controls. The geometry follows chiplet transforms and stacking offsets, including True Z mode. Bumps are instanced and links are batched. These links show connectivity, not routed wires or TSV geometry.
I merged upstream and regenerated the browser assets. I also fixed the grid-layout resize feedback loop and camera fitting on load, reset and resize.
Browser testing caught another bug: hierarchical designs without a top-level block omitted their database scale from the tech response. The app then used 1000 DBU per micron for a design using 2000. The serializer now returns the actual positive database scale. New tests exercise the real tech handler for hierarchical, flat and empty databases.
Testing on a Linux GCP VM, with test-result caching disabled:
Browser coverage used the exact regenerated bundle and fresh native fixture replay with software WebGL. It is not a live C++ network-server test, a GPU memory or speed benchmark, or a claim of full-app mobile support. Portrait coverage is the original bundled 3D component.
This is part of #11048 and replaces #11847.