Skip to content

sync - #2

Open
nanocoh wants to merge 2918 commits into
keplertech:masterfrom
The-OpenROAD-Project:master
Open

sync#2
nanocoh wants to merge 2918 commits into
keplertech:masterfrom
The-OpenROAD-Project:master

Conversation

@nanocoh

@nanocoh nanocoh commented May 23, 2026

Copy link
Copy Markdown

Summary

[Describe your changes here]

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Documentation update

Impact

[How does this change the tool's behavior?]

Verification

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

Related Issues

[Link issues here]

AcKoucher and others added 25 commits August 12, 2026 17:18
…rting point

Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
…ngestion

GRT: Fix FastRoute blocked tracks computation
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
dpl: check edge spacing DRC on opt_mirror
…ff-register-latch-mask

gpl: Separate registers and latches in MBFF masks
Signed-off-by: Mike Inouye <mikeinouye@google.com>
Signed-off-by: Mike Inouye <mikeinouye@google.com>
…place-bracket

odb: Avoid Verilog namespace collisions
mbff: dont count Enable input pin as data input
Add `global_placement -place_ios`, which turns every movable IO port into a
variable of the same Nesterov solve as the cells instead of treating it as a
fixed anchor. The solved pin locations are written to the database; snapping
them onto routing-track slots stays place_pins' job, the same way
detailed_placement follows global_placement for cells.

An IO pin becomes a GCell in NesterovBase::ioPinStor_ and its existing GPin is
re-linked to it, so the unchanged weighted-average wirelength gradient starts
differentiating the pin - no IO-specific gradient code. A pin is confined to
the die perimeter by projecting it, every iteration, onto a set of perimeter
segments that models the unconstrained case, set_io_pin_constraint regions and
exclude_io_pin_region uniformly. Mirrored pairs carry one degree of freedom:
the follower is the exact reflection of its master, and initIoConstraints()
intersects the master's locus with the mirror of the follower's so that
reflection is always legal - ppl treats it as a hard constraint and errors
(PPL-82) otherwise.

Three interactions needed care:

  - IO pins carry no 2-D density force. They sit on the die perimeter, outside
    the core bin grid, and contribute no area to it, so -place_ios does not
    move the cells' overflow convergence point.
  - Their step-to-step displacement is a projection jump rather than a gradient
    step, and the Barzilai-Borwein step length is shared by every GCell, so
    they are excluded from that norm. Including them left overflow oscillating
    at 0.25-0.43 instead of reaching the 0.1 target.
  - The device-resident coordinate, wirelength and HPWL pipelines are built
    from NesterovBaseCommon::gCellStor_ and do not model ioPinStor_, so
    -place_ios keeps them host-resident and forces the CPU backends.

Ports the solve does not model - power/ground, and anything not on a signal or
clock net - own no GPin, so their gradient would be identically zero. They are
left for place_pins to place, which means an unplaced port is a normal state
during global placement and no longer an error (GPL-326).

-place_ios is mutually exclusive with -timing_driven, -routability_driven,
-incremental, -skip_io and -skip_nesterov_place.

Signed-off-by: Minju Kim <mkim@precisioninno.com>
NesterovBase::ioNbPos() called std::vector<GCellHandle>::size() inside the
class body, where GCellHandle is still only forward declared - its definition
follows NesterovBase in the same header. GCC accepted it, clang/libc++ did not:

  vector.h:385:48: error: arithmetic on a pointer to an incomplete type
  'gpl::GCellHandle'

which broke every Bazel target depending on //src/gpl:gpl.

Keep the declaration in the class and move the body below GCellHandle.

Signed-off-by: Minju Kim <mkim@precisioninno.com>
- indexOfGCell() compared pointers from two different pools with < and >=,
  which is undefined behavior. std::less gives a total order over all
  pointers, so the range check stays valid when the GCell comes from
  ioPinStor_.
- getDistance() divided by n, which is nb_gcells_.size() - ioPinStor_.size()
  and can be zero when every movable object is an IO pin. The resulting NaN
  step length propagates into the whole solve.
- A blocked region that does not overlap the edge span produced a free
  segment reaching past the end of that span. Not reachable today, since
  odb clamps every region into the die area, but the function no longer
  depends on that.
- A constraint region that is not a die-edge interval is a top-layer region:
  the pin belongs on the top-layer grid inside the die, not on the perimeter,
  and ppl makes the same distinction in getConstraintsFromDB(). Such a pin
  was seeded as a movable IO GCell and warned that it would be placed on the
  free perimeter; the solve then dragged it to a die edge and placed the
  cells against a position place_pins discards. Leave those pins to
  place_pins, and reuse GPL-172 to report how many the perimeter model
  skipped.

Signed-off-by: Minju Kim <mkim@precisioninno.com>
odb: temporary fix for wire ordering of input bump nets
…ROAD into ant_order_independent_merge

Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Equivalence checking needs an oracle. kepler-formal is GPL-3.0 and OpenROAD is
BSD-3-Clause, so it is invoked as a subprocess and never linked: //src/tst:lec
builds the config, runs the binary, and classifies the verdict, linking nothing
of it.

It builds from source as a bazel dev_dependency, so a developer needs no manual
install and the tests cannot silently run against whatever binary happens to be
on $PATH. Two things make that work:

  - //bazel:kepler.bzl carries a Starlark transition that strips `-xc++` from
    cxxopts for kepler's subtree. OpenROAD sets that flag globally, and it
    breaks cmake's compiler probe inside rules_foreign_cc, which kepler's
    oneTBB dependency uses. The transition keeps the flag for OpenROAD's own
    compilation, which is what it is there for.

  - bazel/kepler-patches/ drops a linkopt that naja's python-embed leaks, which
    would otherwise export a system library search directory into the link.

At run time lec.cpp resolves the binary from $KEPLER_FORMAL, then the test's
runfiles, then $PATH, so a locally installed binary still wins for anyone who
wants to point at their own build. A test that declares :kepler_formal_bin in
data always has it, so under bazel the tool is never missing and a LEC test can
never quietly degrade into a no-op.

Three details about reading kepler's output, all of which have been wrong at
some point and each of which fails silently rather than loudly:

  - The SEC encoding is dual_rail_steady, not binary. Which one is sound
    flipped between kepler revisions; the comment in lec.cpp records the
    mutation battery that settled it.

  - A proved verdict has two spellings. dual_rail_steady prints "No
    binary-defined difference was found" where binary prints "No difference
    was found", and matching only the first turns every pass into
    "inconclusive".

  - The verdict text is read BEFORE the exit status, and the exit status only
    decides what an absent verdict means. kepler-formal used to exit 0 whether
    it proved equivalence or found a difference; it now exits 3 on a
    difference. An exit-code-first classifier therefore reports every genuine
    inequivalence as a tool error, which is the worst direction to be wrong in:
    it makes a real defect indistinguishable from a broken fixture.

The installer gains the host tools kepler's own build reaches outside bazel for
(bison, flex, m4, cmake, pkg-config, python3 headers); without them the failure
surfaces as an unhelpful repository-rule error inside a bazel fetch.

The tests for this land in the next commit, with the fixture they need.

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
One netlist loaded through one link mode, owning its own db and sta, with
read_verilog -> link_design [-hier] -> write_verilog behind three calls. It is
deliberately separate from IntegratedFixture: a consumer that only wants to
read, link and write a netlist should not link the optimization tool chain to
do it, and the hierarchy conformance suites that follow want nothing else.

The link mode is a constructor argument rather than two classes, so a test can
run the same netlist both ways and attribute a difference to the mode. Errors
surface as exceptions, since link_design reports failure by throwing from
utl::Logger::error(), and a netlist that one mode refuses while the other
accepts is itself a finding worth catching rather than a crash.

TestLoadedDesign covers the fixture. TestLec covers the LEC wrapper from the
previous commit and needs this fixture to produce a gate netlist, which is why
the two arrive together: a proved pair, a real logic difference (an inverter
where the gold has a buffer), and a dropped top-level port. The last two matter
most -- both were once reported by kepler-formal with a zero exit code, and the
inequivalent one now exits 3, so neither the exit status nor a bare "no
difference" line can be trusted on its own.

Their netlists live in TestNetlists.h and are written to the test's temp dir
rather than checked in as .v files. The security pre-commit hook blocks *.v
outside an allowlist of test directories and src/tst/test is not on it, no .v
file having ever lived there. Inlining three small netlists costs nothing and
buys something: the two defective variants are derived from the good one by a
named edit, so the single difference each is testing is visible in the code
rather than being a comment on a copy that can drift. The derivation throws if
the text it edits is ever renamed, since a silent no-op would leave a
"defective" fixture identical to the netlist it came from -- one that still
runs and proves nothing.

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
Two suites over one corpus, both running read_verilog -> link_design [-hier]
-> write_verilog and comparing the result against the INPUT netlist.

dbsta_hier_conformance proves logical equivalence of each emitted netlist to
the input, once per link mode. Deliberately not flat_out == hier_out: the flat
dbNet view is built by mode-independent code, so a reader bug upstream of the
hierarchy split corrupts both outputs identically and that comparison passes
while both are wrong. Running the modes separately also gives attribution -- a
hier-vs-flat mismatch says the two disagree but not which is wrong.

dbsta_hier_structural diffs the emitted netlist against the input for the ten
things a LEC cannot see: module set, top and submodule port order, declared
nets, instance bindings, cell census, assigns, name identity, namespace rules,
and whether the output can be read back at all. A netlist can be provably
equivalent and still have its port list reordered, which silently miswires any
parent that instantiates it positionally.

Known failures are XFAIL rather than DISABLED_, so a fix turns the suite red
with an actionable message instead of quietly leaving the case switched off.
The list lives in hier_expected_fail.bzl, grouped by failure mode, and BUILD
renders it into the manifest each suite reads. Keeping it in Starlark means
Starlark can reject a malformed entry when the package loads: an unknown check
or path, a ':' that would truncate the row, or one netlist listed twice under
one key.

Corpus membership is a directory scan, so adding a case is adding a file. What
a folder listing cannot carry -- a case's top module -- lives in BUILD as
HIER_TOP_OVERRIDES.

This commit carries the fixtures that belong to the suites rather than to any
one construct family: inherited/, symlinks to netlists other tests own, so a
fixture written for rsz or odb is also held to conformance; structural/, cases
only the structural suite can run because a LEC cannot adjudicate them (several
are refused by the reader outright); and crash/, five netlists that kill the
process and are therefore a data dependency of neither suite.

Both suites guard against loading zero cases, because a corpus that silently
resolved to nothing would make them vacuously green. That guard earns its keep
immediately: the structural suite never had inherited/ in its data, so those 41
cases had never run under it. Adding the dependency turns up 22 rows' worth of
structural failures in netlists other tests already own -- port reorders, a
uniquification clone, and an added assign -- and the same guard now covers
inherited/ so the hole cannot reopen.

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
126 netlists written while establishing that the two link modes disagree at
all. Each isolates one construct that crosses a hierarchy boundary: bus slices
and part-selects handed to a submodule, feedthroughs at depths 1 to 4, aliases
where an assign renames a net across a boundary, escaped identifiers, name
order interacting with the writer, and instances whose ports are connected
positionally.

They are the cases that made the campaign worth running: the flat writer drops
a submodule's bus-slice feedthrough assign and leaves two top outputs undriven,
the hier writer hoists an internal feedthrough into the parent and double-drives
a port, and a synthesized hierarchy path collides with a user's escaped name.
All three are recorded here as XFAIL with the netlist that reproduces them.

Each case carries a `// TARGETS:` header naming the constructs it exercises,
which the suite lints: an authored case must declare what it is for, and two
cases may share a target set only if their bodies differ.

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
272 netlists covering a four-axis cross product exhaustively rather than by
inspiration: hierarchy depth 1 to 4, signal shape (scalar, bus MSB-first, bus
LSB-first, bus split across boundaries), port wiring (feedthrough, fanout,
gated, partial), and identifier form (plain, bracketed, escaped-slash, mixed).

A sweep like this is worth its size because the interesting failures live in
the interaction, not in any one axis. Depth alone is uninteresting -- but a
bracketed identifier on an LSB-first bus split across a depth-4 boundary
exercises the path join, the bus regroup and the escape logic at once, and only
the cross product reaches it.

Generated cases are held to the same rule as authored ones: each declares its
TARGETS, and the suite rejects two cases that declare the same targets and have
identical bodies, so a generator that emits duplicates fails the build rather
than inflating the corpus.

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
185 netlists across the two families the campaign keeps finding defects in.

bx_naming_* covers what an identifier can legally be and what OpenROAD does
with it: escaped identifiers holding a slash, a bracket, a backslash, a dollar,
a plus, a keyword, a leading digit or a name that differs from another only by
case, in every object kind (module, instance, net, port, bus, bus bit, top) and
at depths 1 to 4. Escaping is where the round trip is hardest to get right --
the writer must re-derive an escape the reader threw away -- and these record
that it frequently does not: `wire wire;`, `output output;` and
`INV_X1 assign (...)` all reach the emitted netlist unescaped.

bx_collisions_* sets up the case nobody writes: a user object that ALREADY owns
the name OpenROAD is about to synthesize. Flattening joins a hierarchy path
with '/' and escapes the result, so instance x containing y becomes `\x/y ` --
which collides with a net literally named `\x/y `. Uniquification names a clone
`sub_u1`, which collides with a module the user named `sub_u1`. The writer
invents `_NC1` for an unconnected port, which collides with a wire named _NC1.
Each of those is a line of the corpus, with the victim declared explicitly,
because the collision is invisible without it.

The emitted netlist in these cases is not merely ugly: it declares one name
twice, which is illegal Verilog that OpenROAD itself silently merges on
re-read, so two electrically distinct nets become one.

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
gadfort and others added 30 commits August 21, 2026 14:10
…check

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Signed-off-by: Mike Inouye <mikeinouye@google.com>
Signed-off-by: Mike Inouye <mikeinouye@google.com>
…ayer

grt/cugr: support pins above the max routing layer
rsz: Fix UB in performEarlySizingRound
pad: reorganize RDLRoute into per net to allow to better route continuity checking
gui: avoid call to sortItems since that triggers from bazel segfault
place_macro validated the core-containment rectangle using the
instance's
bounding box *before* the requested orientation was applied. The four
right-angle orientations (R90, R270, MXR90, MYR90)
swap width and height, so whenever the requested orientation differed
from the current one in right-angle-ness the check used the wrong
footprint: it rejected legal placements of rotated macros with
MPL-0034, and symmetrically would accept ones that do not fit.

Compute the width and height the macro will actually have once
`orientation` is applied, and validate that instead. The comparison is
made against the instance's current orientation rather than assuming
R0, so a macro that has already been rotated is handled correctly too.

The new test places a 100x400um macro at R90 in a 499.89x198.80um
core, where the rotated footprint (400x100) fits and the unrotated one
(100x400) does not. It also asserts the same origin is still rejected
at R0, so the fix cannot silently over-correct.

Signed-off-by: sfmth <sfmth0@gmail.com>
The new test was added to COMPULSORY_TESTS but not to the per-test
resource dict. src/mpl/test/BUILD does not glob the testcases/
directory -- the filegroup globs only `test_name + ".*"`, which picks up
the .tcl, .ok and .defok, so each test's LEF and DEF have to be listed
explicitly.

The Bazel target therefore resolved and ran without
testcases/place_macro_rotated.{lef,def}, while the CMake build passed
because it collects test inputs differently.

bazel query deps(//src/mpl/test:place_macro_rotated_resources) now lists
all six required inputs, and
bazel test //src/mpl/test:place_macro_rotated-tcl_test passes.

Signed-off-by: sfmth <sfmth0@gmail.com>
Set the orientation before building the containment rectangle instead of
computing the post-orientation footprint by hand, per review. getBBox()
then already reports the footprint the macro will have, so the explicit
width/height swap and its right-angle-ness comparison are unnecessary.
setOrient() had to precede setLocation() in any case; it now precedes
the
core-containment check as well.

Rename the test to place_macro_with_right_angle_rotation and reduce it
to
the single R90 placement. The negative case it also asserted is no
longer
sound: with setOrient() ahead of the check, a rejected placement leaves
the instance rotated, so catching MPL-0034 and then writing the DEF
would
serialize the macro at the rejected orientation.

Signed-off-by: sfmth <sfmth0@gmail.com>
Rename the test and all five of its files to
place_macro_right_angle_rotation, so the CMake test name, the Bazel
targets, the .tcl/.ok/.defok and the testcases LEF/DEF agree. The
half-applied rename currently on the PR branch renamed only the Bazel
entries, which fails the package load -- "Error in glob: glob pattern
'place_macro_with_right_angle_rotation.*' didn't match anything" -- and
takes all of Jenkins down with it.

Reword the comment above setOrient() and cut the test down to its header
line, both verbatim as requested in review.

Signed-off-by: sfmth <sfmth0@gmail.com>
mpl: check core containment against the post-orientation footprint
…w-metrics-check

test: check flow metrics under bazel
Add README sections for save_display_controls and restore_display_controls,
and bring the web_server entry in line with web.tcl: -stop, an OS-assigned
default port, and -dir as deprecated rather than required.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Both tests were registered against the wrong dependency set, so each failed
in one build system while passing in the other.

display_state_test imports jsdom, but was registered with JS_FILES, which
carries only the product's own JS sources.  jsdom comes from
:node_modules/jsdom, bundled into DOM_TEST_DATA with the other DOM tests.
Node could not resolve the import and the test aborted before running a
single case, so nothing it covers was ever actually checked.

TestSaveDisplayControls compiles gui/src/stub.cpp to pick up the three
gui::Gui symbols web_serve.cpp needs, and stub.cpp's initGui calls
DescriptorRegistry::initDescriptors, which lives in init_descriptors.cpp.
Bazel's :gui_stub deps on :gui and gets that file; CMake has no equivalent
library, since gui_descriptors deliberately holds only the registry and the
descriptor implementations live inside the Qt gui target the test cannot
link.  The reference was therefore unresolvable, and the test now compiles
the three Qt-free descriptor sources itself.  bufferTreeDescriptor.cpp is
not among them despite init_descriptors.cpp calling BufferTree::setSTA: it
reaches QColor through colorGenerator.h and does not compile without Qt,
and stub.cpp supplies the BufferTree definitions instead.

The link failed under GNU ld on every CMake platform, not only the one CI
reported; elsewhere a parallel gmake aborted before reaching the target.
Bazel is unaffected because lld discards the never-called initGui before
its relocation has to resolve.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
…ebGUI-display-controls-overlays-grids-misc2

Addresses a portion of #10619, 2.5 Display controls — overlays, grids & misc
Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
pad: remove unused variable
pdn: dont rely on just the intersection for via insertion, check actual layer shape if initial ensure does not yield valid vias
…ction-highlight-inspection

master brought in the display-controls/overlays/grids web work, which
touched the same files as the selection/highlight/inspection work here.
Conflicts resolved in:

  src/web/src/inspector.js
  src/web/src/main.js
  src/web/src/request_handler.cpp
  src/web/src/request_handler.h
  src/web/src/style.css
  src/web/src/ui-utils.js
  src/web/test/cpp/TestRequestHandler.cpp
  src/web/test/js/test-ui-utils.js

Beyond the textual conflicts:

- ScopedDbuFormat takes master's dbDatabase* (multi-die: a hierarchical
  top chip owns no dbBlock) and keeps this branch's convert_string
  installation; all call sites updated.
- Highlight collection goes through master's clearSelectionHighlights /
  setSelectionHighlights / setSelectionSetHighlights helpers so
  highlight_source stays in sync and a "Flywires only" flip re-derives
  from the right source.  appendHighlightShapes also appends the
  ShapeCollector flight lines this branch added.
- writeInspectPayload combines both signatures; use_dbu and the logger
  are threaded through this branch's writeInspectTrailer helper.
- handleOverlayTile keeps master's error handling, coordinate
  validation, flywire re-derive and highlight_selected gate, plus this
  branch's stale-selection drop and highlight-group shapes.
- applyBounds picks up master's tileSizeCss() and maxUsefulZoom() cap so
  both the boot and resync paths get them.

src/web/test/BUILD merged cleanly but both sides had added a js_test
named ui_utils_test; dropped the duplicate.

Also drops two no-op apply lambdas in the set_property test descriptor
that only discarded a nodiscard any_cast, and switches the marshalling
assertions to pointer-form any_cast so a regression reports as a named
assertion instead of an escaping bad_any_cast.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Do not collect an unrouted net's flywires twice.  This branch's
ShapeCollector captures drawLine, and for a net with no wire and no
guides DbNetDescriptor::highlight draws the driver->sink fan itself, so
the descriptor's copy landed on top of the one collectNetFlightLines
derives.  Keep the derived copy, which honours kMaxFlywires, and drop
the descriptor's.

Derive highlight-group shapes through appendHighlightShapes instead of
calling sel.highlight() directly, so a net parked in a persistent group
honours "Flywires only" the way the current selection does, and rebuild
the group shapes when the toggle flips.

Rebuild highlight geometry after an object moves.  A set_property edit
only rebuilt the editing session's shapes; every other session was told
to redraw, and redrew from its own cache of the old placement.  The
session's odb callbacks now raise highlight_geometry_stale on
inDbPostMoveInst / inDbInstSwapMasterAfter, which fire in all sessions,
and the overlay handler re-derives both the selection and the group
shapes from it.

Re-run a selection-browser refresh that was skipped because a
list_selection was already in flight.  The outstanding response predates
the mutation, so dropping the refresh left the table -- and the row
indices inspect/deselect send back -- describing the old set until the
next mutation.

Each fix has a regression test confirmed to fail without it.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
…ebGUI-selection-highlight-inspection

Addresses a portion of #10619, 2.6 Selection, highlight & inspection
master has landed a large amount of src/web work since this branch was cut,
including its own implementations of things this branch built independently.
Conflict resolutions, by theme:

Both sides added at the same point; kept both:
- ui-utils.js: buildVisibilityFlags/applySelectionFlags next to master's
  toast, modal, scale-bar and selection-ownership helpers.
- tile_generator.h: ColoredPolygon next to TileFrame/dbuPrecision.
- request_handler.h: kFind and kContextAction next to kCancel.
- TestRequestHandler.cpp: FindHandlerTest and master's SetPropertyTest
  collided at one insertion point; both restored in full.

Same problem solved twice; took master's implementation:
- Selection highlight lines are master's colored FlightLine, not this
  branch's pair<Point, Point>; the duplicate ShapeCollector::drawLine
  override is gone.
- Highlight groups: master already had highlight_groups plus
  rebuildHighlightGroupShapesLocked/removeFromHighlightGroupsLocked, so
  handleContextAction's highlight_* branch now drives that state instead of
  keeping its own parallel copy.  The context menu and the inspector's
  highlight button therefore share one code path, and the group index is
  validated the way master's handlers validate it.
- web.cpp uses master's assetPathFromTarget(), which strips the fragment as
  well as the query, in place of the inline query strip; /download/image
  matches on the cleaned path and gets the raw target for its parameters.
- handleOverlayTile is master's rewrite (request validation, error
  responses, dpr/tile_px, flywires and geometry-stale rebuilds) with this
  branch's colored-polygon channel added back.

