Web gui parity 2.3 2.5 by jorge-ferreira-pii · Pull Request #10678 · The-OpenROAD-Project/OpenROAD

jorge-ferreira-pii · 2026-06-17T03:49:40Z

No description provided.

Implements the missing display-control toggles from issue The-OpenROAD-Project#10619, tables 2.3 (instances & cells) and 2.5 (overlays/grids/misc), in the Web GUI: - Block grids & overlays on the transparent overlay tile: GCell grid, Manufacturing grid (>=5px zoom gate), Regions (dbRegion fill), and Access points (green/red "X", zoom + layer gated). - Flight-line overlays as toggles: Flywires (unrouted nets) / Flywires only (all signal nets, capped per tile), and a Focused-nets-guides toggle gating the existing route-guide overlay. - Detailed view (forces sub-5px site rendering), Highlight selected (gates the selection highlight; hover drawn separately). - Tcl display-control persistence mirroring the native GUI: set/check/save/restore_display_controls (WebServer + web.i + web.tcl), with a server->client push applied via the frontend onPush handler. - Polish: scale-bar unit formatting (formatScaleBarLabel) and a Background color picker (overrides --bg-map, persisted in localStorage). Tests are tracked separately and not included in this commit. Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

Gui::checkDisplayControlsVisible() and the sibling display-control accessors dereferenced the global `main_window` without a null check. In headless/web mode the Qt MainWindow is never created (only the headless viewer hook is set), so `main_window` is null. When the web tile renderer's debug-graphics overlay (drawRendererOverlay, enabled by the "Debug Graphics" toggle) invokes a registered gui::Renderer's drawObjects(), and that renderer calls checkDisplayControl(), the call routes to Gui::checkDisplayControlsVisible() -> main_window->getControls() on a null main_window -> SIGSEGV. Guard these accessors against a null main_window, mirroring the existing guards in register/unregisterRenderer: check* return the default (visible=true, selectable=false), setters and save/restore become no-ops. This makes gui::Renderer::checkDisplayControl() safe in headless/web. Pre-existing crash (reproduces on master with the web "Debug Graphics" overlay enabled); independent of the web display-control parity work. Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

jorge-ferreira-pii · 2026-06-17T03:51:23Z

Addresses a portion of #10619

