Conversation
Both fields were reachable only through the Object Defaults preferences page, where they persisted but could never be applied: the engine had no setter for either. With openswmm.engine exposing swmm_subcatch_set/get_zero_imperv_pct and a drying_time argument on the Curve Number accessors, wire them up. - SWMMSubcatchPropertyAdapter gains pctZeroImperv and cnDryTime. Curve Number and its drying time share one engine setter, so both writers funnel through writeCurveNumber(), which re-reads its sibling and restores the infiltration model code the setter would otherwise stamp to CURVE_NUMBER. - Attribute table gains "Zero-imperv %" and "CN dry time" columns. The Curve Number column moves off the raw engine function pointers onto cnSet/cnGet wrappers, matching the existing Horton and Green-Ampt pairs. - The properties panel greys the drying-time row with the rest of the Curve Number block. - ObjectDefaultsApplier now writes both, so the two preferences controls finally reach newly created subcatchments.
Adds the two pieces CMakeLists.txt already referenced but that were never committed, so a clean checkout of the previous commit did not configure. meshcellparams defines one table of the editable per-triangle 2D parameters and the read/write dispatch every editing surface shares, so a new parameter is added in exactly one place. It drives the mesh-editing toolbar selector, the Cell Data assignment dialog and the undo command's value plumbing. Parameters awaiting engine support (the 2D two-zone groundwater set) are listed disabled so they show greyed rather than missing. meshcommands makes those writes undoable. One command covers a whole selection or a whole raster/shapefile assignment, so a bulk write is a single Ctrl+Z. Writes funnel through SWMM2DMeshLayer::applyMeshTriangle*, which emits attributeChanged and keeps the toolbar, properties panel and map in sync regardless of which surface made the edit.
test_landuse_unified_editor.cpp and test_meshenginesync.cpp were committed without the data they load, so both fail on a clean checkout. Adds the six land-use .inp decks (round-trip, rename, pollutant add, impact delete, tab handling, external-guard) and the mesh_sync_fixture.oswp project.
Testing/Temporary/{LastTest.log,CTestCostData.txt} is rewritten by every
ctest run and showed up as untracked each time.
Audit of the mesh pipeline for degeneracies reachable with a very large DEM (up to 20 GB) that is non-rectangular, carries NoData, and is only partially covered by the drainage network. Twelve crash-class defects, all on paths that input combination turns on. Segfault / terminate: - dtmthinner sampleMany re-evaluated anchorFor() per point in the strip loop but discarded its verdict. At the floor() range-test boundary FP contraction can flip that verdict between the two inline expansions, leaving the Anchor unwritten and indexing the strip buffer with indeterminate rows — a wild read scaled by the raster width. A network straddling the DEM edge generates exactly these edge-exact queries. - meshgenerator wrote through six unchecked std::malloc results while packing Triangle's input (~1.3 GB contiguous at the point cap). malloc returns NULL rather than throwing, so the pipeline's bad_alloc guard never saw it. - A freshly generated mesh was committed with deferHeavyGeometry=false, running the full multi-GB scene-geometry build synchronously inside a GUI slot; the file-open path already deferred it. Both a UI freeze and an uncatchable bad_alloc. - watcher->result() rethrows a worker exception on the GUI thread; the deferred-geometry, overview and mesh-open handlers left it uncaught. Out-of-bounds and undefined behaviour: - Triangle's output vertex indices are now validated once at copy-out. reorderMeshHilbert and the NoData coverage fill index vertex arrays with them unchecked, so a corrupt index was a heap write, not a clean failure. - The coverage-fill CSR build (NoData-only path) gained bounds checks and 64-bit offsets; 6 * nTriangles overflowed int at ~358 M triangles. - fillBandGrid computed its buffer offset as int * int, which wraps negative once a strip spans ~50k rows of a wide raster — reachable because rotated or sheared geotransforms bypass the row budget entirely. Offsets are qsizetype and the read is capped with an actionable message. - buildMeshOverviewData narrowed an unbounded aspect ratio to int. A sliver bbox, plausible when a mostly-NoData DEM leaves a thin usable strip, saturates to INT_MAX on ARM64 and asks for a 17 GB grid. - DTMSampler::sample narrowed to int before its range test, the UB already fixed in DTMThinner::sampleAt. PSLG degeneracies specific to an irregular domain: - Intermediate constraint vertices were clipped against the domain bounding box rather than the ring, so a conduit bulging outside a non-rectangular boundary carried segments across it — the non-planar PSLG Triangle aborts on. - Exterior rings are validated after RDP simplification, as hole rings already were, falling back to the unsimplified ring. Only RDP can break a GEOS-produced ring, and a self-intersecting outer ring floods the exterior carve. - The domain ring packer counted its first vertex twice, letting a polygon that quantised to two distinct vertices emit the degenerate reversed pair (a,b),(b,a) as a closed ring. Closure is now gated on emitted segments. - Triangle's longjmp buffer is thread-local; two concurrent triangulate_safe() calls would otherwise clobber each other's setjmp frame and longjmp onto a dead stack. Tests: edge-exact mixed-batch sampling parity, minimal and collapsed ring closure, far-outside-raster sampling. Suite 167/167.
…assignment Replace the mesh toolbar's lone Manning's spin box with a parameter selector beside a value editor, so any per-cell 2D attribute can be prescribed to a selection from one pair of widgets. The selector, the new Cell Data dialog and the cell property adapter all read one registry (mesh/meshcellparams.h), which is deliberately layer-free and read-only: writes live in map/meshcommands.cpp and funnel through SWMM2DMeshLayer::applyMeshTriangle*, so every view refreshes and every edit lands on the undo stack no matter which surface made it. Adding a parameter is now one registry row plus one dispatch arm. Cell Data assigns a parameter across the mesh from a raster sampled at cell centroids (band, scale, offset) or from a polygon layer's attribute field matched by centroid containment, scoped to all or selected cells. Preview reports how many cells would receive a value and why the rest were skipped; Apply writes the whole assignment as a single undo entry. Groundwater (2D) previews the per-cell two-zone editor against the engine's draft [2D_AQUIFER] design (Ks, zs, theta_s, the four soil models with their extra parameters, the four closure modes, initial hu/hg). The engine kernel does not exist yet, so every input is disabled behind an explanatory banner and the gw.* registry keys are listed disabled — visible roadmap, no silent data loss. Activating it later is a flag flip plus dispatch arms.
…atching Three gaps the INIT_DEPTH work exposed: - test_meshcellparams: the registry contract every editing surface depends on — live parameters first and enabled, gw.* disabled with a stated reason, NaN for unset attributes (so callers can tell "unset" from "explicitly zero"), and length labels carrying the project unit rather than a hardcoded metre. - test_inpmeshwriter: [2D_TRIANGLES] columns are positional, so a depth or tag cannot be written without a MANNINGS_N token. Assert an edited depth survives a row that has no Manning's on either side, and that a depth-only edit leaves a Manning's the file already carried untouched. - test_mesh2dgroundwaterdialog: hold the preview dialog to being a preview — no editable input while the engine kernel is missing — and pin the soil and closure vocabularies, which become INP tokens once it lands. test_meshenginesync also now asserts the initial-depth push, which round-trips verbatim on an SI deck because INIT_DEPTH carries the mesh's length units.
Follow-up audit for segfaults reported on Windows at large cell counts. The primary defect is platform-specific and had nothing to do with the degeneracies fixed in 0cc4543. Triangle keeps a triangle's address and its orientation in a single pointer, tagging the orientation into the low two bits, and every encode/decode round-trips that pointer through `unsigned long`: (otri).tri = (triangle *) ((unsigned long) (ptr) ^ (unsigned long) (otri).orient) `unsigned long` is 32 bits on 64-bit Windows (LLP64) and 64 on macOS/Linux (LP64), so on Windows alone this discards the high half of every heap pointer. Shewchuk documents the assumption at triangle.c:849. That is invisible while the pools sit below 4 GiB, which is where the Windows heap serves small blocks from. Triangle's first element-pool block is 144 bytes per input point, so past a few thousand vertices it exceeds the large-block threshold and comes from NtAllocateVirtualMemory, which under ASLR routinely returns addresses above 4 GiB. From that point every decode yields a garbage low-memory pointer: access violation, or corrupt topology that surfaces later as out-of-range vertex indices. Small mesh fine, large mesh dies, Windows only. Converted the 40 pointer-carrying casts to uintptr_t — the encode, decode, sencode, sdecode, infect, uninfect and infected macros plus the seven `alignptr` pool-alignment locals. The printf("%lx") debug casts and the randomseed arithmetic are left alone; neither holds a pointer. On LP64 `unsigned long` and `uintptr_t` are the same width, so this is a no-op on macOS and Linux and cannot shift their output. MSVC has been reporting this all along as C4311/C4312, so those warnings are now deliberately left enabled for triangle_lib rather than blanket suppressed, and a regression fails the build instead of the mesh. Also fixed, found in the same pass: - The point guard bounded on the vertex pool (32 bytes/point). The binding pool is the element pool: initializetrisubpools passes 2*invertices-2 items of 72 bytes, i.e. 144 bytes per input point, so the ceiling was 4.5x too permissive. Now 14,913,080, and re-documented as a fail-fast on a >2 GB contiguous request rather than an overflow guard, since the arithmetic itself is now size_t. - trimalloc took an int and passed it to malloc through an (unsigned int) cast. Both truncate, and the cast is the worse half: a caller's overflowed product arrived negative and became a plausible ~2 GB request that SUCCEEDS on a large-memory machine, returning a block far smaller than Triangle then writes into. Silent heap corruption, made more likely by having more RAM, not less. size_t throughout, along with the call-site casts and the two int*int pool-block products that wrapped before any cast applied. - The natural-neighbour interpolator called triangulate_safe with no point bound at all; its 'n' switch widens elements to ~112 bytes per seed. Same guard applied. - randomseed was a plain global while the longjmp buffer had already been made thread-local for the same reason: triangulate_safe runs concurrently on the mesh worker and the interpolator, and each run resets the seed in triangleinit(). Windows stack reserve raised to 8 MiB. Threads there default to 1 MiB from the PE header and Qt passes dwStackSize=0, so QtConcurrent workers inherit it — against 8 MiB for the macOS main thread the code is routinely exercised on. triangulate() alone puts ~81 KiB of bad-triangle queues in one frame before any recursion, and vertexsort recurses on both partitions with no smaller-half depth guard. Reserved address space, not committed memory. 32-bit Windows configures now hard-error: nothing pinned the architecture and the Ninja presets inherit whatever cl.exe is on PATH. Suite 169/169. Note this cannot be verified on macOS by construction — the conversion is provably inert on LP64. Windows testing required.
SceneTri::dv0/dv1/dv2 carries a signed depth eta-z whose value exactly 0
is a NO-DATA sentinel. Only the profile path knew that; every map path
read it as a plain depth and failed two ways on an adverse bank:
- the QSG fills gate on the CELL-MEAN depth, so a solver-dry cell
holding the pooling wedge emitted no geometry and the inundation
truncated at the cell edge;
- the marching bands/isolines are ungated but interpolated the
sentinel linearly, dragging the waterline out to the dry vertex --
painting 1.5 m of water on bed standing ABOVE the pool feeding it.
The scalar that is linear on a triangle is eta-z, not clamped depth: the
bed z is linear by construction and the extrapolated eta is constant in
the dry direction. extrapolateDryCorners fills a partially-wet cell's
dry corners with maxEta - z_k (negative above the pool), applied per
triangle in applyCurrentDepths_, so the marching passes reproduce
max(0, eta - z) exactly and cut the shoreline on the true sub-cell bed
intercept. Bands, isolines and the CPU painter twin need no extractor
change; the expanded per-vertex fill gets a strictly additive gate.
Fully-wet and fully-dry cells stay byte-identical, no new SceneTri
fields, and the profile path is bitwise unaffected on the canonical bank
(an extrapolated corner below the pool is non-supplying in
CellSurfaceInterp exactly as the sentinel was).
The two indexed QSG fills keep the strict gate deliberately: they key on
shared vertices, where per-corner surfaces collide and a max-reduce
would stamp a deep pool's driving head onto a thin film's triangle
across a ridge.
Measured (pool eta=5, bank z 0->8 in one dry cell, true waterline
x=16.25): painted waterline moves from x=20.000 to x=16.250.
Two defects in 70d16f9, both visible as water smeared across dry land in the live view and the fills. 1. Extrapolation ran downhill without limit. maxEta - z_k at a dry corner whose bed sits BELOW the driving head is large and POSITIVE, so a dry cell touching a pool was handed metres of standing water and the pool spread one cell in every direction -- the flood-fill behaviour this feature deliberately does not do. Only the adverse-slope case (d <= 0, the bed rising into the cell) carries pooling geometry; where the bed falls away the solver's dryness is meaningful and the sentinel stands. That also restores the invariant the rest of the change relies on: extrapolation can never turn a corner wet, so it cannot flip the fill gate or CellSurfaceInterp's supplying set. 2. The expanded Gouraud fill clamped instead of fading. ClassificationScheme::colorAtF clamps its ramp position to [0,1], so a negative corner value painted the ramp's SHALLOWEST colour at full opacity rather than vanishing. 70d16f9 asserted these would "render at the ramp's transparent low end" -- they do not. Corners below the wet/dry threshold now emit transparent, so the Gouraud pass fades to the shoreline; the exact cut remains the marching band pass's job. The two indexed fills are unaffected: they clamp a 0 sentinel and a negative value to the same ramp edge, so their output is unchanged. Pinned by doesNotExtrapolateDownhill and extrapolationNeverCreatesAWetCorner. Canonical bank still lands the waterline at x = 16.250 (true 16.25); 170/170 green.
The colour ramp was anchored to the run's absolute peak depth, so a few very deep cells -- a coupling spill, a pit, a pond -- stretched the scale until ordinary water sat in the bottom few percent of the palette and read as dry ground. On a Bellinge run with a 9-10 m peak the median wet cell (0.36 m) landed at 3.6% of the ramp. Histogram the wet cell depths over every frame during the load scan and anchor the ramp at the 98th percentile instead. That trims the extreme tail only, so genuinely deep water still reads deep; the value never exceeds the true peak and falls back to it when the sample is too small. Fixed bins keep this a single pass. Seek still lands on the true peak frame, which is the interesting one. Applies to loaded results: setMaxDepth marks the range user-set, so the clip wins over the layer's own scan. The live path still grows max_depth_ toward the running maximum -- a streaming percentile is a separate change.
The GUI runs the engine in-process, so relative sidecar paths named in the .inp resolved against whatever directory the .app was launched from rather than the model folder. Pin the cwd to the .inp's directory for the duration of the run and restore it after, via an RAII guard at the top of the worker lambda so it covers the 6.0 in-process path, the 5.x legacy worker, and every early return. Hardening, not a fix for an observed failure: the engine's resolve_external_file_slots already anchors gage and timeseries paths against the .inp directory, so no known case depends on this. It matches what legacy does via addAbsolutePath and closes the gap for any consumer that opens a raw relative token. QDir::setCurrent is process-global, so concurrent runs from different folders would race. Runs are launched one at a time today.
Assigning a boundary condition means selecting a long run of boundary edges — an outfall face, a road crest, a domain perimeter — and clicking each one was the only way to do it. Adds mesh::MeshBoundaryGraph, a boundary-edge connectivity graph built from the layer's existing per-slot boundary flags, and routes between two edges with a multi-source Dijkstra weighted by geometric length (not hop count, so a mesh that is fine in one place and coarse in another follows the physically shorter arc). The graph is built lazily on the layer and cached until the boundary flags are rebuilt; during a progressive load it reports empty rather than caching the half-loaded state. In the edge tool, Ctrl/Cmd is repurposed from Toggle to path picking. The run starts from the edge the tool last selected on its own — a plain click, or the far end of the path it just committed — so consecutive Ctrl-clicks keep extending the run; failing that, from a lone selected edge on the mesh, whichever view picked it. With nothing to start from, the first Ctrl-click drops an anchor instead and the next one commits. A box select clears that memory, since "the selected edge" is then ambiguous. Also fixes three selection paths in the same tool (right-click select, Escape-clear, anchor set) that invalidated Overlay alone. The mesh highlight is layer content behind the cached QSG framebuffer, so those repainted from stale caches and the highlight only appeared after a zoom. Tests: test_meshboundarygraph covers the shorter arc, disconnected loops, start == end, a marker-tagged degree-3 vertex, non-boundary slots, and a case where the shorter route has more edges than the longer one. test_meshboundarypath covers the layer accessor, cache stability and the deferred-load contract.
…tribute Table Adds three tables per loaded 2D mesh to the Attribute Table dock, and makes every mesh attribute edit undoable no matter which surface makes it. Undo (map/meshcommands): MeshSetVertexAttributeCommand (id 45) and MeshSetEdgeAttributeCommand (id 46), plus push helpers mirroring pushCellParamEdit. Both snapshot the WHOLE element rather than the one changed field: clearing a coupling makes the layer reset Cd/Area to the engine defaults, and the BC fields are interdependent through `type`, so a partial restore would leave state the user never saw. Conveyance dispatches through applyMeshEdgeConveyance in both directions so the interior-edge mirror survives undo. The mesh-editing toolbar's seven commit slots now route through these helpers, so toolbar edits become Ctrl+Z-able. No UI change there. Tables (ui/panels/meshattributetablemodel): Fully virtual over SWMM2DMeshLayer — no shadow copy, reads go straight to mesh()/edgeBCs(), writes go through the push helpers. An interior edge owns two slots but gets one row (the lower slot); either half resolves to it. Cost is the one-time O(3T) row index, so a multi-million-cell mesh is fine. The Edges view stays empty until sceneGeometryReady() — pairing needs the vertex adjacency a progressive load builds in the background — and the combo entries are greyed until then. Referential integrity: Coupled Node, Time Series and Rating Curve are closed pickers over what the model actually defines (DataObjectPickerEditor), so a mesh cannot cite an object that isn't there. Adds DataObjectRef::Node for the coupling target. The curve picker is deliberately NOT narrowed to CURVE_RATING: the engine resolves it against the whole table namespace (SurfaceRouter2D), so filtering would hide choices it accepts. Without a bound model the three degrade to plain text rather than offering an empty dropdown. Within a boundary edge only the parameter its BC type reads is editable; the rest render as inapplicable. Mirrors the toolbar's contextual param widget, and keeps a boundary from carrying a value the engine never reads. Panel: Fourth source model alongside SWMM / tabular / GIS. Query bar, selection ops, bulk apply, Copy, CSV export, zoom-to and two-way selection sync all work against it; delete and change-type stay off (a mesh element has neither). Column widths persist per element kind, since layerId is a fresh UUID each session. x/y and the derived columns (edge length, cell area, centroid) are read-only — move a vertex on the map instead. Coupling Area is labelled (m2) rather than resolved through UnitKind: the engine column is metres-squared regardless of the project's flow units. Tests: tests/gui rather than tests/unit — the commands need the real SWMM2DMeshLayer, which tests/unit (Qt6::Core + GTest only) cannot link. Panel wiring stays manual-smoke; MapCanvas is not constructible offscreen.
…he stats The Cells attribute table's Area column arrived with its own copy of the shoelace formula, duplicating what meshcellstats.cpp already computed for the layer-properties Metadata tab. Two definitions of "cell area" that can drift is the wrong shape for a number people use to hunt slivers, so extract mesh::triangleArea(mesh, tri) and have both read it. Adds a test pinning the two together: every row of the table reports the same value the statistics pass measured, and the table's min/max/mean match CellAreaStats on a mesh deliberately given a sliver. No behaviour change — the column already computed the same number.
The Windows installer shipped an application whose CRS pickers enumerated zero coordinate reference systems. Four independent causes, none of which surfaced as an error: - The portable GDAL/PROJ staging block was wrapped in two silent guards (VCPKG_INSTALLED_DIR defined, share trees EXISTS). If either failed, configure, build and CPack all succeeded and the installer simply had no proj.db. Both guards now report: FATAL_ERROR on Windows, where there is no system PROJ to fall back on, WARNING elsewhere so a non-vcpkg configure stays buildable. - Nothing verified the packaged payload. CI gated on the legacy worker only, so a missing proj.db reached release artifacts unnoticed. Adds a "Verify GUI bundles PROJ/GDAL data" step between Install and Package that asserts proj/proj.db and gdal/gml_registry.xml exist in the install tree, matching both the bin/ (Windows, Linux) and Contents/Resources/ (macOS) layouts that setupBundledGisDataPaths() probes. - SWMMVisApplication built the main window in its member-initialiser list, i.e. before the constructor body published GDAL_DATA/PROJ_DATA. The SWMMVis constructor pumps events and can create a MapCanvas, and PROJ caches its data search paths when a PJ_CONTEXT is first created, so any context born in that window never saw the bundled data. A system-installed PROJ masked this on macOS and Linux; Windows has none. mSWMMVisGUI now starts null and is allocated after setupBundledGisDataPaths(). - CRSManager::queryDatabase took constData() off the temporary QByteArray returned by authority.toUtf8(), leaving authFilter dangling before OSRGetCRSInfoListFromDatabase read it. Undefined behaviour, and MSVC release reuses that memory far more readily than clang. The buffer is now held in a named QByteArray. Also adds a one-shot diagnostic when the database yields nothing, logging the executable directory, PROJ_DATA, whether proj.db actually exists at that path, and GDAL_DATA — enough to tell "data not deployed" from "deployed but not found" without a debugger.
Several Simulation Options round-trips were broken, so changed values
reverted the next time the dialog opened:
- REPORT_STEP / ROUTING_STEP: the engine renders step values as
std::to_string(double) ("900.000000") but the read path parsed
integers only, silently substituting the preferences default and
writing it back on OK. parseStepSeconds (new static helper, unit
tested) accepts plain seconds, decimal seconds and H:MM:SS.
- MINIMUM_STEP: required engine-side support (engine 2932a5b0 adds the
key to swmm_options_get/set); no dialog change beyond the shared
parse hardening. The "Apply fast preset" button now fully applies.
- LAT_FLOW_TOL / SYS_FLOW_TOL: the options API now speaks percent on
both get and set (engine 2932a5b0), so the dialog drops its
fraction conversions - previously each OK shrank the tolerance 100x.
- RULE_STEP: the QTimeEdit clamped values to 23:59:59 on read and wrote
the clamp back; now a QCustomTimespanEdit (days + HH:mm:ss) like the
other step rows, so 48:00:00 survives.
- Unchecked toDouble/toInt parses in readFromEngine/read2DFromEngine
seeded 0 into spin boxes on unparseable engine strings and then
persisted it as a real edit; numeric reads now keep the fallback
(optDouble/optInt/extDouble/extInt).
- writeIfChanged compared formatted strings ("0.00" vs "0.000000") so
every OK rewrote every key and dirtied the project with no edits;
optionValueEquals (unit tested) compares numerically.
Tests: helper cases for parseStepSeconds/optionValueEquals; engine-ABI
round-trips for MINIMUM_STEP, percent-idempotent tolerances, >24h
RULE_STEP and THREADS (0 = auto) in test_options_hydration_contract;
new test_simoptions_persistence_contract pins the full
open -> set -> swmm_model_write -> reopen -> get disk round-trip.
Requires an engine install with 2932a5b0.
…tual options Adds Finite Volume (FLOW_ROUTING FV, the explicit Godunov solver) to the routing combo in Simulation Options, Preferences defaults, and New Project. Two new groups on Routing & Hydraulics expose all 18 engine FV_* keys (mesh, numerics, transport, coupling, performance), enabled only while FV is selected; the limiter follows 2nd order and LTS tiers follow the LTS toggle. applyEngineConstraints greys the FV item out on engines that predate the solver — probed via FV_CFL, since the string-keyed C ABI is otherwise indistinguishable across builds. Contract tests pin the canonical FV token round-trip (FINITE_VOLUME is an .inp-parser alias only), all 18 key round-trips in the exact string forms the dialog writes, and bad-enum-token rejection.
…header Double-clicking the row-number strip zooms the map to that element plus the rest of the selection, through the existing zoom-to-selected path (SWMM + mesh extents, CRS projection, point/areal padding). Unselected rows are selected first so the zoom and the selection bus stay in sync. Lives on the vertical header so cell double-click editing is untouched; tabular/GIS sources carry no refs and stay a natural no-op.
…r kind
Clicking a SWMM kind sub-row ("Storage", "Conduits", …) in the Layers
panel now selects and scrolls to the matching category in the Object
Browser. New LayerTreePanel::kindSelected(layer, kindOrdinal) fires
alongside layerSelected (which collapses kind rows to their parent);
SWMMVis forwards it for the active project's model layer only.
ObjectBrowserPanel::selectCategory holds the applying-from-bus guard while
it moves the tree selection — a category header resolves to zero object
refs, so an unguarded change would push an empty Replace onto the
SelectionManager and wipe the user's map selection.
The Plot Time Series toolbar action (Ctrl+T) now opens PlotVariablePickerDialog: the 14 system variables plus one checkable group per selected node/link/subcatchment, with tri-state group cascade, filter, Select All/None/Invert, and per-.out availability gating (unsupported attributes disabled with a tooltip). OK bulk-adds every checked series — the old flow armed a two-click pick, popped context menus, and silently plotted only the first selected feature. Right-click entry points keep their quick single-attribute menus. Supporting refactors: the per-kind and system attribute enumerations that had drifted across four call sites now live once in plot/plotattribute (attributesForKind dispatcher in irunlayer.h — the nested ObjectRef::Kind cannot be named in plotattribute.h without a cycle), and the six verbatim copies of the ComparisonPlotDialog find-or-create block collapse into SWMMVis::ensureComparisonPlotDialog(). The long-dead two-click machinery (mPendingPlotTimeseriesPick, onPlotTimeSeriesPickComplete) and the empty systemvariablepickerdialog.h stub are removed. The system menu order in onAddSystemSeriesClicked unifies on the canonical Rainfall-first order. test_plotvariablepickerdialog pins the shared lists (6/5/5/14, canonical order anchors, dispatch) and the dialog's tree, gating, and selection behaviour against a stub run layer.
File → New no longer writes a synthetic .inp into the temp directory — the project is a blank BUILDING-state engine (swmm_engine_new) stamped with the preference defaults through the options C API (SWMMModelLayer::createBlankEngine/adoptNewEngine, replacing synthesizeBlankInp). Creation is synchronous: a blank engine builds instantly, so openUntitledProject skips the async hop and the file-open bookkeeping; window construction is factored into createProjectWindow, shared with openSingleINP (which now guards empty paths — the QFileInfo dedupe treated every pathless window as "already open"). Closing a never-saved project always prompts — even pristine — with Save As… / Discard / Cancel; Save As runs the real path-picking flow (hoisted into SWMMVis::saveProjectWindowAs so the filter normalization and last-filter memory stay single-source), replacing the old "use File → Save As before closing" dead-end. The app-quit path gets the same guard. First Save As promotes the window and renames the layer. Pathless fixes in adoptOpenEngine: the layer keeps its "Untitled" name, and the CRS derives Local (ft)/(m) from the engine's flow units instead of scanning the .inp — so the CRS picker can never interrupt File → New. Known gap: the engine has no setter for [MAP] UNITS, so a first-saved .inp carries Units None; the .oswp sidecar preserves the CRS on reopen. Blank models must never run swmm_finalize_model (validation demands a node and an outfall); swmm_model_write works from BUILDING as-is. test_asyncload grows coverage for the defaults round-trip, the BUILDING-state write + reopen, the Local-CRS derivation, the always-prompt close (Cancel keeps, Discard closes), and the object-browser category sync bus guard.
…assignment Implements workplans/LOCAL_RASTER_BASEMAP_PLAN_2026-08-09.md. Add Basemap gains a Local File tab: pick a GeoTIFF/PNG/JPEG/BMP, optionally a world file and a CRS (CRSSelectionDialog); georeferencing persists through the driver's native update path or a merged GDAL PAM .aux.xml sidecar (io/rastergeoref — world-file parse with the pixel-center → corner GeoTransform shift, probe, authCodeToWkt), so every later plain GDAL open — initial open, pooled tile-warp handles, overviews, project restore — sees it with no raster-layer changes. The layer is a GISRasterLayer flagged setIsBasemap(true), retagged to the Basemaps tree category and rendered through the existing tile pyramid. Connections persist under QSettings BasemapConnections/localraster (no auth); the .oswp serializer round-trips a "localraster" entry by relative path and skips a missing file with a warning instead of crashing. serializeBasemapLayer/deserializeBasemapLayer thread the .oswp path for that. The hidden-since-June Add Basemap action is re-enabled — the Local File tab has no other entry point, the divergence the hiding comment anticipated — and opens preselected on that tab; the service tabs keep their own actions. 11 unit tests cover parsing, the corner shift (incl. rotation terms), sidecar candidates, and an end-to-end PAM write + plain-reopen verify on a generated PNG; fixtures land in test_artifacts/localraster/.
Adding non-spatial data objects (time series, curves, patterns, ...) never refreshed the Object Browser: the tree's only live trigger was geometryChanged (spatial-only), while the editor dialogs stage providers in typed registries at submit and defer the engine flush - and saveToEngine never deletes engine rows, so engine-sourced counts were also wrong after a delete. - SWMMModelLayer::dataObjectsChanged(), emitted from all 11 typed registries' providerAdded/AboutToBeRemoved/Renamed (connected once per registry instance, after the initial engine seed) and from createDataObject, which now also mirrors direct engine adds into the live registry via the idempotent loadFromEngine. - dataObjectCount/dataObjectNameAt are registry-preferred with engine fallback, gated by a single liveRegistry helper so counts and names never mix sources; staged objects appear at dialog submit, deleted ones disappear despite the lingering engine row. - SWMMObjectTreeModel subscribes via a 0-ms coalescing scheduleReload() (also wired to transectChanged/controlRulesChanged); bursts collapse to one reset and pre-removal signals read post-mutation state. - ObjectBrowserPanel guards modelAboutToBeReset/modelReset so model-initiated reloads no longer wipe the map selection. - LayerTreePanel kind-count labels repaint on geometryChanged; AttributeTablePanel refreshes (queued) on dataObjectsChanged. - test_objectbrowser_tree_refresh: 6 cases pinning staged-add visibility, delete-despite-engine-row, rename, burst coalescing, and the createDataObject paths.
The BC combos (stage/flow TS, rating curve) and the coupled-node dropdown re-queried their listers only on project-tab switch or after the toolbar's own picker closed, so a time series added from the Object Browser or an editor dialog never appeared for selection. The active project's model layer now drives the toolbar: dataObjectsChanged -> refreshBCNameLists (queued, so pre-removal emissions read post-mutation state) and geometryChanged -> refreshNodeList. Current combo text is preserved through repopulation. Other TS pickers (property panel, attribute table, NodeCompoundEditDialog) already query at pick time and needed no change.
cbuahin
added a commit
that referenced
this pull request
Aug 14, 2026
…yyyy HH:mm The editor's time column handed Qt a bare QDateTime, so it rendered in the system locale's short form (8/14/26 5:06 AM) and edited through the default delegate — no calendar, and a format that matches neither the .inp nor the rest of the dialog. Times now read and edit as MM/dd/yyyy HH:mm, the same stamp [TIMESERIES] carries, through a calendar-popup QDateTimeEdit. One format for the whole editor: the grid, the rotate-pivot and scale-anchor fields (were yyyy-MM-dd HH:mm), the t-range readout (was ISO 8601) and the chart's time axis all take it from core::swmmDateTimeDisplayFormat(). Minute resolution hides a seconds field that SWMM does store — the engine's [TIMESERIES] writer emits HH:MM:SS — so the delegate seeds the editor with the full QDateTime and relies on QDateTimeEdit preserving the sections its display format omits. editingAStampPreservesHiddenSeconds pins that: a stamp at 00:15:30 whose minute is bumped commits 00:16:30, not 00:16:00. Silently zeroing it would be the defect class GH #1 was about. The format constant lives in its own header, not in swmmdatetime.h: that one includes the engine's openswmm_datetime.h, and unit targets like test_timeseries_table_model compile the model with no engine include path. Clipboard copy still writes ISO 8601 — it is an interchange format, and the paste parser accepts both.
cbuahin
added a commit
that referenced
this pull request
Sep 17, 2026
…yyyy HH:mm The editor's time column handed Qt a bare QDateTime, so it rendered in the system locale's short form (8/14/26 5:06 AM) and edited through the default delegate — no calendar, and a format that matches neither the .inp nor the rest of the dialog. Times now read and edit as MM/dd/yyyy HH:mm, the same stamp [TIMESERIES] carries, through a calendar-popup QDateTimeEdit. One format for the whole editor: the grid, the rotate-pivot and scale-anchor fields (were yyyy-MM-dd HH:mm), the t-range readout (was ISO 8601) and the chart's time axis all take it from core::swmmDateTimeDisplayFormat(). Minute resolution hides a seconds field that SWMM does store — the engine's [TIMESERIES] writer emits HH:MM:SS — so the delegate seeds the editor with the full QDateTime and relies on QDateTimeEdit preserving the sections its display format omits. editingAStampPreservesHiddenSeconds pins that: a stamp at 00:15:30 whose minute is bumped commits 00:16:30, not 00:16:00. Silently zeroing it would be the defect class GH #1 was about. The format constant lives in its own header, not in swmmdatetime.h: that one includes the engine's openswmm_datetime.h, and unit targets like test_timeseries_table_model compile the model with no engine include path. Clipboard copy still writes ISO 8601 — it is an interchange format, and the paste parser accepts both.
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
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.
No description provided.