Genuinely merged:
- handleSelect carries both the context flag and show_connectivity; the
  "keep the inspected object on an empty right-click" rule now also guards
  master's runDeselectAction.
- selectAtLatLng keeps this branch's extraction into a reusable function;
  master's Ctrl/Cmd+click became a showConnectivity option supplied by the
  click handler.  lastSelectionBounds now derives from data.bbox rather
  than selected[0].bbox, for the reason master gives at that call site: for
  a net the two are different rects.
- pulseHighlight/highlightBBox keep the "don't outline a net's full extent"
  skip alongside master's animation teardown and multi-selection branch.

Fixes for breakage the merge introduced without conflicting:
- drawColoredPolygons still took (dbu_tile, scale) after master moved every
  draw helper to TileFrame.  Converted, using toPxXd/toPxYd and
  penWidthCss so an oblique edge keeps its slope.
- handleFind called the pre-merge ScopedDbuFormat, writeInspectPayload and
  collectMultiHighlightShapes signatures.
- saveImage's split into renderImagePng dropped master's WEB-0024 text, so
  renderImagePng now reports the encoded dimensions through optional
  out-params and the message stays "Saved {}x{} image to {}".
- highlight_group_polys was missing from two stale-selection cleanups.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Clearing a selection or a highlight from the canvas context menu left the
Select/Highlight panel listing objects the server had already dropped.

master drives that panel through one hook, app.refreshOverlay
(scheduleRefreshOverlay in main.js), which refreshes the overlay tiles and
then calls selectionBrowser.scheduleRefresh().  Every other mutation site
uses it: the menu bar's "Clear Highlights", the panel's own toolbar, and the
selection_invalidated push handler.  The context menu predates the hook and
ended its request with redrawAllLayers(), which calls the bare
refreshOverlay() instead -- so the overlay tiles repainted, which is why the
highlight vanished from the canvas, but the panel was never told anything
had changed.

Route every context action through the shared hooks:
- app.refreshOverlay() last, so the panel refresh sees the cleanup.
- clear_selections/clear_all also reset the client-side pieces of the
  selection the server cannot reach -- inspector, selection animation, the
  outline rectangle and lastSelectionBounds -- mirroring what the
  selection_invalidated handler does for the same server-side state.
- Everything else refreshes the inspector, for the selection count/index
  and the highlight-group badge.
- Only clear_focus_nets/clear_route_guides/clear_all still redraw the base
  tiles, which are what actually carry focus nets and route guides.  The
  remaining actions live entirely in the overlay and no longer pay for a
  full base-tile redraw.

Also drop the menu's private _toast()/.cm-toast in favour of ui-utils
showToast(), which master added for the same purpose: two transient-notice
implementations with different placement and timing was one too many.  The
notice now appears bottom-centre with every other notice, lasts 4s instead
of 1.8s, and replaces itself on repeat instead of stacking.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
… shortcut

