Skip to content

web: add schematic back navigation - #11040

Draft
yl12839-a11y wants to merge 18 commits into
The-OpenROAD-Project:masterfrom
yl12839-a11y:feature/web-schematic-back-button
Draft

web: add schematic back navigation#11040
yl12839-a11y wants to merge 18 commits into
The-OpenROAD-Project:masterfrom
yl12839-a11y:feature/web-schematic-back-button

Conversation

@yl12839-a11y

Copy link
Copy Markdown

Summary

Adds a "back" navigation control to the web schematic viewer so users can return to the previous view after drilling into a cell with double-click.

Note

Type of Change

  • New feature

Impact

Users can navigate back to the previous schematic view after double-clicking into a cell, instead of having to reload the schematic. Purely additive frontend behavior. No changes to placement, routing, timing, or any physical-design pipeline.

Verification

  • [ x ] I have verified that the local build succeeds (./etc/Build.sh).
  • [ x ] I have run the relevant tests and they pass.
  • [ x ] My code follows the repository's formatting guidelines.
  • [ x ] I have included tests to prevent regressions.
  • [ x ] I have signed my commits (DCO).

Related Issues

Stacked on #10961.

Classify connected Liberty cells as buffers, inverters, basic gates, and DFF variants when generating schematic JSON.

Add OpenROAD-owned NetlistSVG templates, a symbol/box view selector, improved symbol labels and hit testing, and double-click cone expansion.

Add C++ and JS coverage for Liberty-based symbol generation, schematic merging, and frontend symbol rendering behavior.

Signed-off-by: sunny <yl12839@nyu.edu>
…ic-inverter-symbol

Signed-off-by: sunny <yl12839@nyu.edu>

# Conflicts:
#	src/web/src/request_handler.cpp
#	src/web/src/schematic-widget.js
#	src/web/test/js/test-schematic-widget.js
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
Skip the port name entry when a flat AND/OR/XOR operand's Liberty
port is not resolvable, and reflow existing comments/call sites to
satisfy clang-format.

Signed-off-by: sunny <yl12839@nyu.edu>
…ranch

Signed-off-by: sunny <yl12839@nyu.edu>

@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 enhances the schematic viewer by adding support for standard logic symbols (including DFF, DFFR, and DFFS registers), a back button with navigation history, double-click cell expansion, and improved label placement to prevent overlapping. The backend was updated to classify and map register pins, and the frontend now supports merging schematic cones and toggling between symbol and box views. The review feedback highlights a potential race condition when merging in-flight expansion requests and a performance issue (layout thrashing) caused by calling getBoundingClientRect() inside a sort comparator during label layout.

Comment thread src/web/src/schematic-widget.js Outdated
Comment on lines +571 to +573
const netlist = this._currentNetlist
? this._mergeSchematicNetlists(this._currentNetlist, data)
: data;

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

There is a potential race condition here. If the user double-clicks a cell to expand it, a websocket request is sent. If the user performs another action (like a refresh or loading a different schematic) while the request is in flight, this._currentNetlist will be updated. When the expansion request completes, the new cone will be merged into the new netlist instead of the one where the double-click occurred. Using the captured previousSnapshot.netlist as the base for merging avoids this race condition.

                    const baseNetlist = previousSnapshot ? previousSnapshot.netlist : this._currentNetlist;\n                    const netlist = baseNetlist\n                        ? this._mergeSchematicNetlists(baseNetlist, data)\n                        : data;

Comment on lines +1360 to +1364
records.sort((a, b) => {
const rectA = a.group.getBoundingClientRect();
const rectB = b.group.getBoundingClientRect();
return rectA.top - rectB.top || rectA.left - rectB.left;
});

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

Calling getBoundingClientRect() inside the sort comparator causes layout thrashing (forced synchronous layout) because the browser is forced to recalculate styles and layout repeatedly during the sort operation (O(N log N) times). Pre-calculating and caching the bounding rectangles in a Map before sorting reduces the number of layout queries to exactly O(N), significantly improving rendering performance.

        const rects = new Map(records.map(r => [r, r.group.getBoundingClientRect()]));\n        records.sort((a, b) => {\n            const rectA = rects.get(a);\n            const rectB = rects.get(b);\n            return rectA.top - rectB.top || rectA.left - rectB.left;\n        });

Read this._currentNetlist before issuing the schematic_cone
websocket request rather than after the response resolves. If the
user loaded a different netlist while the request was in flight,
the returned expansion would previously be merged into (and
overwrite) that new netlist. Capturing the base at request time
localizes the response to the netlist the user actually acted on.

Signed-off-by: sunny <yl12839@nyu.edu>
Compute each record's bounding rectangle once into a Map before
sorting, instead of calling getBoundingClientRect() from inside
the comparator. That eliminates the O(N log N) forced synchronous
layouts the browser was doing during label placement.

Signed-off-by: sunny <yl12839@nyu.edu>
…ic-inverter-symbol

Signed-off-by: sunny <yl12839@nyu.edu>
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