Web GUI: display-control parity with the native GUI (issue #10619, tables 2.3 & 2.5)

Closes the pending items of table 2.3 (Display controls — instances & cells) and
table 2.5 (overlays, grids & misc) from the parity roadmap, and fixes a pre-existing
GUI crash exposed by the web viewer.

Legend: ✅ Full (parity) · 🟡 Partial · ❌ Missing · ➕ Web-only · ⬆ changed in this branch

2.3 Display controls — instances & cells

Feature	GUI	Web	Status
Stdcell / macro / pad / physical subtypes	`displayControls`	`TileVisibility`	✅
Liberty classifications (buf/clk/seq/level-shift…)	yes	`classifyInstance()`	✅
Instance names / pins / pin-names	yes	`inst_names/inst_pins/inst_pin_names`	✅
Instance blockages (master OBS) toggle	yes	`blockages` flag (gates `vis.blockages`) + regression test	✅ ⬆ (was 🟡)

2.5 Display controls — overlays, grids & misc

Feature	GUI	Web	Status
Access points	yes	`drawAccessPoints` (green/red "X", zoom + layer gated)	✅ ⬆ (was ❌)
Regions (dbRegion)	yes	`drawRegions` (`getRegions`/`getBoundaries`)	✅ ⬆ (was ❌)
Manufacturing grid	yes	`drawManufacturingGrid` (≥5px zoom gate)	✅ ⬆ (was ❌)
GCell grid	yes	`drawGCellGrid` (`getGridX`/`getGridY`)	✅ ⬆ (was ❌)
Detailed view (LOD)	yes	`detailed` flag (forces sub-threshold site rendering)	✅ ⬆ (was ❌)
Flywires / flywires-only	yes	`drawFlywires` (per-tile cap) + toggles	✅ ⬆ (was ❌)
Focused-nets guides toggle	yes	`focused_nets_guides` flag gates route guides	✅ ⬆ (was 🟡)
Highlight selected	yes	`highlight_selected` flag (hover drawn separately)	✅ ⬆ (was 🟡)
Module view	yes	`_modules` layer	✅
Scale bar	yes	client-side toggle + `formatScaleBarLabel` (mm/µm/nm/pm)	✅ ⬆ (was 🟡)
Background color	yes	color picker (Misc) → `--bg-map`, persisted	✅ ⬆ (was 🟡)
Save/restore display state via Tcl	`save/restore_display_controls`	`set/check/save/restore_display_controls` + client push	✅ ⬆ (was ❌)

Additional fix (outside tables 2.3/2.5)

Item	GUI	Web	Status
SIGSEGV in the "Debug Graphics" overlay (headless)	n/a	null-guard in `Gui::checkDisplayControls*` (`gui.cpp`)	✅ ⬆ (was 💥 pre-existing crash)

gemini-code-assist

Code Review

This pull request implements display-control state synchronization and overlay rendering for the web GUI, mirroring the Qt GUI's display controls (such as grids, regions, access points, flywires, and detailed view) and adding a background color picker. The review feedback highlights several critical issues: potential out-of-bounds writes and incorrect grid calculations for negative coordinates in the manufacturing grid rendering, program termination bugs caused by using logger_->error instead of logger_->warn for non-fatal file I/O and parsing errors, and incorrect placement of access points for rotated or mirrored instances due to ignoring instance orientation.

gemini-code-assist · 2026-06-17T03:52:06Z

+  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;
+  for (int gx = first_x; gx <= last_x; gx += grid) {
+    const int px = static_cast<int>((gx - dbu_tile.xMin()) * scale);
+    for (int gy = first_y; gy <= last_y; gy += grid) {
+      const int py = 255 - static_cast<int>((gy - dbu_tile.yMin()) * scale);
+      blendPixel(image, px, py, color);
+    }
+  }


The grid start/end calculations (first_x, last_x, etc.) can be incorrect for negative coordinates due to C++ integer division truncation, and they will skip grid lines that lie exactly on the lower boundary of the tile (e.g., when dbu_tile.xMin() % grid == 0). Additionally, there are no bounds checks on px and py before calling blendPixel, which can lead to out-of-bounds writes (and potential crashes/memory corruption) if coordinates fall on the upper boundary (e.g., px = 256) or outside the tile.

Using std::ceil and std::floor provides a robust way to calculate the grid lines for both positive and negative coordinates, and adding explicit bounds checks ensures memory safety.

Suggested change

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;

for (int gx = first_x; gx <= last_x; gx += grid) {

const int px = static_cast<int>((gx - dbu_tile.xMin()) * scale);

for (int gy = first_y; gy <= last_y; gy += grid) {

const int py = 255 - static_cast<int>((gy - dbu_tile.yMin()) * scale);

blendPixel(image, px, py, color);

}

}

const int first_x = static_cast<int>(std::ceil(static_cast<double>(dbu_tile.xMin()) / grid)) * grid;

const int last_x = static_cast<int>(std::floor(static_cast<double>(dbu_tile.xMax()) / grid)) * grid;

const int first_y = static_cast<int>(std::ceil(static_cast<double>(dbu_tile.yMin()) / grid)) * grid;

const int last_y = static_cast<int>(std::floor(static_cast<double>(dbu_tile.yMax()) / grid)) * grid;

for (int gx = first_x; gx <= last_x; gx += grid) {

const int px = static_cast<int>((gx - dbu_tile.xMin()) * scale);

if (px < 0 || px >= 256) {

continue;

}

for (int gy = first_y; gy <= last_y; gy += grid) {

const int py = 255 - static_cast<int>((gy - dbu_tile.yMin()) * scale);

if (py < 0 || py >= 256) {

continue;

}

blendPixel(image, px, py, color);

}

}

gemini-code-assist · 2026-06-17T03:52:06Z

+    logger_->error(utl::WEB, 50, "Cannot open file for writing: {}", filename);
+    return;


In OpenROAD, calls to logger_->error terminate the program. Using it here for a non-fatal file writing error will crash the entire application (including the web server or interactive session) instead of gracefully returning as intended by the return; statement. Use logger_->warn instead to report the issue without terminating the process.

Suggested change

logger_->error(utl::WEB, 50, "Cannot open file for writing: {}", filename);

return;

logger_->warn(utl::WEB, 50, "Cannot open file for writing: {}", filename);

return;

References

Calls to logger_->error are expected to terminate the program, so additional error handling like returning an error code or throwing an exception is not necessary.

gemini-code-assist · 2026-06-17T03:52:06Z

+    logger_->error(utl::WEB, 51, "Cannot open file for reading: {}", filename);
+    return false;
+  }
+  std::ostringstream ss;
+  ss << in.rdbuf();
+  boost::json::value parsed;
+  try {
+    parsed = boost::json::parse(ss.str());
+  } catch (const std::exception& e) {
+    logger_->error(utl::WEB, 52, "Invalid display controls file: {}", e.what());
+    return false;
+  }
+  if (!parsed.is_object()) {
+    logger_->error(utl::WEB, 53, "Display controls file is not a JSON object");
+    return false;


Using logger_->error here will terminate the program on any non-fatal file reading or JSON parsing error, completely bypassing the recovery code (return false;). Use logger_->warn instead to log the warning and allow the function to return false gracefully.

Suggested change

logger_->error(utl::WEB, 51, "Cannot open file for reading: {}", filename);

return false;

}

std::ostringstream ss;

ss << in.rdbuf();

boost::json::value parsed;

try {

parsed = boost::json::parse(ss.str());

} catch (const std::exception& e) {

logger_->error(utl::WEB, 52, "Invalid display controls file: {}", e.what());

return false;

}

