Skip to content

gui: web GUI Tcl menu/toolbar, image/GIF export, editing utilities#10972

Open
jorge-ferreira-pii wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:feature-WebGUI-Scripting-Image-Editing-utilities
Open

gui: web GUI Tcl menu/toolbar, image/GIF export, editing utilities#10972
jorge-ferreira-pii wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:feature-WebGUI-Scripting-Image-Editing-utilities

Conversation

@jorge-ferreira-pii

@jorge-ferreira-pii jorge-ferreira-pii commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Addresses a portion of #10619

2.13 Scripting / Tcl — menu & toolbar buttons

Feature GUI Web Status
Custom menu items via Tcl create_menu_item / gui::remove_menu_item over MainWindow QActions create_menu_item / remove_menu_item Tcl; server-side registry in WebViewerHook (source of truth) pushed to clients as custom_ui; menu-bar.js merges static menus with the registered items, honoring the -path hierarchy (mirrors MainWindow::findMenu, default "Custom Scripts") ❌ → ✅
Custom toolbar buttons via Tcl create_toolbar_button / gui::remove_toolbar_button (text-only) create_toolbar_button / remove_toolbar_button Tcl; new client toolbar.js renders the pushed registry; clicking runs the script through the existing tcl_eval ❌ → ✅
Web additions (1) -icon / -tooltip on buttons; (2) -toggle + -script_off checkable buttons; (3) live multi-client sync (a button created in one browser appears in all, via SessionRegistry::broadcast); (4) Tcl puts/stdout now surfaces in the web console — sta::ReportTcl already tees stdout to utl::Logger, so a Tcl_Flush in TclEvaluator::eval routes it through WebLogSink ✅ (web-only)

2.14 Image / report export

Feature GUI Web Status
Animated GIF save_animated_gif -start/-add/-endGui::gif* over third-party/gif-h save_animated_gif -start/-add/-end server-side; TileGenerator::renderImageBuffer factored out of saveImage and shared as the frame source; web_gif.cpp wraps gif-h in an anonymous namespace (internal linkage) to avoid a duplicate-symbol clash with gui in the same binary ❌ → ✅
Per-widget images save_clocktree_image / save_histogram_image re-render an offscreen QWidget (raster) client-side (widgets render in the browser): new image-export.js (download / SVG serialize / rasterize / CSV / clipboard); Schematic exports SVG (vector) + PNG, 3D viewer exports PNG (preserveDrawingBuffer), Charts export CSV ❌ → ✅ (diverges: renders client-side)
Web additions high-resolution PNG (Nx scale factor); direct browser download (Qt writes to the server disk); copy-to-clipboard; aspect-preserving letterbox when GIF frames differ in size ✅ (web-only)

2.16 Editing utilities

Feature GUI Web Status
Global Connect dialog globalConnectDialog over odb dbGlobalConnect edit-dialogs.js (Tools → Global Connect) + EditHandler::global_connect_info / global_connect_delete / global_connect_apply; add/apply/clear reuse the existing Tcl add_global_connection / global_connect / clear_global_connect. Live rules table (delete-by-fields, Clear all, Apply force), client-side regex validation, "Connected N pins" feedback ❌ → ✅
Insert Buffer dialog insertBufferDialog (dbNet descriptor action) → dbNet::insertBuffer* EditHandler::buffer_info / insert_buffer calling dbNet::insertBufferAfterDriver / insertBufferBeforeLoads; entry via an Inspector button on a selected Net (using the Qt buffer.png glyph as inline SVG). The buffer is spliced into the net and left PLACED at the pin so it renders (detailed_placement still legalizes it) ❌ → ✅
Concurrency Qt is single-threaded DB-mutating handlers acquire the same mutex that serializes the Tcl write path (TclEvaluator), since web requests run on a multi-threaded io_context

Addresses a portion of The-OpenROAD-Project#10619

### 2.13 Scripting / Tcl — menu & toolbar buttons

