Repository navigation
Conversation
Signed-off-by: Caleb <calebgh@gmail.com>
Signed-off-by: Caleb <calebgh@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
- Disable QSG/Metal renderer by default (m_glRenderingEnabled=false) and hide QQuickWidget so its CAMetalLayer never occludes the raster framebuffer; restores CPU paint path for SWMM feature rendering and yellow selection highlight - Fix SWMM 2D mesh CRS: build OGR transform in onCanvasCRSChanged() and reproject mesh edges to canvas CRS before populating QGraphicsScene - Fix GISRasterLayer warpToCanvas(): two-stage GDAL transformer fallback when source dataset has no embedded CRS - Add isBasemapLayer() virtual to OpenSWMMVisLayer; XYZTileLayer overrides true so fullExtent() skips world-spanning basemaps; DTM/WMS included - Set world extent (EPSG:3857) on XYZTileLayer at construction - WMSLayer/WMTSLayer: auto-select first layer/style/format on setServiceInfo(), populate extent and SRS from GetCapabilities bounding box metadata - Add undo stack, preferences manager, layer tree, object browser improvements Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
Signed-off-by: cbuahin <caleb.buahin@gmail.com>
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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const std::string meshPath = base + ".2dm"; | ||
| const std::string inpPath = base + ".inp"; | ||
|
|
||
| std::FILE *mf = std::fopen(meshPath.c_str(), "w"); |
|
|
||
| // Minimal metric SWMM wrapper so the fixture opens in the live GUI. | ||
| // FLOW_UNITS CMS + ;; UNITS: SI keeps every ft→m conversion out of play. | ||
| std::FILE *inf = std::fopen(inpPath.c_str(), "w"); |
Welcome page's Example Projects panel now understands directory-per-example bundles (.inp + .oswp + sidecars + optional example.json manifest) alongside flat .inp examples, and ships the Bellinge coupled 1D/2D model under examples/bellinge_2d/. Payload is seeded into the per-user data dir at startup (version-marker fast path) and always copied to a user-chosen folder before opening so the baseline is never opened in place.
Simulation Options → Routing (FV group): FV_NODE_DT combo (Stability limited / None) and FV_NODE_PICARD spin (1..100), hydrated from the engine with STABILITY/1 defaults and written back only when changed. The Picard spin is gated on FV_NODE_COUPLING == SEMI_IMPLICIT (explicit coupling greys it out; combo changes re-evaluate the gate). 2D tab: ADVECTION checkbox in the marcher group (convective momentum flux, engine default NO); new Output group hosting REPORT_2D plus an OUTPUT_FILE line-edit with Browse — blank means auto <model>.2d.h5 and legitimately clears the key on save. RAINFALL_MODE combo gains the NONE item; NONE projects previously hydrated to the fallback and were silently rewritten on the next save. Preferences → 2D Defaults: ADVECTION checkbox persisted through TwoDDefaults and seeded into blank engines by createBlankEngine (File → New). Range fixes so File → New stops tripping engine validation: THETA spin min 0.0 → 0.001 (engine requires (0,1]), LTS_TIERS 0..16 → 1..8 (engine rejects outside 1..8). Contract tests pin the new defaults, round-trips, the FV_NODE_DT bad-enum rejection, and the advection prefs persistence. Verified: 3/3 targeted GUI ctest (options_hydration_contract, twod_defaults_prefs, 2d_vfr_options_contract) against the rebuilt engine; dialog-written keys round-trip through .inp save/reopen.
identifyByName(), nodeIndex() and linkIndex() scanned m_nodes, then m_links, then m_catchments, then m_gages. Every caller that walks a category therefore ran O(N*M): restoring a 13 KB .oswp onto the 42k-node West Whiteland model spent 22.3 s inside rebuildKindFeatureColors, because each link had to fail a full node scan before its own scan started. Sidecar apply on that model: 22,267 ms -> 257 ms. Reusing the existing m_nameToSoa hash would have been wrong. It is single-keyed across all four kinds and, while links insert only when absent (so nodes win), catchments and gages insert unconditionally and overwrite a colliding node or link. A subcatchment sharing a name with a rain gage resolves to the gage there, where the scans resolved to the subcatchment. SWMM namespaces each kind separately, so those collisions are legal and appear in real models. Four per-kind maps preserve the scan semantics exactly: same kind precedence, same lowest-index winner. They are maintained wherever m_nameToSoa is -- rebuildCategoryIndex, renameInIndices, and all four append*SceneEntry paths. The catchment and gage appends matter: their callers do not rebuild the category index, so a map populated only at rebuild time would leave a newly drawn subcatchment resolving to nothing. Also switch the sidecar's results reopen to the openResultsAsync() that already existed; it was running swmm_output_open's header parse (427 MB on this model) on the GUI thread inside the same blocking window. Not done here: rebuildKindFeatureColors still derives a name from the row and looks it up, rather than using the SoA index it already computed. That round-trip is now O(1), and removing it would change behaviour -- objectNameAt() honours m_objectOrderOverrides while the adjacent soaIndexFor() does not, so the two disagree once a category is reordered. That mismatch is a pre-existing bug and wants its own fix. Suite: 182/182.
Opening one model rebuilt every link's scene coordinates and the link spatial grid five to six times over. Each rebuild walks all 121,902 links twice and reallocates the grid. Down to two on West Whiteland: one from buildGeometryCache, one from the first genuine CRS assignment. setSRS() and applyCRSInternal() compared POINTERS. Every caller allocates a fresh SpatialReferenceSystem, so those guards never fired and setting a layer or canvas to the CRS it already had re-emitted srsChanged and re-fanned onCanvasCRSChanged to every layer. Compare by value instead (equals() == OGRSpatialReference::IsSame). Note the project-window lambda already had an authority-string guard, but a Local CRS has no authority code, so it compared empty strings and suppressed nothing -- on exactly the models where the rebuild is most expensive. applyCRSInternal deletes the object it declines when it was handed ownership; every caller passes ownsSRS=true and none retains the pointer. rebuildTransform() now no-ops when the (layer CRS, canvas CRS) pair is unchanged. The plan called for deleting the canvas's per-layer srsChanged listener as redundant with the project window's, but it is not: it is the only path when the project-window lambda bails out early, which it does for geographic layers and when the canvas already matches the layer's authority. Dropping it would leave the layer->canvas transform stale in those cases. Making the work idempotent fixes the duplication without depending on which listener wins, and covers future call sites. Plan items 2.4 and 2.6 proved unnecessary: the value guard already makes finishModelLoad's redundant setCanvasSRS free, and the gate is met without touching rebuildSceneCoords. Suite: 182/182, including test_crschangedialog and test_layerreprojection.
A field session reached 9.6-10.6 GB. `heap` attributed 4.5 GB to 93.8 million QMacAccessibilityElement objects from libqcocoa, plus ~800 MB of NSMutableArray storage holding them -- 78% of the process heap, in MALLOC_SMALL, while all GPU memory was 357 MB. Reproduced three times; the trigger is switching between model tabs, because macOS re-walks the accessibility tree on window activation. The obvious suspect was the map scene: it carries one QGraphicsItem per network element (122,917 QGraphicsPathItem + 43,861 QGraphicsEllipseItem on West Whiteland). This test measures it and the suspicion is WRONG -- Qt does not enumerate QGraphicsScene contents as accessible children, so a 5,000-item scene reports fewer than 100. A fix aimed there would have changed nothing. The Attribute Table is the source: at 41,898 rows x 20 columns its accessible interface reports 879,879 children. 94M / 879,879 is about 107 tree walks, matching a session with 28 table refreshes plus window activations -- each walk mints a fresh wrapper per cell and the previous set is never released. Committed as measurement, not as a fix. A first attempt at deferring AttributeTablePanel::refresh() while hidden was reverted: refresh() must populate the model synchronously even when the panel has never been shown, because callers read it programmatically (selectionAsTsv), and test_selectionops caught that immediately. Suite: 182/182 before this test, 183 with it.
A stack sample of a live session pegged at 100% CPU put every one of
4,485 of 4,486 main-thread samples in one chain:
onActiveSubWindowChanged -> MeshEditingToolbar::rebindCanvas
-> refreshNodeList -> QComboBox::addItem (per node)
-> endInsertRows -> QAbstractItemViewPrivate::rowsInserted
-> -[QMacAccessibilityElement updateTableModel]
-> populateTableArray -> malloc
The combos were filled one item at a time. Each insert makes Qt's macOS
accessibility bridge rebuild that combo's ENTIRE element array, so
filling the coupled-node list -- one entry per node, 42,809 on West
Whiteland -- is quadratic. `heap` caught 93.8 million live
QMacAccessibilityElement objects holding 4.5 GB, 78% of the process
heap, with another 800 MB of NSMutableArray storage. rebindCanvas runs
on window activation, which is why the trigger was opening a model or
switching tabs, and why dwelling on a tab made the next switch worse.
The QSignalBlocker already present never helped: it silences the combo,
not its internal model, and it is the model's rowsInserted that reaches
accessibility.
Two changes. addItems() brackets the whole range in a single
begin/endInsertRows, so accessibility rebuilds once rather than N times
-- O(N^2) to O(N), applied to the coupled-node combo and the three BC
combos that shared the pattern. And rebindCanvas only populates these
dropdowns when the project actually has a mesh layer, with a stale flag
so a mesh added later still gets them; a 1D-only model was enumerating
every node, timeseries and curve to fill dropdowns nobody could open.
Earlier hypotheses this replaces, both disproved by measurement rather
than argument: the map QGraphicsScene (Qt does not expose scene items to
accessibility at all) and the Attribute Table (it does expose 879,879
cells, but is not what the sample caught).
Suite: 183/183, including all 25 mesh tests.
identifyAt's link tier walked every link and built a fresh QVector<QPointF> for each one -- 121,902 heap allocations per call on West Whiteland. It is called from the select tool's hover feedback and from the snap engine, so that ran on every mouse MOVE, not every click. Reject on the cached bounding box first. m_linkBboxes is already maintained alongside m_links, is layer-space like the click coordinates, and stores min/max rather than a QRectF so there is no top/bottom orientation trap. Four comparisons replace an allocation plus point-segment math for every link that is nowhere near the cursor; only links actually in range build a polyline. Guarded on size so a layer whose geometry cache has not been built yet still tests everything rather than silently matching nothing. Also removed two pieces of debug scaffolding that ran in the same path: a 12-operand qDebug in the link tier, the six locals that existed only to feed it, and a second trace inside cachedLinkPolyline. Both were marked "DEBUG (last-link-unselectable bug, 2026-05-25) ... remove once the off-by-one is pinned down". That bug is fixed and test_selectionops covers the last link, so this deletes them rather than hiding them behind a logging category -- gating dead scaffolding just preserves the clutter. They were formatting strings on every mouse move regardless of whether any sink consumed the output, and were most of the 4,600-line log a short session produced. Suite: 183/183.
West Whiteland's 2024 model has 10 junctions sitting up to 40,000,000 units from a network that really spans 27,000 x 22,000 -- a projection failure on one imported group. LinkSpatialGrid sized itself from the raw union of link bounding boxes, so the extent became 23.8M x 70.3M, cell size scaled up to satisfy the 1024^2 cap, and the ENTIRE real network fitted inside one cell. Every query returned all 281,049 links, so hover, hit-testing and paint each degenerated to a linear scan. That is why that file was unusable while the clean West Whiteland was fine. Grid on that model: one occupied cell -> 144 x 123, about 16 links per cell. Size from a Tukey-fenced extent (k=10 on bbox centres) instead. The fence is deliberately generous: on a well-formed model it is wider than the data, the extent is exactly the union, and nothing changes. Percentile clipping was rejected -- 10 bad rows out of 281,902 is 0.0035%, below any percentile coarse enough to be safe, so even a 0.5% clip would have discarded 1,405 legitimate links. Added an occupancy guard: median*16 is link-size aware, which keeps a long trunk from spanning hundreds of cells, but says nothing about how many links share a cell. Shrink toward ~16 per cell, never below the median link size. query() now clamps instead of intersect-and-bail. rebuild() clamps out-of-fence links into edge cells, so a query outside the fenced extent has to look there too -- otherwise the corrupt-coordinate links are stored but unreachable: present in the model and invisible to every click. The new test covers exactly that, and it is driven through the public layer API on a fixture with the same signature rather than against the private grid, so it pins what a user sees. One test expectation was wrong on first run and got corrected, not the code: identifyAt resolves nodes before links, so a click midway between two junctions 21 units apart legitimately returns the junction. Suite: 184/184.
…timers finishModelLoad never zoomed synchronously. It started a 50 ms QTimer whose only job was to check whether the canvas had a size yet, then -- inside that -- a further QTimer::singleShot(50) that walked every item in the scene to log a [postZoom] count. So a model that loads in 16 ms sat on a blank canvas for at least 50 ms before appearing, and the app kept working for another 50 ms after it did, on a 103k-node model counting 167,000 scene items to print a number. Try the zoom immediately and fall back to the retry timer only when the canvas really has no size yet (the MDI subwindow's show + resize cycle has not finished). The retry path is unchanged, cap included. The [postZoom[ sample is now behind openswmm.load.window, joining the openswmm.load.* family, so it costs nothing unless asked for. The [loadModel] line next to it moved to the same category. Suite: 184/184.
…d depth A virtual junction is a computational break point inside one conduit, not a structure, but the profile drew it as a manhole: a shaft knocked out of the soil down to the invert, its rim at the derived pipe crown. A reach split into five virtual junctions rendered as five holes punched in the ground. New NodeKind::VirtualJunction — no tube, no glyph, no soil knockout, and not click-selectable, so the pipe and the soil above it run through unbroken. The rim still shapes the ground line, and it comes from the node's rendering MaxDepth (engine rim_depth) when the model supplies one, falling back to the crown exactly as before when it does not. The value is editable in two places that write the same field: "Max Depth, Display" on the virtual-junction property adapter, and the node table's "Max depth" cell, which is now row-aware. That cell previously wrote a virtual junction's crown-derived full_depth straight through — a silent corruption of derived solver geometry. Also fixes undo of a virtual-junction delete, which restored a plain junction: the snapshot never captured the flag. It now does, and re-applies it after the cascade links are back, since the two-conduit rule cannot pass before then. toNodeKind / renderRimDepth moved to the pure half of the network adapter so both are unit-tested without the model layer. tests/manual/virtual_junction_rim carries a demo whose single profile shows a virtual junction with a MaxDepth next to one without.
… their parameters Registry rename() updated only the provider. The next saveToEngine() looked up the NEW name, did not find it, and called the *_add API — leaving TWO objects in the engine with the original orphaned along with everything referencing it (a transect's IRREGULAR links, a street's cross-section and conduits, and so on). The aquifer, inlet, LID, street, transect and snow-pack registries now call the engine's rename API first and fail the rename if it fails. A case-only rename is still allowed to bypass the duplicate-name check. Snow packs needed more than that. SnowpackProvider carried a name and nothing else — the engine's melt coefficients had nowhere to live — and saveToEngine skipped any snow pack that already existed, so edits never reached the engine at all. The provider now mirrors AquiferProvider: twenty-seven scalars in an index-addressed array whose order matches the engine's four grouped calls (PLOWABLE, IMPERVIOUS, PERVIOUS, REMOVAL), plus the optional REMOVAL destination subcatchment. The registry maps index ranges onto those calls in both directions and the editor dialog exposes them. Adds test_registry_rename.cpp — five rename-does-not-duplicate slots, the duplicate-name rejection, and the snow-pack parameter round trip. Its CMake registration was already committed without the source, so a fresh clone could not configure.
…eeps them onRunSimulation only saves when hasChanges() is true, and SimulationRunner then re-opens the .inp from disk in a fresh engine. The properties panel and the attribute table write the engine directly through property adapters, so nothing they did reached a SWMMModelLayer signal and the flag stayed false: a value typed in either view was silently dropped from the run, and the user saw results for the last saved state. Both panels' objectEdited notifications now route into the active window's layer->markEdited(). Also resolves the three .inp facts onRunSimulation needs — the 2D-enabled setting, whether a mesh resolves, and the [2D_OPTIONS] OUTPUT_FILE — exactly once instead of three times. Each was a full re-read of a file that has not changed since the auto-save immediately above. Adds test_dirtytracking_run.cpp: one representative edit from each family that previously had no path to the flag, plus the negative cases (selection changes and a registry flush with no edit must stay silent, and a freshly loaded project must be clean — a false positive costs a full .inp rewrite before every run). Its CMake registration was already committed without the source.
…ing it A virtual junction is a computational break point inside one conduit, so the profile already suppressed its shaft and glyph. The pipe itself still stopped short of it: every conduit end is trimmed 3.5 px and capped with a vertical line so it butts against the manhole tube, and a split reach inherited that at each break. A conduit divided into five virtual junctions read as five separate pipes with gaps between them. The trim and the cap are now per-end, and both are dropped at a virtual junction. Consecutive conduits meet at the same chainage carrying their own slopes, so the invert and crown read as one continuous polyline that kinks where the slope changes rather than parting. A thin dashed vertical line from the invert up to the ground line marks the break that is no longer drawn. The same 3.5 px inset governed the HGL line, its fill and the Max HGL band, and three per-node passes bridged the gap it left by filling the manhole shaft from the invert up to the rim. At a virtual junction that column climbed to a rim well above the pipe crown, pushing water up into the soil, and the nodal HGL stub flattened 7 px of a sloping water surface. hglEdgePixels now owns the inset for all four link-side call sites and takes none at a virtual junction, and the three bridging passes skip it, so the water surface crosses the break exactly as continuously as the pipe does. tests/manual/virtual_junction_rim gains the by-eye checks for all three.
…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.
Adds a dockable Section View, a live preview in the cross-section editor, and a per-type layer diagram in the LID editor, all rendered from the engine's swmm_xsect_* geometry API rather than static artwork. Surfaces the five cross-section shapes the picker never offered and replaces the 26 hand-drawn *_xsect.svg thumbnails with procedural, theme-aware icons. Fixes a pre-existing bug in LinkCompoundEditDialog: shape comparisons used literals from before the 6.0 SWMM_XSectShape renumbering, so IRREGULAR (21) was tested as 19 (VERT_ELLIPSE) and the transect picker never appeared. The slice was written without a Qt toolchain and had never been compiled or drawn. It builds as-shipped; what a screen had to say about it: - Leader labels were placed in the raw canvas rect at an unclamped y, so the crown label landed on top of the subtitle and the invert label on the footer — in every link section and every node profile. They are now confined to the band left after the header, footer and plan inset, and de-conflicted per side so a manhole whose links share an invert stops writing one label over another. - Side margins are measured from the labels they have to hold. A fixed 46 px elided "C1 Inv 93.00" to "C1…", and reserving only the text width still clipped by exactly the length of the leader's own elbow. - Dimension labels got the same treatment vertically: at 220 px the width dimension was drawn through the subtitle. - The profile's crown leader sat on the centred "L … S …" run dimension; it now anchors at a quarter point. - Plan-inset spokes sharing a bearing — the straight-through manhole, i.e. the common case — drew their labels on top of each other. - LID layer labels repeated the thickness already dimensioned beside each block, which pushed the parameters unique to that layer out of the margin. test_xsectsampler asserted the sampled outline reproduces wMax to 1e-3. It does not, and cannot: for the shapes SWMM stores as normalized tables the width table peaks at 0.9992 of the wMax reported from the shape parameter. Measured with tests/scratch/sp_wmax_probe.c — bit-identical at 64, 256, 512 and 4096 rungs, while analytic CIRCULAR is exact — so the ladder is not the cause and refining it is not the fix. The bound is now relative, and says so. Not addressed here: the dock has no view.dock.* row in actioncatalog.h, so it has no keyboard shortcut and no entry in shortcut customization (the menu toggle works — docks supply their own toggleViewAction). Ctrl+Alt+8 is already taken by view.layerStylingDock.
Crown (soffit) elevation was annotated inconsistently across the three section views, and two of the three were misleading: - Cross-section reported only the upstream node's invert/crown, though a conduit has two ends. It now labels both -- "Crown El. 101.00 / 99.00 ft" -- collapsing to a single value when the run is flat. - Link profile carried one crown interpolated at the 0.28 point along the barrel, a value at no real station. Replaced with the true barrel-end crowns. These point OUTWARD, against this function's inward convention for rim/invert: the run dimension writes "L ... S ... %" along the crown line itself, so an inward label lands on top of that text -- which is what forced the quarter-point anchor in the first place. - Node profile had no crown at all. Each connecting pipe now gets one, gated on the section having a real height: the stub height used for a pump or DUMMY link is a drawing minimum, not an elevation, and labelling it would print a number the model does not contain. Side margins and model bounds are unchanged, so no drawing shrinks. Adds tests/gui/test_sectionmodelbuilders.cpp -- the builders had no test coverage at all before this.
…g them The profile options tree offers a plot-level pen for each of HGL, EGL, Max HGL and Max EGL plus two fill brushes. Editing any of them did nothing. Series styling is resolved before it reaches the widget: the dialog reads a pen per output kind from each results LAYER, so overlaid scenarios stay distinguishable. The six ProfilePlotOptions pens had only one reader — ProfilePlotWidget::themeEglPen() and its three siblings — and nothing ever called those, so the values went nowhere. The dead accessors are removed; they are what made the property look wired. Editing a plot-level style now pushes it onto every loaded source, then the series rebuild picks it up. `ProfilePlotOptions::changed` is a single signal for every property, so pushing blindly would restyle all sources whenever any unrelated option changed and quietly undo per-source customisation; only the styles that actually differ from a snapshot are pushed. The snapshot is seeded when the options object is bound, so opening the dialog restyles nothing and the first real edit is still recognised as an edit. Per-source styling in the same dialog's right-hand pane is unchanged and still wins for a single scenario — until the next plot-level edit.
…are integer Axis number format was two controls: a Decimals/SignificantFigures enum and a free integer precision. The integer carried no meaning on its own — "3" never said whether it meant decimal places or significant figures — and the pair could express nonsense like 0 significant figures. Both collapse into a single dropdown of the combinations that mean something: Integer, Decimals1-4, Decimals6, SigFigs3, SigFigs4, SigFigs6. numberformat.h owns the list and the mapping to a real mode + digit count; each Q_OBJECT that publishes it mirrors the values as its own Q_ENUM, because QPropertyModel resolves a property's enumerator list through the declaring class's meta-object and labels each row with the enumerator's own name. Applied to every chart surface: ProfilePlotOptions, ChartProperties (x, y and the statistics readout), MeshProfilePlotOptions, TransectChartView, and the Preferences default — where the entries can be spelled out properly, e.g. "12.35 (2 decimals)". Mode + count remain the internal representation, so every label formatter and the stored QSettings keys are untouched: the preset is a view over them, and there is no settings migration. A stored count with no exact preset snaps to the nearest one in the same mode, ties going to the preset with MORE digits — showing fewer digits than the user asked for is the worse error. Also fixes a test the timeseries date-time work left failing: the grid's time column now returns formatted text for DisplayRole and a QDateTime for EditRole, and test_timeseries_table_model still asked DisplayRole for a QDateTime. It went unnoticed because that test carries the `unit` label and the earlier run filtered on `gui`.
The Section View dock's toggle action had no action-catalog entry, so it carried no icon, no shortcut and no tab assignment — it was reachable only from View → Panels. Register it as view.dock.sectionView (Ctrl+Alt+9; 1–7 are the other docks, 8 is Layer Styling) with a new theme-aware dock_section_view icon: a pipe cross-section with water surface and a crown-to-invert dimension arrow, in the existing dock-icon family. Add the button to the View tab's Panels group (between Properties and Attribute Table) and to createPopupMenu() so the ribbon, the View → Panels menu and the dock right-click menu all list the same panels.
…he list Opening the editor with nothing bound -- createNew(), or pickTimeseries() with an empty or unknown name -- disables the whole toolbar, and only the Create-submit path (bindNewProvider_) switched it back on. Picking an existing series out of the list goes through rebindActiveProvider_ instead, which bound the provider, filled the grid and drew the chart but left every mutation greyed out. An inline time series looked fully loaded and could not be edited at all. refreshSourceModeCardForProvider_ already re-enables each action on rebind, but that cannot help: a disabled QToolBar disables its buttons regardless of each QAction's own enabled state. The toolbar carries every mutation (Edit / Rotate / Scale / Add Row / Delete Rows / Paste / Undo / Redo), so its enabled state belongs to "is a series bound?" -- rebindActiveProvider_ now owns it. Reached from ComprehensiveEditorRegistry (always createNew) and from any pickTimeseries call arriving with no current name. Opening from the Object Browser was unaffected, since that constructs with an initial provider, which is why the behaviour looked inconsistent. The new test asserts on the QToolBar and not only on the QActions -- that distinction is the bug -- and was confirmed to fail without the fix.
…welcome Two QMdiArea behaviours, both landing on the welcome screen. QMdiArea snapshots palette().brush(QPalette::Dark) once in its constructor (qmdiarea.cpp:1681) and has no PaletteChange handling, while its paintEvent fills the viewport with that snapshot. SWMMVisApplication builds SWMMVis at :104 and only installs Fusion + the token theme at :107-117, so the brush was frozen at the pre-theme palette and no later Appearance switch reached it. welcomeWidget is a plain QWidget with no autoFillBackground, painting no background of its own, so that stale brush is what filled the entire welcome tab -- and the empty workspace once every tab was closed. Separately, TabbedView never hides an inactive sub-window; it relies on the active one being maximized to cover the viewport. That state is handed over in _q_deactivateAllWindows (qmdiarea.cpp:682-683) only when an outgoing window is BOTH maximized and visible. Hiding the welcome sub-window in place -- the startup branch when "show on startup" is off, and the tab's X via eventFilter -- breaks the chain, so the next activation lands in Normal state: a small framed child window, complete with the default Qt window icon since no windowIcon is set, painted over the tab the user selected. installMdiWorkspaceChrome() writes the surfaceWindow token onto the backdrop and re-runs on themeChanged, and re-asserts showMaximized() on every activation. The backdrop reads the token rather than area->palette(): QApplication::setPalette updates only qApp synchronously and *posts* ApplicationPaletteChange to widgets (qapplication.cpp:1750-1759), so inside that handler the widget palette still holds the outgoing scheme. The maximize guard skips explicitly hidden sub-windows -- showMaximized() also shows a window, and a welcome tab the user dismissed must stay dismissed. Extracted rather than inlined into SWMMVis so the test can drive the real code against a bare QMdiArea. Ground truth for the Qt behaviour is the pre-existing tests/scratch/mdi_tab_probe.cpp; the new ctest mirrors uic's exact construction order (TabbedView set before welcomeWidget reaches addSubWindow while the area is still hidden, which is what routes it through pendingPlacements) and the app's async open sequence. Of its seven cases hiddenWelcomeDoesNotStrandTheNextTab is the one that fails without the fix; the switching cases guard the ordinary paths against the fix. Not addressed, pre-existing and same class of symptom: "Bring All to Front" raises every project sub-window, and Qt leaves the non-active ones Normal.
cbuahin
added a commit
that referenced
this pull request
Oct 7, 2026
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
3 participants
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.
Initial SWMM 6.0.0 gui