Two problems with the Go to Position dialog.

Shift+G opened it with the X field set to "G".  The dialog focuses and
select()s its X field synchronously, so the keystroke's own default action
was then delivered to that field and replaced the whole prefilled value with
the shortcut's letter.  Ctrl/Cmd+F already called preventDefault for the
browser's find bar; Shift+G needs it for this reason, so the explanation now
sits above both branches.

The dialog also hardcoded microns -- labels, prefill, parsing and the error
message -- while the rest of the UI follows the "Show DBU" setting (Qt's
MainWindow::useDBU).  All three fields now read and write whichever unit is
being displayed, the labels say "(µm)" or "(DBU)" to match, and the Size
window is interpreted in the same unit.  The prefill picks up a second fix
from formatDbu: it was hardcoded to .toFixed(3), which is one decimal short
of distinguishing adjacent DBU on a 2000 DBU/µm technology.

Doing that needed a showDbu-aware string->DBU parse, which ruler.js already
had as a local copy of the inverse of app.formatDbu.  Rather than add a
third copy, the display-unit rules move to ui-utils as pure functions
(formatDbu, parseDbu, formatDistance, unitLabel) taking {showDbu,
dbuPerMicron}; the app object and ruler.js delegate to them.  The ruler and
search-nav test harnesses now delegate too, instead of reimplementing
formatDbu/formatDistance as they did before -- a copy in a harness can only
agree with the app by luck.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
…lette