| Feature | GUI | Web | Status |
|---|---|---|---|
| **Custom menu items via Tcl** | `create_menu_item` / `gui::remove_menu_item` over `MainWindow` QActions | `create_menu_item` / `remove_menu_item` Tcl; server-side registry in `WebViewerHook` (source of truth) pushed to clients as `custom_ui`; `menu-bar.js` merges static menus with the registered items, honoring the `-path` hierarchy (mirrors `MainWindow::findMenu`, default "Custom Scripts") | ❌ → ✅ |
| **Custom toolbar buttons via Tcl** | `create_toolbar_button` / `gui::remove_toolbar_button` (text-only) | `create_toolbar_button` / `remove_toolbar_button` Tcl; new client `toolbar.js` renders the pushed registry; clicking runs the script through the existing `tcl_eval` | ❌ → ✅ |
| **Web additions** | — | (1) `-icon` / `-tooltip` on buttons; (2) `-toggle` + `-script_off` checkable buttons; (3) live multi-client sync (a button created in one browser appears in all, via `SessionRegistry::broadcast`); (4) Tcl `puts`/stdout now surfaces in the web console — `sta::ReportTcl` already tees stdout to `utl::Logger`, so a `Tcl_Flush` in `TclEvaluator::eval` routes it through `WebLogSink` | ✅ (web-only) |

### 2.14 Image / report export

| Feature | GUI | Web | Status |
|---|---|---|---|
| **Animated GIF** | `save_animated_gif -start/-add/-end` → `Gui::gif*` over `third-party/gif-h` | `save_animated_gif -start/-add/-end` server-side; `TileGenerator::renderImageBuffer` factored out of `saveImage` and shared as the frame source; `web_gif.cpp` wraps `gif-h` in an anonymous namespace (internal linkage) to avoid a duplicate-symbol clash with `gui` in the same binary | ❌ → ✅ |
| **Per-widget images** | `save_clocktree_image` / `save_histogram_image` re-render an offscreen QWidget (raster) | client-side (widgets render in the browser): new `image-export.js` (download / SVG serialize / rasterize / CSV / clipboard); Schematic exports **SVG** (vector) + PNG, 3D viewer exports PNG (`preserveDrawingBuffer`), Charts export **CSV** | ❌ → ✅ (diverges: renders client-side) |
| **Web additions** | — | high-resolution PNG (Nx scale factor); direct browser download (Qt writes to the server disk); copy-to-clipboard; aspect-preserving letterbox when GIF frames differ in size | ✅ (web-only) |

### 2.16 Editing utilities

| Feature | GUI | Web | Status |
|---|---|---|---|
| **Global Connect dialog** | `globalConnectDialog` over odb `dbGlobalConnect` | `edit-dialogs.js` (Tools → Global Connect) + `EditHandler::global_connect_info` / `global_connect_delete` / `global_connect_apply`; add/apply/clear reuse the existing Tcl `add_global_connection` / `global_connect` / `clear_global_connect`. Live rules table (delete-by-fields, Clear all, Apply force), client-side regex validation, "Connected N pins" feedback | ❌ → ✅ |
| **Insert Buffer dialog** | `insertBufferDialog` (dbNet descriptor action) → `dbNet::insertBuffer*` | `EditHandler::buffer_info` / `insert_buffer` calling `dbNet::insertBufferAfterDriver` / `insertBufferBeforeLoads`; entry via an Inspector button on a selected Net (using the Qt `buffer.png` glyph as inline SVG). The buffer is spliced into the net and left PLACED at the pin so it renders (detailed_placement still legalizes it) | ❌ → ✅ |
| **Concurrency** | Qt is single-threaded | DB-mutating handlers acquire the same mutex that serializes the Tcl write path (`TclEvaluator`), since web requests run on a multi-threaded io_context | ✅ |

Tests: new gtests (`TestGif`, `EditHandler`/`CustomUi` cases in `TestRequestHandler`) and JS tests
(`test-toolbar`, `test-image-export`, `test-edit-dialogs`, extended `test-menu-bar`), registered in
both CMake and Bazel.

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds animated-GIF export and custom UI capabilities (Tcl-registered menu items and toolbar buttons) to the OpenROAD web GUI, mirroring the Qt GUI features. It also introduces client-side editing dialogs for Global Connect and Insert Buffer, along with helper utilities for image and CSV export. The feedback highlights a few critical issues: a Tcl crash in web.tcl due to calling getDbuPerMicron directly on the database object instead of the tech object, a memory leak in edit-dialogs.js where the keydown event listener is not always cleaned up, and a robustness issue in web.cpp where a failed GIF encoder initialization is not properly reset.

