web: add schematic back navigation - #11040
Draft
yl12839-a11y wants to merge 29 commits into
Draft
Conversation
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>
…ic-inverter-symbol
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[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 on lines
+571
to
+573
| const netlist = this._currentNetlist | ||
| ? this._mergeSchematicNetlists(this._currentNetlist, data) | ||
| : data; |
Contributor
There was a problem hiding this comment.
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; | ||
| }); |
Contributor
There was a problem hiding this comment.
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>
Signed-off-by: sunny <yl12839@nyu.edu>
…ic-inverter-symbol Signed-off-by: sunny <yl12839@nyu.edu>
Signed-off-by: sunny <yl12839@nyu.edu>
…ic-inverter-symbol Resolves the schematic-widget.js conflict with the selection-token work in 6c121a6: keep master's beginSelection/isCurrentSelection ownership check and this branch's multi-line schematic_inspect request, which returns its promise so callers can await the inspect round-trip. Signed-off-by: sunny <yl12839@nyu.edu>
_textBBox had no callers at all. Three copies of the same hide-measure-
restore dance (_groupBoundsWithoutText, _groupBoundsWithoutLabel,
_groupScreenRectWithoutLabel) collapse onto one _measureHidden helper,
and the two rect builders that each open-coded the same field math become
boundsFromBBox/expandRect.
_rectOverlapArea carried fallbacks for a {x, y, width, height} rect shape
that no caller ever passes; every rect now comes from expandRect, so the
edges are always present.
_svgContentBoundsFromSvgBBox (and _elementBBoxToSvgRect, its only user)
recomputed per element, via a CTM round trip, the union that the root
getBBox() on the line above already returns. The screen-space and label
passes stay -- they measure things getBBox does not.
renderNetlist also ran _layoutInstanceLabels/_padSvgToContent twice: once
before the rAF pair and once inside it. The first pass measured text the
browser had not laid out yet, which is the reason the rAF pair exists,
and its result was overwritten before anything read it.
No behaviour change intended.
Signed-off-by: sunny <yl12839@nyu.edu>
… guard makeInteractiveCell, makeClassOnlyInteractiveCell and makeHitTargetCell were the same 15-line SVG setup three times over, differing only in how the cell id is discoverable and in the stubbed screen geometry. Also adds the schematic-side coverage for the shared selection token. ui-utils' beginSelection/isCurrentSelection guard was only exercised through display-controls, so nothing pinned the schematic dropping an inspect response that another panel had already superseded. Signed-off-by: sunny <yl12839@nyu.edu>
classifyRegister mixed 'return result' (on a still-empty result) with
'return {}' for the same not-a-register outcome; it now uses {} for every
rejection and declares result only where it is populated.
The AOI/OAI branch emitted a gate_ports map holding just the output pin.
The viewer derives AOI/OAI port ids from gate_terms and never reads it,
so a partial map was only a source of confusion.
gate_kind, gate_ports and gate_terms were undocumented despite being the
schematic response's only departure from the Yosys schema, so describe
them in server-api.md as the rendering hints they are.
Signed-off-by: sunny <yl12839@nyu.edu>
OpenROAD builds with -Werror=deprecated-declarations, so the call added by classifyRegister failed the build: request_handler.cpp:1569:47: error: 'bool sta::LibertyCell::hasSequentials() const' is deprecated The guard was redundant anyway. hasSequentials() returns !sequentials_.empty() || statetable_ != nullptr, so it cannot be false when sequentials() holds exactly one element -- the size check that follows already covers every case it rejected. Dropping it removes the deprecated call without changing which cells classify as registers. Signed-off-by: sunny <yl12839@nyu.edu>
This branch rewrites several bits of working code, and the diff alone does not say why. Comment-only; no behaviour change. - classifyGate's switch breaks instead of returning, so the shared pin mapping runs for every gate kind. - emitSchematicCell tries classifyRegister first, since classifyGate rejects sequential cells and would drop flops to a generic box. - The skin's dff symbol grew and was re-pinned to gain a QN port. - Cell hit-testing resolves a class token and a transparent rect because skin symbols are open paths that do not carry the id on the group. - The Boxes view renders the un-canonicalized netlist so generic boxes keep the design's real pin names. - refresh() returns a promise so callers can sequence after the render. - _ensureOpenRoadSymbolLabels supersedes _applyPinLabels, which could not place pins nested under translated helper groups. - The skin is fetched no-store so a stale copy cannot lack new symbols. Signed-off-by: sunny <yl12839@nyu.edu>
Picks up the master sync, the dead-code and duplication cleanup, and the deprecated-hasSequentials fix. The only conflict was in test-schematic-widget.js, where the back-nav tests and the new inspect selection-guard tests were appended at the same spot; both are kept. Signed-off-by: sunny <yl12839@nyu.edu>
Reduces the churn this branch puts on pre-existing lines. No behaviour change beyond the skin fetch noted below. - Restore the SKIN_COMPOUND_TYPES and SKIN_MULTI_TYPES comments, which were deleted outright even though the constants are unchanged. - Restore master's wording on the canonicalizeCell and AOI/OAI pid comments, and on 'custom skin' where it had been reworded for no reason. - Keep master's skin-canonicalization header, extending only the one clause that is now inaccurate (registers are recognised too), instead of rewriting the whole paragraph. - Drop 'cache: no-store' from the skin fetch, restoring master's line. A stale skin is better handled by server cache headers than by refetching the asset on every schematic init. schematic-widget.js now deletes 111 of master's lines instead of 126. Signed-off-by: sunny <yl12839@nyu.edu>
…bol' into feature/web-schematic-back-button
Author
Four spots replaced pre-existing behaviour without saying why. Comment-only; no behaviour change. - _handleSelectClick delegates to _cellHitFromTarget so double-click expansion resolves a click the same way. - _fetchInspect returns a promise so callers can wait for it to land. - _registerSvgCellHitTarget registers both id forms, because the old probe missed skin symbols that carry the name on a child's class. - canonicalizeCell falls back to register inference, since the backend only tags combinational cells. Signed-off-by: sunny <yl12839@nyu.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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
./etc/Build.sh).Related Issues
Stacked on #10961.