Find now ADDS to the selection instead of replacing it.  Qt's find dialog
goes through Gui::select, which hands its matches to
MainWindow::addSelected, so successive searches accumulate and a selection
the user built by hand survives one; this handler claimed to mirror that
flow but overwrote the set.  The cycling iterator comes from the insert of
the first match, so prev/next still start from what was found.

The capture now rasterizes the ruler distance labels.  They are L.divIcon
<div>s in a marker pane rather than SVG, so the SVG-pane pass could not see
them and both Visible and Entire exports kept a ruler's line and ticks while
dropping the measurement it exists to show.  They are painted from their
computed style rather than serialized through an <img> data URL like the SVG
panes: a detached document does not inherit the app's stylesheet, so a
foreignObject copy would come out with no background and a default font.

Two review comments needed no code change:
- `Color bg` is already value-initialized, and Color declares `= 0` on every
  member, so even plain `Color bg;` runs the implicit default constructor and
  is zero -- not the uninitialized POD the comment describes.
- The capture background already guards the transparent cases before falling
  back to #111.

One is declined, with a comment recording why: dark_magenta and purple are
both #800080 because gui::Painter::kDarkMagenta and kPurple are defined with
identical RGB in gui.h.  The swatch has to show what the server will fill
with, so "correcting" this one to #8b008b would make the menu lie about the
result.  The duplication is real, but it has to be fixed in gui::Painter
first.

FindHandlerTest's findCount() now clears the selection between searches.  It
asserts pattern matching only, and accumulation means two of the fixture's
null-descriptor Selecteds would otherwise be ordered against each other --
Selected::operator< dereferences the descriptor once the payload types
match.  The new accumulation test pre-seeds a FakeDescriptor-backed entry,
whose differing payload type keeps that comparison off the null pointer.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
…ebGUI-contextmenu-search-navigation

feat(web): add right-click context menu and search/navigation to Web GUI
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.