Comment thread src/web/src/web.tcl
Comment on lines +207 to +208
set db [ord::get_db]
set resolution [expr $keys(-resolution) * [$db getDbuPerMicron]]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The dbDatabase object (returned by ord::get_db) does not have a getDbuPerMicron method directly. Attempting to call [$db getDbuPerMicron] will result in a Tcl error and crash the command. Instead, you should retrieve the dbTech object first using getTech and then call getDbuPerMicron on it.

    set db [ord::get_db]
    set resolution [expr $keys(-resolution) * [[$db getTech] getDbuPerMicron]]

Comment thread src/web/src/web.tcl
Comment on lines +243 to +244
set db [ord::get_db]
set dbu [$db getDbuPerMicron]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The dbDatabase object does not have a getDbuPerMicron method directly. Attempting to call [$db getDbuPerMicron] will result in a Tcl error. Use [[$db getTech] getDbuPerMicron] instead.

    set db [ord::get_db]
    set dbu [[$db getTech] getDbuPerMicron]

Comment thread src/web/src/edit-dialogs.js Outdated
Comment on lines +25 to +29
const close = () => overlay.remove();
overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); });
document.addEventListener('keydown', function onKey(e) {
if (e.key === 'Escape') { close(); document.removeEventListener('keydown', onKey); }
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The onKey event listener is added to document every time makeModal is called, but it is only removed if the user presses the 'Escape' key. If the modal is closed via clicking the overlay or clicking the 'Close'/'Cancel' buttons, the event listener is never removed, leading to a memory leak. Refactor makeModal to ensure the keydown event listener is always cleaned up when the modal is closed.

    function onKey(e) {
        if (e.key === 'Escape') close();
    }
    const close = () => {
        overlay.remove();
        document.removeEventListener('keydown', onKey);
    };
    overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); });
    document.addEventListener('keydown', onKey);

Comment thread src/web/src/web.cpp
Comment on lines +1475 to 1479
if (!gif->encoder.begin(gif->filename, w, h, d)) {
logger_->error(
utl::WEB, 55, "Failed to open GIF file {}.", gif->filename);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If gif->encoder.begin fails (e.g., due to file permission issues), the GIF stream is left in an uninitialized state with frame_count remaining 0. Subsequent calls to gifAddFrame will repeatedly attempt to call begin and fail, flooding the log. Resetting the unique pointer in gifs_[idx] on failure prevents further redundant attempts and cleans up the slot.

Suggested change
if (!gif->encoder.begin(gif->filename, w, h, d)) {
logger_->error(
utl::WEB, 55, "Failed to open GIF file {}.", gif->filename);
return;
}
if (!gif->encoder.begin(gif->filename, w, h, d)) {
logger_->error(
utl::WEB, 55, "Failed to open GIF file {}.", gif->filename);
gifs_[idx].reset();
return;
}

…10972)

- edit-dialogs.js: makeModal now removes its `keydown` listener on every close
  path (overlay click / Close / Cancel / Escape), not only on Escape, fixing a
  listener leak when the modal is dismissed without pressing Escape.
- web.cpp: reset the GIF stream slot (`gifs_[idx].reset()`) when
  GifEncoder::begin() fails, so subsequent gifAddFrame calls stop retrying a
  dead stream and flooding the log.

Not applied: the suggested `[[$db getTech] getDbuPerMicron]` change is a false
positive. `getDbuPerMicron` is a method of dbDatabase (not dbTech, which has
getDbUnitsPerMicron), so `[$db getDbuPerMicron]` is correct and matches the
existing save_image usage; the suggested form would raise "Invalid method" and
crash save_image/save_animated_gif with -resolution or a non-zero -area.

Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>
@jorge-ferreira-pii
jorge-ferreira-pii marked this pull request as ready for review July 23, 2026 15:52
@jorge-ferreira-pii
jorge-ferreira-pii requested a review from a team as a code owner July 23, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant