web: add display-control overlays from parity table 2.5 (#10619)#10806
Conversation
…-Project#10619) Add the missing overlay/grid/misc display controls to the web viewer, mirroring the Qt GUI behavior: - Access Points (Misc toggle, default off): dbAccessPoint X markers, green/red by hasAccess(), ITerms translated by instance origin and gated on inst_pins, BPins in global coords, sub-pixel LOD (RenderThread::drawAccessPoints parity). - Regions (Misc toggle, default on): dbRegion boundaries as semi-transparent gray fills with 1px outline (drawRegions parity). The client only creates the layer when the tech response reports has_regions, avoiding always-transparent tile requests. - Manufacturing grid (Misc toggle, default off): white dots on absolute grid multiples with the 5px-per-point LOD (drawManufacturingGrid parity). - GCell grid (Misc toggle, default off): dbGCellGrid lines clipped to the die area and closed at the die boundary (drawGCellGrid parity). - Flywires only (Misc toggle, default off): selected nets highlight as straight driver->sink lines instead of routed wire/guides; unrouted nets now show flywires even with the toggle off (NetDescriptor fallback parity). - Die and core area outlines are now always drawn on the _instances pass for single-die designs (drawChip parity). Rendering infrastructure: pseudo layers are registered in a single name->flag->painter table consumed by the tile dispatch and saveImage; drawLine clips segments to the tile (Liang-Barsky) so region edges and flywires stay cheap at deep zoom; per-block caches for BPin access points and gcell grids; the flywire driver-sink product is capped. The initial layer attach follows the visibility state restored from cookies so checkboxes and layers stay in sync at startup. Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>
- designated initializers for FlightLine and the PseudoLayerDef table - std::ranges::lower_bound in the gcell grid painter - std::min/std::max in the drawLine Liang-Barsky clip Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
![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
This pull request adds several visualization overlays to the web viewer to match the Qt GUI, including access points, region boundaries, manufacturing grids, GCell grids, and a 'flywires only' net highlighting mode. The feedback identifies a potential null pointer dereference when retrieving instance bounding boxes, incorrect grid alignment for negative coordinates due to integer division truncation, and a potential ReferenceError when checking the global techData variable in JavaScript. Additionally, it is recommended to replace C-style casts with C++-style casts for better type safety.
| } else { | ||
| const odb::Rect rect = iterm->getInst()->getBBox()->getBox(); | ||
| center = odb::Point((rect.xMax() + rect.xMin()) / 2, | ||
| (rect.yMax() + rect.yMin()) / 2); | ||
| } |
There was a problem hiding this comment.
In collectNetFlightLines, iterm->getInst()->getBBox() is called and dereferenced directly via ->getBox(). In OpenDB, getBBox() can return nullptr if the instance has no master or physical footprint, even if its placement status is PLACED. Dereferencing a null pointer will cause a crash.
To prevent this, we should perform a null check on the returned dbBox* pointer before calling getBox(), and fall back to the instance location if the bounding box is unavailable.
} else {
odb::dbBox* bbox = iterm->getInst()->getBBox();
if (bbox) {
const odb::Rect rect = bbox->getBox();
center = odb::Point((rect.xMax() + rect.xMin()) / 2,
(rect.yMax() + rect.yMin()) / 2);
} else {
int ix = 0;
int iy = 0;
iterm->getInst()->getLocation(ix, iy);
center = odb::Point(ix, iy);
}
}| const int first_x = ((dbu_tile.xMin() / grid) + 1) * grid; | ||
| const int last_x = (dbu_tile.xMax() / grid) * grid; | ||
| const int first_y = ((dbu_tile.yMin() / grid) + 1) * grid; | ||
| const int last_y = (dbu_tile.yMax() / grid) * grid; |
There was a problem hiding this comment.
The calculation of first_x, last_x, first_y, and last_y uses standard integer division dbu_tile.xMin() / grid, which truncates towards zero in C++. If the layout contains negative coordinates (which is common in some PDKs or custom layouts), this truncation will lead to incorrect grid alignment, missing grid dots, or drawing grid dots outside the tile boundaries.
We should use a robust integer arithmetic formula that correctly handles both positive and negative coordinates.
int first_x = dbu_tile.xMin() / grid * grid;
if (first_x < dbu_tile.xMin()) {
first_x += grid;
}
int last_x = dbu_tile.xMax() / grid * grid;
if (last_x > dbu_tile.xMax()) {
last_x -= grid;
}
int first_y = dbu_tile.yMin() / grid * grid;
if (first_y < dbu_tile.yMin()) {
first_y += grid;
}
int last_y = dbu_tile.yMax() / grid * grid;
if (last_y > dbu_tile.yMax()) {
last_y -= grid;
}| const int64_t pixel_xl = (int64_t) ((xl - dbu_tile.xMin()) * scale); | ||
| const int64_t pixel_yl = (int64_t) ((yl - dbu_tile.yMin()) * scale); | ||
| const int64_t pixel_xh = (int64_t) std::ceil((xh - dbu_tile.xMin()) * scale); | ||
| const int64_t pixel_yh = (int64_t) std::ceil((yh - dbu_tile.yMin()) * scale); |
There was a problem hiding this comment.
In modern C++, C-style casts like (int64_t) should be avoided in favor of C++-style casts like static_cast<int64_t> to ensure type safety and clarity.
| const int64_t pixel_xl = (int64_t) ((xl - dbu_tile.xMin()) * scale); | |
| const int64_t pixel_yl = (int64_t) ((yl - dbu_tile.yMin()) * scale); | |
| const int64_t pixel_xh = (int64_t) std::ceil((xh - dbu_tile.xMin()) * scale); | |
| const int64_t pixel_yh = (int64_t) std::ceil((yh - dbu_tile.yMin()) * scale); | |
| const int64_t pixel_xl = static_cast<int64_t>((xl - dbu_tile.xMin()) * scale); | |
| const int64_t pixel_yl = static_cast<int64_t>((yl - dbu_tile.yMin()) * scale); | |
| const int64_t pixel_xh = static_cast<int64_t>(std::ceil((xh - dbu_tile.xMin()) * scale)); | |
| const int64_t pixel_yh = static_cast<int64_t>(std::ceil((yh - dbu_tile.yMin()) * scale)); |
| // always come back transparent. (Regions created via Tcl mid-session | ||
| // need a page reload to appear.) | ||
| app.regionsLayer = null; | ||
| if (techData && techData.has_regions) { |
There was a problem hiding this comment.
Checking techData directly can throw a ReferenceError if the global variable techData is not defined (e.g., during early initialization or in unit test environments).
Using typeof techData !== 'undefined' is the standard, safe way in JavaScript to check for the existence of a global variable without risking a reference error.
| if (techData && techData.has_regions) { | |
| if (typeof techData !== 'undefined' && techData && techData.has_regions) { |
- mfg grid: snap grid anchors correctly for negative coordinates (integer division truncates toward zero; tile bounds include the label margin, which can go negative near the origin) - outlineRectInTile: replace C-style casts with static_cast Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>
dbInst::getBBox() cannot currently return null (the bbox is allocated in dbInst::create), but the guard is cheap and keeps the web viewer robust if that invariant ever changes; fall back to the instance location. Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>
1 participant
Addresses a portion of #10619
2.5 Display controls — overlays, grids & misc
drawAccessPointsdrawAccessPointsLayer()(_access_pointslayer, green/red X,inst_pinsgate, sub-pixel LOD)drawRegionsdrawRegionsLayer()(_regionslayer, default-on, gated byhas_regionsin the tech response)drawManufacturingGriddrawMfgGridLayer()(absolute grid multiples, 5px-per-point LOD)drawGCellGriddrawGcellGridLayer()(clipped to die, mesh closed at the die boundary)isFlywireHighlightOnly()collectNetFlightLines()+flywires_onlytoggle; unrouted nets now show flywires with the toggle off (GUI fallback parity)drawChip_instancespass for single-die designs_moduleslayersave/restore_display_controls