if (!parsed.is_object()) {

logger_->error(utl::WEB, 53, "Display controls file is not a JSON object");

return false;

logger_->warn(utl::WEB, 51, "Cannot open file for reading: {}", filename);

return false;

}

std::ostringstream ss;

ss << in.rdbuf();

boost::json::value parsed;

try {

parsed = boost::json::parse(ss.str());

} catch (const std::exception& e) {

logger_->warn(utl::WEB, 52, "Invalid display controls file: {}", e.what());

return false;

}

if (!parsed.is_object()) {

logger_->warn(utl::WEB, 53, "Display controls file is not a JSON object");

return false;

References

Calls to logger_->error are expected to terminate the program, so additional error handling like returning an error code or throwing an exception is not necessary.

gemini-code-assist · 2026-06-17T03:52:06Z

+    int x = 0, y = 0;
+    inst->getLocation(x, y);
+    const odb::dbTransform xform({x, y});


Using inst->getLocation(x, y) and constructing a translation-only transform ignores the instance's orientation (rotation and mirroring). This will cause access points for rotated or mirrored instances to be drawn in the wrong locations. Use inst->getTransform() directly to correctly handle both translation and orientation.

Suggested change

int x = 0, y = 0;

inst->getLocation(x, y);

const odb::dbTransform xform({x, y});

const odb::dbTransform xform = inst->getTransform();

On the access-points suggestion (inst->getLocation() → inst->getTransform()): this one is intentional, and I've left it as-is. drawAccessPoints here mirrors the native Qt GUI's RenderThread::drawAccessPoints (src/gui/src/renderThread.cpp), which uses inst->getLocation(x, y) + a translation-only dbTransform({x, y}). The access-point coordinate returned by dbITerm::getPrefAccessPoints() is already stored in the instance's
oriented frame, so only the translation to block coordinates is needed. Switching to
inst->getTransform() (which also applies rotation/mirroring) would apply the orientation a second time and misplace access points for rotated/mirrored instances — the opposite of
the intended fix. Keeping getLocation() preserves parity with the native GUI's rendering.

…alc) - saveDisplayControls/restoreDisplayControls: use logger_->warn instead of logger_->error for non-fatal file I/O and JSON parse failures. error() terminates the command and bypassed the return/return false recovery; warn keeps the web server / Tcl session alive and returns gracefully. - drawManufacturingGrid: compute first/last grid lines with std::ceil/std::floor on doubles instead of integer division, so lines exactly on the tile's lower boundary aren't skipped and negative coordinates are handled correctly (integer division truncates toward zero). blendPixel() already bounds-checks px/py, so no out-of-bounds write was possible and no extra guard is added. Addresses the gemini-code-assist review on PR The-OpenROAD-Project#10678. The access-point orientation suggestion is intentionally not applied: drawAccessPoints mirrors the native GUI (renderThread.cpp uses inst->getLocation()+translation, since the access-point coordinate is already oriented); applying inst->getTransform() would double-apply orientation and misplace points for rotated/mirrored cells. Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>

maliberty · 2026-06-17T15:20:20Z

For gcell grid in gui:

vs

it is missing the right/top edges

maliberty · 2026-06-17T15:24:49Z

There is no checking for invalid controls

>>> check_display_controls asdf visible
1

maliberty · 2026-06-17T15:26:31Z

set_display_controls Tracks visible 1
set_display_controls Tracks/Pref visible 1
but

jorge-ferreira-pii added 2 commits June 17, 2026 03:38

jorge-ferreira-pii self-assigned this Jun 17, 2026

github-actions Bot added the size/L label Jun 17, 2026

gemini-code-assist Bot reviewed Jun 17, 2026

View reviewed changes

jorge-ferreira-pii marked this pull request as ready for review June 17, 2026 04:39

jorge-ferreira-pii requested review from a team as code owners June 17, 2026 04:39

jorge-ferreira-pii requested a review from maliberty June 17, 2026 04:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web gui parity 2.3 2.5#10678

Web gui parity 2.3 2.5#10678
jorge-ferreira-pii wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:web-gui-parity-2.3-2.5

jorge-ferreira-pii commented Jun 17, 2026

Uh oh!

jorge-ferreira-pii commented Jun 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Uh oh!

jorge-ferreira-pii Jun 17, 2026

Uh oh!

maliberty commented Jun 17, 2026

Uh oh!

maliberty commented Jun 17, 2026

Uh oh!

maliberty commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		logger_->error(utl::WEB, 50, "Cannot open file for writing: {}", filename);
		return;

Conversation

jorge-ferreira-pii commented Jun 17, 2026

Uh oh!

jorge-ferreira-pii commented Jun 17, 2026

Web GUI: display-control parity with the native GUI (issue #10619, tables 2.3 & 2.5)

2.3 Display controls — instances & cells

2.5 Display controls — overlays, grids & misc

Additional fix (outside tables 2.3/2.5)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

jorge-ferreira-pii Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty commented Jun 17, 2026

Uh oh!

maliberty commented Jun 17, 2026

Uh oh!

maliberty commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants