Update CreateMechanicalSystem.cs#176
Open
michaldengusiak wants to merge 77 commits into
Open
Conversation
During the SAM-BIM Newtonsoft.Json -> System.Text.Json migration the quarterly sow/2026-Q2 branch carries the binary-breaking change. CI needs to consume the migrated SAM (and any sibling dep) from sow/2026-Q2, not from master, until the quarter-end merge. - Add "sow/2026-Q2" to push/pull_request branch triggers - For each dep clone, ls-remote sow/2026-Q2 and prefer it when present; fall back to default branch (e.g. master) otherwise. After the quarter merges back to master, this fallback naturally restores prior behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Refines the previous workflow patch so CI on the migration PR can succeed before anything has been merged. Each dep repo is cloned from: 1. github.head_ref (feature/remove-newtonsoft on these PRs) - has the migration right now on every dep 2. sow/2026-Q2 - source of truth after these PRs merge 3. default branch - source of truth after quarter-end merge Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors the fix already applied on SAM_Revit / SAM_Revit_UI: this workflow also clones the private SAM_Solver dep, so the cascade ls-remote and clone must use the token-prefixed URL when $token is defined, otherwise GitHub prompts for credentials and CI fails with 'Cannot prompt because user interactivity has been disabled'. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The harmonized spdx-check workflow requires both header lines in the top 80 lines of every changed .cs file. SAM_UI was missed in the earlier fix-spdx pass; adding the header to the 45 files in the migration diff (excluding Properties/ and *.xaml.cs which the workflow skips). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The earlier fix_spdx pass detected only the en-dash form of the copyright line. Files that already had a hyphen-minus header got a second header pair prepended above the original instead of being recognised as already-compliant. Strip the duplicate, keeping the first (en-dash) pair. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Remove Newtonsoft.Json — migrate to System.Text.Json.Nodes
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93f9da9e60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+96
to
+99
| $has = (git ls-remote --heads $url $cand 2>$null | Out-String).Trim() | ||
| if ($has) { | ||
| Write-Host "Cloning $org/$r @ $cand" | ||
| git clone --depth 1 --branch $cand $url $r |
There was a problem hiding this comment.
In .github/workflows/build.yml (clone step), git ls-remote --heads $url $cand is used as a truthy check before git clone --branch $cand. ls-remote treats the last argument as a pattern ([<patterns>...] in git ls-remote -h), so a branch name like release can match refs such as foo/release; this makes $has non-empty even when refs/heads/release does not exist, and the subsequent clone fails for that dependency. Use an exact ref check (for example refs/heads/$cand with --exit-code) before attempting --branch $cand.
Useful? React with 👍 / 👎.
Aligns the repo with the SAM-BIM workspace-wide pin to the LTS-aligned System.Text.Json 8.0.5, replacing the previously pinned 10.0.8 preview.
Pin System.Text.Json to 8.0.5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf9a08da2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+88
to
+91
| $headRef = '${{ github.head_ref }}' | ||
| $candidates = @() | ||
| if ($headRef) { $candidates += $headRef } | ||
| $candidates += 'sow/2026-Q2' |
There was a problem hiding this comment.
In .github/workflows/build.yml, the dependency-branch candidate list is built from github.head_ref, which is empty on push events, so pushes to main/master will always fall back to cloning sow/2026-Q2 when that branch exists in dependency repos. That means CI for main can compile and test against a different dependency branch than the code being merged, masking real integration failures (or introducing false ones) for release branches.
Useful? React with 👍 / 👎.
* fix: pin AssemblyVersion/FileVersion to 1.0.0.0 (drop 1.0.* wildcard) Eliminates CS1607 warnings and restores deterministic builds. Aligns with the workspace's existing fixed-version files. Scope: AssemblyInfo.cs + .csproj version attributes only. The // commented-out template example is left intact for documentation. * fix: add SPDX + copyright header to modified AssemblyInfo.cs and refresh .csproj <Copyright> Satisfies the spdx-check workflow which requires every changed .cs file to declare the LGPL-3.0-or-later SPDX identifier and the 2020-2026 attribution in its first 20 lines. Also updates the stale '<Copyright>Copyright (c) 2020</Copyright>' line in .csproj files to match the SPDX header style.
* build: Directory.Build.props for centralised SAMVersion stamping Mirrors SAM-BIM/SAM#7. Stage 2 of the AssemblyVersion versioning migration. * fix: relax SAMVersion.g.cs Target condition to != 'true' Codex P1 on SAM_LadybugTools#4: classic (non-SDK) csprojs don't set GenerateAssemblyInfo at all, so the previous '== false' condition skipped them. Switching to '!= true' catches both GenerateAssemblyInfo=false (SDK projects with legacy AssemblyInfo.cs) AND empty (classic projects). * ci: Codex P2 fixes — four-part SAMVersion, prefer head_ref for sow PRs - Append .0 so SAMVersion is 4-part (AssemblyFileVersion expects 4 parts). - Prefer github.head_ref when it matches sow/yyyy-Qx (release-promotion PRs). - Mirrors SAM sow/2026-Q2 6d87d98e.
Mirrors SAM sow/2026-Q2. Adds AssemblyInformationalVersion attribute generation to Path D's SAMVersion.g.cs Target — covers Grasshopper / Tas-bridge assemblies that have GenerateAssemblyInfo=false, so they also get the CI commit SHA stamp in their ProductVersion field. SDK projects pick up InformationalVersion via PropertyGroup auto-gen as before. Local dev builds unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e305f98d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+77
to
80
| JsonArray jArray = jObject["ZoneCategories"] as JsonArray; | ||
| if(jArray != null) | ||
| { | ||
| ZoneCategories = new List<string>(); |
There was a problem hiding this comment.
After switching ZoneCategories parsing to System.Text.Json.Nodes.JsonArray, the loop still declares foreach (string category in jArray). JsonArray enumerates JsonNode entries, and JsonNode→string is not an implicit conversion, so this path will fail to compile/build for this project and block CI. Iterate JsonNode items and extract strings explicitly (for example via GetValue<string>()) before adding them to ZoneCategories.
Useful? React with 👍 / 👎.
Neither iText 7 nor iTextSharp APIs are used anywhere in SAM_UI source (only one commented-out using statement). PDF export in the Mollier UI uses Excel COM via NetOffice, not iText. Removing the PackageReferences also eliminates the transitive Newtonsoft.Json 13.0.1 (pulled by itext.commons), which had been leaking into the installer payload through the standalone EXE PostBuild that xcopies bin/* to %APPDATA%\SAM\. Installer slim-down: Newtonsoft.Json.dll, itext.* runtime DLLs, iTextSharp.dll, BouncyCastle.Cryptography.dll, and the legacy Microsoft.Extensions.DependencyModel 1.1.0 chain. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fce0a32c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+124
to
+126
| $sowRef = if ($baseRef -match '^sow/') { $baseRef } elseif ($refName -match '^sow/') { $refName } else { '' } | ||
| if ($sowRef -and $sowRef -ne $headRef) { $candidates += $sowRef } | ||
| $candidates += 'sow/2026-Q2' |
There was a problem hiding this comment.
For pull requests into main/master from a non-sow/* branch, $headRef is set (feature branch), $sowRef is empty, and the next candidate is the hardcoded sow/2026-Q2; if that branch exists in dependency repos, CI clones release dependencies instead of the target branch lineage. That means PR validation for main can run against mismatched dependencies and miss real integration breakages (or report false failures).
Useful? React with 👍 / 👎.
Mirrors SAM sow/2026-Q2. Adds AssemblyInformationalVersion attribute generation to Path D's SAMVersion.g.cs Target — covers Grasshopper / Tas-bridge assemblies that have GenerateAssemblyInfo=false, so they also get the CI commit SHA stamp in their ProductVersion field. SDK projects pick up InformationalVersion via PropertyGroup auto-gen as before. Local dev builds unchanged.
* feat(stage-1): retire WinForms SimulateForm/Control; migrate SAM.Core.UI to WPF dialogs - Delete WelcomeForm (unused), SimulateForm + SimulateControl (WinForms) - Move HooverTimer from SAM.Core.UI to SAM.Core.Mollier.UI (only used there) - Drop RibbonWinForms NuGet from SAM.Analytical.UI.csproj - Remove UseWindowsForms from SAM.Core.UI.csproj; add System.Drawing.Common 7.0.0 - Migrate UIJSAMObject dialogs to WPF MessageBox + Microsoft.Win32.SaveFileDialog - Add UpdateConstructionLayersByPanelType checkbox to WPF SimulateControl - Wire WPF Modify/Simulate.cs second overload to WPF SimulateWindow + WindowInteropHelper Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Codex review on stage-1 PR - Add SPDX header to MollierControl.cs (fixes SPDX CI failure) - Primary Simulate(UIAnalyticalModel) overload: read UpdateConstructionLayersByPanelType from SimulateWindow instead of hardcoding true - Second Simulate(AnalyticalModel, path) overload: add projectName and outputDirectory validation guards before starting simulation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
- Path-based Simulate overload now sets Sizing=true and UpdateConstructionLayersByPanelType=true on the new WPF SimulateWindow, matching the WinForms SimulateForm defaults that were lost during the WinForms → WPF swap. Prevents silent regression where clicking OK without touching the Sizing checkbox would skip sizing. - Persist UpdateConstructionLayersByPanelType through SimulateOptions: added property (default true), copy constructor entry, JSON read/write, and the SimulateControl.GetSimulateOptions/SetSimulateOptions round-trip. Users' checkbox state now survives ActiveSetting save/restore. Codex findings on PR #7 (already merged): P1 Sizing default regression and P2 UpdateConstructionLayersByPanelType persistence. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…l-sites SAM_Tas extended ToSAM(path, importUnused) to ToSAM(path, importUnused, importSurfaceShades) as part of the _importSurfaceShades_ feature (SAM-BIM/SAM_Tas#9). The two existing call-sites in SAM_UI (Import.cs and Simulate.cs) must pass the new third argument explicitly to preserve their current behaviour — no shade data extraction on regular TBD imports from the UI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI's spdx job flagged this file (modified for the ToSAM call-site update) as missing the required SPDX/copyright header. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fix: update ToSAM call-sites for new importSurfaceShades parameter (SAM_Tas#9 compat)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
ZiolkowskiJakub
approved these changes
May 27, 2026
* SAM.Core.Mollier.UI.WPF: port Mollier UI to WPF/OxyPlot (Stage 2a-2e batch 4)
New net8.0-windows project SAM.Core.Mollier.UI.WPF (UseWPF, no UseWindowsForms) replacing the WinForms SAM.Core.Mollier.UI, plus a throwaway MollierSpike validation app.
- 2a: OxyPlot.Wpf spike (hit-test, axis transforms, PNG/SVG/EMF-via-Metafile-bridge, drag-select) - all passed.
- 2c: chart-builders ported Chart->PlotModel (Convert/ToChart, Modify/AddMollier*, converters); both chart types render.
- 2d: MollierControl over OxyPlot PlotView (axis setup, hit-test, pixel<->value transforms, drag-zoom, hover, PNG/SVG/EMF export) + full label-collision solver layer.
- 2e: NumberBoxControl + 3 leaf controls + 7 process controls + 4 composites + 6 dialogs (incl. live Edit via Apply); all dialogs use a modeless-safe DialogOk flag instead of WPF DialogResult.
Builds 0 errors/0 warnings; headless smoke tests render both chart types (+labels) and construct all controls/forms at designer sizes. Deferred to batch 5: UIMollierObjectsForm, PointListOptionForm, MollierControlSettingsForm, full MollierForm (skeleton present), OpenJSONForm; plus PVP axis/PDF/Print and 2f consumer switch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* SAM.Core.Mollier.UI.WPF: complete Stage 2e batch 5 (final forms)
Ports the last 7 WinForms types to WPF/OxyPlot, completing Stage 2e:
- OpenJSONForm (merge/replace chooser)
- ListPointsOptionControl + PointListOptionForm (point picker)
- PointManageControl + UIMollierObjectsForm (DataGridView -> DataGrid;
explicit columns, template Visible/Edit/Remove cells, ContextMenu)
- MollierControlSettingsForm (View/Ranges/Tools; Button.BackColor ->
Tag + Background brush helper; ToolTips in XAML; integer-only input)
- full MollierForm, replacing the skeleton (MenuStrip -> Menu, Shown ->
ContentRendered, WinForms file dialogs -> Microsoft.Win32; an
`initialized` flag guards toolbar TextChanged handlers from firing
during InitializeComponent)
All 30 ported controls/forms construct at runtime (MollierSpike
--controls: RESULT PASS). Deferred as before: PDF export (Save(PDF)
no-op), Print (stub), partial-vapour-pressure secondary axis.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Stage 2f (staged): point isolated consumers at the WPF Mollier UI
Switches the two consumers that reference only SAM.Core.Mollier.UI (no
transitive WinForms-Mollier, so no CS0433 ambiguity) onto the new WPF
assembly SAM.Core.Mollier.UI.WPF:
- Standalone "SAM Analytical Mollier" exe: ProjectReference -> WPF, add
UseWPF, and host the now-WPF MollierForm via a WPF Application instead
of WinForms Application.Run(Form).
- Grasshopper MollierDiagramComponent: ProjectReference -> WPF; map the
WinForms Form events to WPF (FormClosing -> Closed, Shown ->
ContentRendered).
Also removes the throwaway MollierSpike spike/smoke-test project.
The old WinForms SAM.Core.Mollier.UI is intentionally kept in place: its
remaining consumers (SAM.Analytical.UI + SAM.Analytical.UI.WPF, coupled
via the headless Excel-print path in PrintAirHandlingUnitsByTemplate) are
deferred to a verified follow-up. Validated by CI (local full-workspace
build is blocked on unrelated missing dependency build outputs).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Stage 2f (complete): migrate remaining consumers to WPF Mollier UI; retire WinForms project
Swaps the rest of the SAM.Core.Mollier.UI consumers onto SAM.Core.Mollier.UI.WPF
and removes the old WinForms project (no references remain anywhere in the repo).
Consumers (ProjectReference -> WPF + call-site fixes):
- SAM.Analytical.UI: AHU/Space diagram dialogs (drop using(); FormWindowState ->
System.Windows.WindowState; Form.Name -> Window.Title; ShowDialog(owner) ->
ShowDialog()). Headless Excel-print path in PrintAirHandlingUnitsByTemplate:
MollierControl{Visible=false}/.Size/.Refresh()/Save(EMF) -> Regenerate() +
SaveImage(path.emf, w, h) (size-correct vector EMF; .emf temp paths so the
extension-keyed exporter picks EMF).
- SAM.Analytical.UI.WPF: AnalyticalWindow "Open Mollier Chart" (drop using()).
- SAM.Analytical.UI.Grasshopper: 3 ShowDiagram components (Form.IsDisposed reuse
guard -> Closed handler nulls the field; FormWindowState -> WindowState;
Name -> Title).
- SAM Analytical + SAM.Analytical.UI.WPF.Grasshopper: reference-only swaps.
Removed Mollier\SAM.Core.Mollier.UI from SAM_UI.sln and deleted the project.
SPDX headers added to the changed .cs files. Validated by CI (the WPF assembly
builds standalone; full solution builds in VS).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): restore chart interactions lost in WPF port (PR #10 review)
Addresses the regressions found while testing the WinForms->WPF Mollier port:
- Chart mouse events (zoom-rectangle select, click-to-select for the e/SHR
tools, and the "..." point picker in the process dialogs) all depend on the
control raising MollierPointSelected from MouseUp. OxyPlot PlotView's default
PlotController grabbed the left/right buttons and marked the routed events
handled, so none of those handlers ran. Install a controller that unbinds the
left/right buttons (right is left for the WPF context menu, wheel-zoom kept).
- Help -> Wiki crashed: Process.Start(url) throws on .NET 8 (UseShellExecute
now defaults to false). Use ProcessStartInfo { UseShellExecute = true }.
- Add Process window clipped the widest (Cooling) control: widen to 700 and
allow manual resize (ResizeMode=CanResize + MinWidth/MinHeight).
- Ctrl shortcuts were swallowed once focus left the window (WinForms used
KeyPreview): switch Window KeyDown -> PreviewKeyDown and mark handled; add
InputGestureText hints and Alt-mnemonics to the menus.
- Partial Vapour Pressure axis was never ported (TODO): add the secondary
pW [kPa] axis (top for Mollier, left for Psychrometric), gated on the View
toggle, mirroring the WinForms CreateXAxis/CreateYAxis.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): hover value-box, point-pick snapping, image-save rename
Follow-up to the PR #10 review feedback:
- Hover tooltip (the WinForms "yellow box") was lost because the previous
commit unbound the left mouse button, which is OxyPlot's only default
trigger for the tracker (SnapTrack). Drive the tracker from the Hover
hit-test instead: when the Hover toggle is on and the cursor is over a
process / point, show OxyPlot's tracker using the series' GUID-free
TrackerFormatString (built on the fly from the point tag for the
multi-point scatter). Background grid lines are excluded so the GUID/auto
titles never appear in the box.
- Click-to-pick now snaps: GetNearestMollierPoint returns the existing chart
point within ~12 px of the click, so the process / epsilon / SHR pickers
return that point's exact values; otherwise the click location is used
(previous behaviour, matching the old WinForms GetMollierPoint).
- Rename "Save as JPG" -> "Save as Image…" since the save dialog already
offers PNG, JPG and SVG.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): hover highlights grid lines again; prioritise points
The previous hover change limited BOTH the highlight and the value-box to
processes/points, so hovering the grid lines stopped highlighting them
(a regression from earlier today), and points that sit on top of a grid line
never won the nearest-series test so they neither highlighted nor showed data.
- Highlight (BumpThickness) now applies to ANY hovered series again, restoring
the "select lines" feedback on an empty chart.
- The value-box is still limited to the meaningful objects (clean, GUID-free).
- NearestPointSeries gives existing points priority over the lines they overlap,
so hovering a point now highlights it and shows its values.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(mollier-wpf): click shows the value-box for the clicked location
With Hover on, a left-click now pops the same formatted value-box (t, x, phi,
h, rho, v, p, pS, pW, pL) for the Mollier point at the click location, or for
the snapped existing point if one is within proximity. Reuses Query.ToolTipText
via a manually positioned TrackerHitResult.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): suppress OxyPlot's default raw tracker box
The built-in PlotController also bound a hover tracker, which popped a second
value-box with the raw series title + full-precision X/Y (e.g. "MollierPoints /
Humidity Ratio x: 17.7539... / Dry Bulb Temperature t: 36.5057...") competing
with our formatted box. Clear all default controller bindings (UnbindAll) and
keep only wheel-zoom, so only our own formatted tracker is shown.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): formatted hover box for points (no raw default label)
Hovering an added point still showed OxyPlot's default tracker ("MollierPoints /
Humidity Ratio x / Dry Bulb Temperature t") with raw chart X/Y - which differs
between Mollier and Psychrometric and shows the diagram axis value, not the true
dry-bulb temperature. Cause: we mutated the GetNearestPoint result's .Text, but
because that result still referenced its Series the WPF tracker re-derived the
default text at render time and ignored the override.
Now show a fresh, series-less TrackerHitResult (same approach as the working
click box), so the multi-point scatter renders the full formatted ToolTipText
(t, x, phi, h, rho, v, p, pS, pW, pL) from the point's tag - identical in both
chart types.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): always build hover label from the tag, never OxyPlot default
The raw default box ("MollierPoints / Humidity Ratio x / Dry Bulb Temperature t")
kept appearing on points and on the cooling-process auxiliary lines, because
those series either have no TrackerFormatString or one we did not override, so
OxyPlot rendered its own series-derived text.
Compute the value-box text directly from the hovered series' domain tag
(UIMollierProcess / MollierProcess -> process tooltip; UIMollierPoint or the
nearest multi-scatter point -> point tooltip) and show it via a fresh,
series-less TrackerHitResult. Series with no meaningful tag (grid lines) return
no text and hide the tracker. Result: the formatted, chart-independent label is
the only box that ever shows.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(mollier-wpf): native PDF export (A4/A3, portrait/landscape)
Replaces the deferred NetOffice/Excel PDF pipeline (Save(PDF) was a no-op) with
OxyPlot's SkiaSharp PdfExporter - fully managed/.NET-native, no GDI, no Excel.
- Add OxyPlot.SkiaSharp 2.2.0 (chosen over the obsolete GDI OxyPlot.Pdf, which
only targets .NET Framework and triggered NU1701).
- MollierControl.Save gains a (PageSize, PageOrientation) overload and SavePdf,
rendering the plot to the page size in points (A4 595x842, A3 842x1191;
swapped for landscape).
- The File > Save as PDF > A3/A4 Portrait/Landscape menu items now export.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore(mollier-wpf): drop NetOffice/Excel and unused refs from the Mollier app
Now that PDF export is native (OxyPlot SkiaSharp), nothing in the Mollier UI
needs Excel. Reviewed and trimmed the SAM Analytical Mollier.exe dependency
closure:
- Remove the SAM.Core.Excel reference from SAM.Core.Mollier.UI.WPF: it was
unused (only a stale doc comment mentioned it) and was the sole thing pulling
NetOfficeFw.Excel (+ a VBIDE COM reference) into the Mollier app. Neither
SAM.Analytical.Mollier nor SAM.Core.Mollier reference Excel, so the app's
closure is now NetOffice-free.
- Remove the legacy net-framework references System.Data.DataSetExtensions and
Microsoft.CSharp from the EXE: unused (no DataSet/dynamic) and unresolvable on
net8.0-windows (they emitted MSB3245/MSB3243 warnings). EXE now builds clean.
- Refresh the MollierControl class summary (PVP axis, value-box and native PDF
are done; only WinForms Print remains a stub).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(mollier-wpf): native WPF printing (File > Print)
Print was a stub showing "Printing is not yet available". Implement it natively
with System.Windows.Controls.PrintDialog: render the chart with OxyPlot's WPF
PngExporter at a supersampled resolution and PrintVisual it scaled into the
selected printer's printable area. No Excel/NetOffice involved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): vector print (render via PlotView, not a bitmap)
Print used PngExporter -> a raster bitmap, so the printout pixelated when zoomed.
Render the model through an off-screen OxyPlot PlotView (WPF vector shapes/text)
and PrintVisual that, so the print is vector / high quality. The shared model is
detached by the temp view, so it is re-attached to the live chart afterwards.
Note: PDF export was already vector (OxyPlot SkiaSharp PdfExporter renders through
Skia's PDF canvas), so no NetOffice/Excel is needed for high-quality output.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): print crash - detach model before reuse in print PlotView
OxyPlot throws InvalidOperationException ("This PlotModel is already in use by
some other PlotView control") when a model attached to the live chart is
assigned to the off-screen print PlotView. Detach it from MollierChart first,
assign to the print view, and re-attach to the live chart in a finally block
(so the chart is restored even if printing throws).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mollier-wpf): blank print - render print PlotView in an off-screen window
The off-screen PlotView printed blank: OxyPlot only draws during a real WPF
render pass, so Measure/Arrange/UpdateLayout left its canvas empty. Host the
print PlotView in a tiny off-screen (-32000,-32000) borderless, non-activating
window so it genuinely renders, pump the dispatcher at Render priority to let
the pass finish, then PrintVisual it (vector). Window is closed and the model
re-attached to the live chart in finally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
) The GZip snapshot codec did ToJsonObject() -> ToJsonString() -> UTF8.GetBytes() -> gzip. On the 10k-space model the JSON string is ~50-100 MB (the .NET UTF-16 string ~2x that) plus a second large byte[] - two big allocations and a copy per snapshot. CreateSnapshot now writes the JsonObject tree straight into the GZipStream via a Utf8JsonWriter, and RestoreSnapshot parses straight from the decompressing stream (JsonNode.Parse(Stream)) - no intermediate JSON string and no separate decompressed byte[]. UI-only; the ToJsonObject() tree build is unchanged (a true streaming serializer eliminating the tree would need a per-IJSAMObject writer across all of SAM - deferred). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Concrete plan for merging the ~33k per-object SharpDX scene models into a few material batches to cut ToElement3D.Attach (~3.5 s), with a CPU-side triangle-range -> guid map for picking and a guid -> bounds index for zoom/rect-select. Captures the three hard parts (build, pick-to-guid, per-object recolour), the recolour decision (per-vertex vs overlay vs hybrid), and a flag-gated incremental rollout to verify in VS at each step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ent 1) Mesh-batching increment 1 (render-only, measure the attach win). Adds Convert.ToBatchedElement3Ds: appends every object's primitives into a single SharpDXSceneBuilder so the whole model emits a few merged-by- material models instead of one GroupModel3D per object (~33k on the 10k model). ToElement3D.Attach scales with model count, so this targets the ~3.5 s attach (batch-attach of per-object models gave nothing, PR #36 - only model-count reduction helps). SharpDXViewportControl.Load uses the batched builder when SAM_UI_VIEWPORT_BATCH is set (default off). The merged models carry no per-object IJSAMObject tag, so the existing attach loop skips per-object indexing and picking/hover/selection are inert in this mode - those are re-implemented on the batched representation (triangle-range -> guid map + overlay selection) in later increments. Use only to measure the attach/regen win for now. Design: documentation/3d-viewport-mesh-batching-plan.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Increment 2 of mesh-batching: object identity on the batched scene without one model per object. - SharpDXSceneBuilder(capturePickRanges): records, per merged mesh, the starting vertex index + guid of each contributing object (objects are appended contiguously), and Build() returns a PickMap of MeshGeometryModel3D -> PickBucket (binary-search vertex index -> guid). - Convert.ToBatchedScene returns Element3Ds + PickMap + guid->object map (ToBatchedElement3Ds now wraps it). Untagged objects still render (Guid.Empty range, not pickable) - parity with the per-object path. - SharpDXViewportControl: AttachBatched indexes the pick map, builds one event-payload stub per object, and queues the merged meshes for the deferred picking octree. HitTestGuid resolves a FindHits hit via TriangleIndices.Item1 -> PickBucket -> guid. Select/SelectedSAMObjects/ ContainsAny work off the guid->object map in batched mode. Hover/double-click/context-menu/selection-state + their events now work batched; the in-view selection/hover HIGHLIGHT is the overlay path (increment 3), so ApplyAppearance is a no-op while batched. Bounds-based features (Zoom-selected, rectangle select) are increment 4. Flag-gated (SAM_UI_VIEWPORT_BATCH), default off. Builds clean (0 errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…- zoom) Reported in batched mode: Zoom-Selected did nothing and Zoom-Extents on a single item zoomed out wrong. Cause: TryGetBounds read the (empty in batched mode) per-object dictionary_Element3D, so Zoom-Selected got no bounds and ZoomExtents fell back to the built-in Viewport3DX.ZoomExtents, which mis-frames a single small object (the reason FrameCamera exists). Capture per-object world bounds during the batched build (SharpDXSceneBuilder.AccumulateBounds over the appended mesh/line positions) and carry them through BatchedScene.Bounds into dictionary_BatchedBounds. TryGetBounds and ZoomExtents now use that map when sceneBatched, so Zoom-Selected, Zoom-Extents and the selection rotation pivot frame correctly again via the custom FrameCamera path. Rectangle (screen-rect) selection still iterates per-object geometry and is not yet wired for batched mode - separate follow-up. Builds clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
perf: stream undo snapshot write/read instead of materializing JSON (#16)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…rement 3) Batched mode had no in-view highlight (one merged mesh per material can't be recoloured per object). Add an overlay: the builder records each object's triangle slice (vertex + index sub-range) per merged mesh (GeometrySlice / OverlayMap), so the selected/hovered objects' triangles are sliced straight out of the merged buffers into a small overlay mesh drawn on top - no second copy of the geometry. - Selection overlay: opaque blue (material_Selection, parity with the per-object SelectAction). Hover overlay: translucent blue tint. - Negative DepthBias pulls the overlay in front of the coincident base so it shows without z-fighting; overlays are IsHitTestVisible=false so they never affect picking. - Rebuilt when the selection / hover set changes (Select, ClearSelection, click, hover move, mouse-leave); ApplyAppearance stays a no-op batched. Builds clean (0 errors). DepthBias magnitude (-1000) and the hover tint are first cuts to confirm visually. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The left-drag rectangle (screen-rect) selection still iterated the empty per-object index in batched mode, so drag-box selection did nothing. Add a batched branch to SelectByScreenRect that tests each object's bounding box (dictionary_BatchedBounds) against the rectangle via its projected corners (BoundsHitsScreenRect): Inside = all 8 projected corners inside; InsideOrIntersect = any corner inside or the corners' screen-AABB overlaps the rectangle. An approximation of the per-triangle HitsScreenRect used on the per-object path, but fast and adequate for drag-select. Completes batched functional parity (pick, click-select, overlay highlight, zoom, rect-select). Builds clean (0 errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Flip SAM_UI_VIEWPORT_BATCH on by default (set =0 to fall back to the per-object path). Batched mode is validated in VS on the 10k-space model: 3D regen ~10 s -> ~1 s with picking, click + rectangle selection, selection/hover overlay highlight, and zoom all working. Known accepted limitation: the opaque selection fill hides the selected object's own edge lines (overlay is mesh triangles only) - documented as a fast follow-up (line-segment overlay). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
) The opaque selection fill overlay hid the selected object's own edge lines. Add a line overlay alongside the fill: the builder now also records per-object line ranges (LineSlice / OverlayLineMap, mirroring the mesh ranges), so the selected objects' edge segments are sliced out of the merged line buffers into a LineGeometryModel3D drawn on top in the selection edge colour (blue, thickness 2.5, negative DepthBias, not hit-test visible). Stays on the overlay path: base scene untouched, built only for the (small) selection set on selection change - no effect on the ~1 s regen. Builds clean (0 errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
perf: show selected object edges in batched mode (selection-edge overlay) (#16)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The SharpDX viewport port (#30) and mesh-batching (#16) flattened nested geometry collections without switching the owning guid, so an aperture's triangles were recorded under its host panel's guid - clicking a window selected the panel and the EditAperture double-click handler was unreachable. Apertures are hosted inside their panel's Geometry3DObjectCollection (SAM.Analytical GeometryObjectModel). The old Helix ToMedia3D path gave each nested tagged collection its own tagged ModelVisual3D; restore that: - AppendPickable (batched path): record each pickable unit, including a nested tagged collection (the aperture), under ITS own guid and register it in the object map so the double-click payload resolves to the Aperture. - AddPickableGroups (per-object path): emit a separate tagged GroupModel3D per pickable unit instead of folding apertures into the panel group. - AppendDirect: ToElement3Ds(ISAMGeometryObject) (also used by RefreshAppearance) now builds only an object's own primitives, so a panel appearance edit no longer re-folds/duplicates its apertures. No change to SharpDXSceneBuilder, SharpDXViewportControl or the 2D path. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
* Migrate ComboBoxForm<T> to WPF ComboBoxWindow<T> (retire SAM_Windows, wave 1)
First wave of retiring the SAM_Windows (WinForms) dependency from SAM_UI.
- Add SAM.Core.UI.WPF.ComboBoxWindow<T>: a code-only (no XAML, since WPF
has no generic x:Class) WPF replacement for SAM.Core.Windows.Forms.ComboBoxForm<T>,
mirroring its constructors, SelectedItem and Description API.
- Add SAM.Core.UI.WPF.Query.ShowDialog(this Window, IWin32Window): bridges the
legacy WinForms call sites (IWin32Window owner + DialogResult) to the WPF
dialog model, parenting via WindowInteropHelper.
- Rewire the 4 ComboBoxForm consumers (SpaceDiagram, AirHandlingUnitDiagram,
CreateMechanicalSystem, ReplaceNameSpecialCharacters) to ComboBoxWindow<T>.
Public method signatures (incl. IWin32Window owner) are unchanged.
- Reference SAM.Core.UI.WPF from SAM.Analytical.UI.
The SAM.Core.Windows DLL reference is still present (other forms in that DLL
are not yet migrated); it will be dropped once its whole type surface is ported.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix IWin32Window ambiguity; migrate AddressAndLocationForm to WPF (wave 2)
- ShowDialog.cs: fully-qualify System.Windows.Forms.IWin32Window (was
ambiguous with System.Windows.Interop.IWin32Window). Verified building
SAM.Core.UI.WPF and SAM.Analytical.UI locally (clean).
- Add SAM.Core.UI.WPF.AddressAndLocationWindow (proper XAML) replacing
SAM.Core.Windows.Forms.AddressAndLocationForm; mirrors Address/Location API.
- Rewire EditAddressAndLocation to the WPF window; add SPDX header.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Clear SAM.Core.UI.WPF of SAM.Core.Windows; drop its DLL reference (wave 3)
SAM.Core.UI.WPF no longer depends on SAM_Windows. Verified: full VS
solution rebuild 16 succeeded / 0 failed.
- TextMapControl / FiltersControl / FilterWindow: TextBoxForm<string> ->
existing WPF TextBoxWindow.
- TextMapControl / LegendControl: ComboBoxForm<T> -> ComboBoxWindow<T>.
- FilterControl: SearchForm<IUIFilter> -> existing WPF SearchWindow
(GetSelectedItems<T>(), System.Windows.Controls.SelectionMode.Single).
- NumberFilterControl: inline the number-only regex (was
Windows.EventHandler.ControlText_NumberOnly WPF overload).
- GeometryModel3D_Text: port Windows.Query.Width -> SAM.Core.UI.WPF Query.Width
(new Query/Width.cs, GDI TextRenderer measurement).
- Remove the SAM.Core.Windows <Reference>/HintPath from SAM.Core.UI.WPF.csproj.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF custom-parameter grid + MaterialWindow (wave 4-5a)
Pure-WPF replacement for the WinForms PropertyGrid object editors, no new
dependency. Verified: full VS solution rebuild 16 succeeded / 0 failed.
- SAM.Core.UI: port the WinForms-free parameter model (CustomParameter,
CustomParameters) + Create.CustomParameters + Modify.SetValue/SetValues
from SAM.Core.Windows; drop the PropertyGrid-only CustomParameterDescriptor.
- SAM.Core.UI.WPF: new ParametersControl (category-grouped DataGrid, type-aware
value editing) replacing the WinForms PropertyGrid; new MaterialWindow
(pure WPF) replacing SAM.Core.Windows.Forms.MaterialForm + MaterialControl.
- SAM.Analytical.UI: EditMaterial rewired to MaterialWindow (signature
unchanged).
NOTE: MaterialWindow.Material preserves a pre-existing bug verbatim (Density
was read from the SpecificHeatCapacity field in the original MaterialControl).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* MaterialLibraryWindow (WPF) replacing MaterialLibraryForm (wave 5b)
Verified: full VS solution rebuild 16 succeeded / 0 failed.
- SAM.Core.UI: port MaterialLibrary import/export EventArgs (WinForms-free).
- SAM.Core.UI.WPF: new MaterialLibraryWindow (searchable DataGrid list manager:
add/remove/duplicate/import/export, edit on double-click) replacing
SAM.Core.Windows.Forms.MaterialLibraryForm; events exposed as EventHandler<T>.
New Modify.Duplicate (material) using MaterialWindow + WPF owner.
- SAM.Analytical.UI EditLibrary + SAM.Analytical.UI.WPF EditMaterialLibrary
rewired to MaterialLibraryWindow; event-arg types -> ported ones.
EditMaterialLibrary's import/export handlers still use MarqueeProgressForm and
Analytical.Windows.Query.Import (later waves), so DLL refs remain for now.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF MaterialLayersControl + SelectMaterialWindow foundation (wave 6a)
Bottom-up foundation for migrating ConstructionLibraryForm/ApertureConstructionLibraryForm to WPF:
- SelectMaterialWindow + Query.Material (SAM.Core.UI.WPF) replace SelectMaterialForm/Query.Material
- MaterialLayersControl + Query.ConstructionLayers (SAM.Analytical.UI) replace the SAM.Architectural.Windows control
Not yet consumed; editors that use them follow in subsequent waves.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF ConstructionWindow + JsonWindow/JsonForm bridge (wave 6b)
- JsonWindow + Query.JsonForm (SAM.Core.UI.WPF) replace the WinForms JsonForm<T> F12 inspector
- ConstructionWindow (SAM.Analytical.UI) replaces ConstructionForm; embeds the WPF MaterialLayersControl
Not yet consumed; ConstructionLibraryWindow and consumer rewiring follow in wave 6c.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF ConstructionLibraryWindow + Query.Import; flip Construction path off WinForms (wave 6c)
- ConstructionLibraryWindow (SAM.Analytical.UI) replaces ConstructionLibraryForm; same public surface
- WPF Query.Import (ImportConstructionManager) + ported ImportOptions and ConstructionManager import/export EventArgs
- Rewired EditLibrary (.UI) and EditConstructions (.UI.WPF, incl. import/export handlers, TCD path) to the WPF window
- Import builds a ConstructionManager directly (old AdjacencyCluster.UpdateConstructions overload no longer in SAM)
- Qualified SAM.Analytical.Windows.ImportOptions/EventArgs in EditApertureConstructions + EditMaterialLibrary (new SAM.Analytical.UI types shadow them via namespace nesting)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF Aperture construction editors; flip Aperture path off WinForms (wave 7)
- ApertureConstructionWindow (pane+frame MaterialLayersControls) replaces ApertureConstructionForm
- ApertureConstructionLibraryWindow replaces ApertureConstructionLibraryForm; same public surface, EventHandler<T> events
- Rewired EditApertureConstructions (.UI.WPF) to the WPF window + WPF import/export handlers (TCD path preserved)
Completes item 1 (Construction + Aperture library editors). SAM.*.Windows DLL refs remain (other consumers).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF PanelWindow replacing PanelForm; flip Panel editor off WinForms (wave 8)
EditPanel now opens the WPF PanelWindow (read-only identity/geometry,
editable PanelType + custom-parameter grid via ParametersControl) and
picks Constructions through the WPF ConstructionLibraryWindow instead of
SAM.Analytical.Windows.Forms.PanelForm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF ApertureWindow replacing ApertureForm; flip Aperture editor off WinForms (wave 9)
EditAperture now opens the WPF ApertureWindow (read-only identity/geometry,
custom-parameter grid, ApertureConstructionLibraryWindow picker) instead of
SAM.Analytical.Windows.Forms.ApertureForm. This SAM.Analytical.UI.ApertureWindow
is distinct from the airflow-properties SAM.Analytical.UI.WPF.ApertureWindow;
the nested-namespace type still wins in .UI.WPF so EditOpeningProperties is
unaffected (both projects build clean).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF InternalConditionWindow; move EditInternalCondition to .UI.WPF off WinForms (wave 10)
Completed the .UI.WPF InternalConditionWindow stub into a modal wrapper around
the existing InternalConditionControl (single InternalCondition / single Space
ctors, read-only InternalCondition/Space getters; bulk-editor Apply hidden).
Moved EditInternalCondition from SAM.Analytical.UI (which used
SAM.Analytical.Windows.Forms.InternalConditionForm) up to SAM.Analytical.UI.WPF,
where the control lives, and repointed the two AnalyticalModelControl call sites
from UI.Modify. to Modify. (the .UI.WPF Modify partial). Both projects build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF SpaceWindow + OccupancyWindow; move EditSpace to .UI.WPF off WinForms (wave 11)
New single-Space editor in SAM.Analytical.UI.WPF replacing SpaceForm/SpaceControl:
SpaceWindow (editable Name, read-only Guid, InternalCondition with Modify ->
InternalConditionWindow and Remove, ParametersControl, Occupancy -> OccupancyWindow)
and OccupancyWindow (ported the self-contained OccupancyControl incl. the retired
SAM.Analytical.Windows.Modify.UpdateOccupancy helper, inlined). Moved EditSpace from
.UI to .UI.WPF/Modify; repointed AnalyticalModelControl call site UI.Modify.->Modify.
(AnalyticalWindow extension calls rebind to the .UI.WPF Modify automatically). Both
projects build clean.
NOTE: bulk EditSpaces (legacy SpacesForm/808L SpacesControl) is a separate wave - the
existing .UI.WPF SpacesControl is a tree selector, not the bulk parameter editor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF SpacesWindow; move bulk EditSpaces to .UI.WPF off WinForms (wave 12)
New SAM.Analytical.UI.WPF SpacesWindow replacing SpacesForm/SpacesControl(808L):
a read-only ~36-column overview DataGrid of every Space and its key properties /
internal-condition profile values (row VM mirrors the legacy UpdateValues), with
double-click on a row opening the WPF InternalConditionWindow to edit that space's
internal condition. Moved EditSpaces .UI->.UI.WPF/Modify; AnalyticalWindow's
extension call rebinds to the .UI.WPF Modify automatically.
NOTE: the legacy grid's purely-cosmetic per-cell colouring (modified/existing/
read-only/error) is not reproduced; values + double-click edit are preserved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF MechanicalSystemWindow; move CreateMechanicalSystem to .UI.WPF off WinForms (wave 13)
New SAM.Analytical.UI.WPF MechanicalSystemWindow replacing MechanicalSystemForm/
MechanicalSystemControl (legacy riser/space tree actions were NotImplemented, so the
editor surfaces Full Name + Type for reference and the editable Id). Moved
CreateMechanicalSystem .UI->.UI.WPF/Modify (keeps the WPF ComboBoxWindow type picker);
AnalyticalModelControl extension call rebinds to the .UI.WPF Modify automatically.
Fixed a latent self-recursive cast in the original GetMechanicalSystem (now reads the
backing field). Completes the bespoke-editor cluster - all six Edit*/Create* paths are
off SAM.Analytical.Windows.Forms.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Rewire Search/ComboBox/TextBox forms to WPF windows in .UI.WPF (wave 14)
Replaces the WinForms SearchForm<T>/ComboBoxForm<T>/TextBoxForm<T> usages with the
SAM.Core.UI.WPF SearchWindow / ComboBoxWindow<T> / TextBoxWindow:
- AnalyticalWindow: Select Filter (SearchWindow), About (ComboBoxWindow), Select By Guid (TextBoxWindow)
- CreateCaseByAperture/ApertureConstruction/FinShade/WindowSize: Select Filter + the
single-select Aperture Constructions picker.
SearchForm<T> -> non-generic SearchWindow(items, text){SelectionMode}; read back via
GetSelectedItems<T>(). DialogResult.OK -> ShowDialog()==true. Owner set to the host
window (Window.GetWindow(this) from the UserControls). Builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Rewire TreeViewForm + MarqueeProgressForm to WPF in .UI.WPF (wave 15)
SetDefaultLayers and RemoveResults now use the SAM.Core.UI.WPF
MultipleSelectionTreeViewWindow (GettingText/GettingCategory handlers +
SetObjects/GetObjects) instead of the WinForms TreeViewForm<T>. Added a reusable
SelectAll() to MultipleSelectionTreeViewControl/Window so RemoveResults can reproduce
the legacy `@checked: x => true` all-checked default. EditMaterialLibrary's TCD import
MarqueeProgressForm -> ProgressBarWindow.
NOTE: the legacy SetDefaultLayers ExpandAll-when-single-group is dropped (the WPF
window has no ExpandAll) - cosmetic only.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Rewire InternalConditionControl TextBox/TreeView forms to WPF (wave 16)
The new-internal-condition-name prompt now uses SAM.Core.UI.WPF.TextBoxWindow and the
select-parameters-to-reset picker uses MultipleSelectionTreeViewWindow (flat,
GettingText only) instead of the WinForms TextBoxForm/TreeViewForm. Clears the last
WinForms-form construction in SAM.Analytical.UI.WPF.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Rewire Mollier process prompts off WinForms TextBoxForm (wave 17)
MollierForm's five SHR / Epsilon / Enthalpy-Difference / Humidity-Ratio prompts now
use SAM.Core.UI.WPF.TextBoxWindow (read back via TryGetValue<double>) instead of the
WinForms TextBoxForm<double>. Added a ProjectReference from SAM.Core.Mollier.UI.WPF to
SAM.Core.UI.WPF (no cycle - it is the base WPF helper lib). Dropped the WinForms-only
Size tweak on the Humidity Ratio prompt (cosmetic). No `new ...Form` WinForms-form
construction remains anywhere in SAM_UI.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Port WindowHandle + marquee progress to SAM.Core.UI.WPF (wave 18)
Ported the IWin32Window adapter WindowHandle into SAM.Core.UI.WPF (WindowInteropHelper)
and repointed all Core.Windows.WindowHandle sites (AnalyticalWindow x5,
AnalyticalModelControl x2). Added static ProgressBarWindow.Show(name, action[, owner])
- runs the work on a background task behind the indeterminate dialog, then closes -
replacing the WinForms MarqueeProgressForm.Show in Check, SolarSimulation and
NCMNameCollectionWindow. IWin32Window param fully-qualified to System.Windows.Forms
(ambiguous with System.Windows.Interop). Builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Determinate ProgressWindow replacing WinForms ProgressForm (wave 19)
New SAM.Core.UI.WPF.ProgressWindow: a determinate step-based progress dialog shown
non-modally and advanced via Update(text), with a WPF DoEvents (DispatcherFrame pump)
so it repaints between steps. Implements IDisposable (Dispose->Close) so the existing
using(...) call sites keep working. Repointed PrintRoomDataSheets and Simulate (x2)
from Core.Windows.Forms.ProgressForm. Builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF number-only input filter replacing Core.Windows.EventHandler (wave 20)
Added SAM.Core.UI.WPF.Query.ControlText_NumberOnly(object, TextCompositionEventArgs)
(the regex filter ported verbatim) and repointed all 13 PreviewTextInput handlers
across 9 .UI.WPF calculation/view controls off Core.Windows.EventHandler. Placed on
Query, NOT a class named EventHandler - that would shadow System.EventHandler for the
many unqualified delegate fields in SAM.Core.UI.WPF (CS0723/CS0066). Builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Rewire remaining (fully-qualified) WinForms form constructions to WPF (wave 21)
Replaces the new Core.Windows.Forms.X uses the earlier grep missed:
- SearchForm -> SearchWindow in Assign{ApertureApertureConstruction,PanelConstruction,
SpaceInternalCondition} (SearchText prefill + single select preserved)
- ComboBoxForm -> ComboBoxWindow<T> in MechanicalSystemsControl (Topmost) and
TryGetWeatherData (owner bridge)
- TextBoxForm -> TextBoxWindow in FilterWindow
- JsonForm<SAMObject> -> JsonWindow (F12 inspector in AnalyticalWindow)
- TreeViewForm -> MultipleSelectionTreeViewWindow in EnableViewSettings
Added using SAM.Core.UI.WPF to TryGetWeatherData for the ShowDialog bridge. Builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WPF LogWindow replacing WinForms LogForm (wave 22)
New SAM.Core.UI.WPF.LogWindow: a read-only grid of a Log's records (type + message);
replaces Core.Windows.Forms.LogForm in Modify.Check (the post-load model log viewer).
Per-row type icon shown as the type name (cosmetic). Builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: WPF Query.Import model-merge (wave 23)
Add merging Import overloads to SAM.Analytical.UI.Query that mirror the
legacy SAM.Analytical.Windows.Query.Import(AnalyticalModel, ...): pick
objects from a JSON file and merge into a copy of the model (materials/
profiles into the libraries; constructions, aperture constructions,
internal conditions, mechanical system types into the AdjacencyCluster).
The two legacy batch calls (AdjacencyCluster.UpdateConstructions/
UpdateApertureConstructions taking a List) are gone from SAM, so imported
(aperture) constructions are added to the cluster via AddObject, with the
missing-material/profile prompts preserved as WPF MessageBox dialogs.
Repoint all three consumers off SAM.Analytical.Windows.Query.Import:
Modify/Import.cs (owner now System.Windows.Window), Analyticalwindow's
Import Objects button (passes the window), and EditMaterialLibrary's
non-tcd material import. Drop now-unused SAM.*.Windows usings from
EditMaterialLibrary and add SPDX headers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: WPF WeatherDataWindow (wave 24)
New SAM.Weather.UI.WPF project (mirrors the SAM.Core.Mollier.UI.WPF
precedent) with WeatherDataWindow, a WPF replacement for the WinForms
SAM.Weather.Windows WeatherDataForm/WeatherDataControl. Identity fields +
ParametersControl (custom-parameter grid) + a values DataGrid + one
OxyPlot line chart per weather data type. The legacy
System.Windows.Forms.DataVisualization chart and the WM_SETREDRAW P/Invoke
(a DataGridView redraw-suspend optimisation) are dropped; the public
surface (two constructors + the WeatherData property) is preserved.
Repoint both consumers and drop the SAM.Weather.Windows DLL:
- EditWeatherData (SAM.Analytical.UI) shows the WPF window, bridging its
IWin32Window owner to the window's native owner via WindowInteropHelper.
- The standalone SAM Weather app launches the WPF window (UseWPF, WPF Main).
- Dropped the now-unused SAM.Weather.Windows <Reference> from
SAM.Analytical.UI, SAM.Analytical.UI.WPF, SAM.Weather.UI and the app.
- Added the project to SAM_UI.sln (under the WPF solution folder).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix empty WeatherDataWindow value columns (wave 24 follow-up)
The values table showed the Date column but all weather-value columns were
blank: the DataTable columns were named by their description
(e.g. "Global Solar Radiation [W/m2]"), and the '[' / ']' / '/' characters
break WPF's auto-generated DataGrid binding paths.
Name DataTable columns by the identifier-safe enum name and build the
DataGrid columns explicitly with indexer bindings ([Name]), showing the
description as the column header. Switched the grid to
AutoGenerateColumns=False and clear columns on reload.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: WPF Convert.ToBitmapSource for ribbon icons (wave 25)
Add SAM.Core.UI.WPF.Convert.ToBitmapSource(this Bitmap) - a WPF
replacement for SAM.Core.Windows.Convert.ToBitmapSource (same HBITMAP ->
BitmapSource path + Core.Modify.DeleteObject cleanup). Repoint all 77
ribbon-icon call sites in AnalyticalWindow (70) and GeometryWindow (7).
GeometryWindow's only SAM.Core.Windows usage was ToBitmapSource, so drop
the SAM.Core.Windows <Reference> from SAM.Geometry.UI.WPF. (SAM.Analytical.UI.WPF
keeps it - other SAM.Core.Windows consumers remain there.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: Profile editor part A - Set dialogs + IntegerOnly (wave 26a)
First part of porting the SAM.Analytical.Windows Profile editor chain to WPF.
- Add SAM.Core.UI.WPF.Query.ControlText_IntegerOnly (sibling to the existing
ControlText_NumberOnly) - digits/sign only.
- New SAM.Analytical.UI.SetProfileValueWindow + SetProfileWindow, WPF
replacements for the WinForms SetProfileValueForm / SetProfileForm (value/
profile to apply over a range or appended). Public surface preserved
(StartIndex/Count/Value/Append, StartIndex/Append/Profile).
Additive - consumed by the ProfileControl port in the next part.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: Profile editor part B - ProfileControl (wave 26b)
WPF ProfileControl (SAM.Analytical.UI): edits a Profile's daily values via an
editable Index/Value DataGrid + an OxyPlot LinearBarSeries chart, with
Set Value / Set Profile / Remove (wired to the part-A dialogs). Mirrors the
legacy SAM.Analytical.Windows ProfileControl public surface (Profile,
ProfileLibrary, Category, Editable). Edits apply live to the working profile.
Chart auto-ranges the value axis (the legacy forced 0 into range, flattening
small ranges like a 26-28 thermostat - the "could be fixed" chart).
Also port Query.CategoryEnums and add the OxyPlot.Wpf package to
SAM.Analytical.UI. Additive - hosted by ProfileWindow in the next part.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: Profile editor part C - ProfileWindow + EditProfile (wave 26c)
WPF ProfileWindow (SAM.Analytical.UI) hosting ProfileControl, replacing the
WinForms ProfileForm. Repoint the two ProfileForm consumers:
- Modify.EditProfile -> ProfileWindow (IWin32Window owner bridged via
WindowInteropHelper).
- InternalConditionControl.ViewProfile -> read-only ProfileWindow.
ProfileLibraryForm/SelectProfile (the library browser + InternalCondition
edit path) still pending in part D.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix Profile chart rendering (wave 26b follow-up)
The OxyPlot LinearBarSeries merged into one solid block (and OxyPlot 2.2.0
has no ColumnSeries). Switch to RectangleBarSeries with one explicit bar per
hour. Anchor the bar baseline at the data minimum (not 0) so both zero-based
profiles (gain 0..1) and small off-zero ranges (26-28 thermostat) render with
visible per-hour variation instead of a flat block; add 5% headroom above the
tallest bar.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: Profile editor part D - ProfileLibraryWindow + SelectProfile (wave 26d)
WPF ProfileLibraryWindow (SAM.Analytical.UI): profile browser with type
filter + search + Add/Duplicate/Remove/Import/Export, editable Type combo
column (re-categorize), double-click to edit via ProfileWindow, F12 JSON,
MultiSelect/TypeEnabled/Enabled. Mirrors the legacy ProfileLibraryForm
public surface (Type, ProfileLibrary, GetProfiles).
Add WPF Modify.SelectProfile (opens the browser locked to a type, mutates
the library in place, returns the picked profile). Repoint:
- Modify.EditProfileLibrary -> ProfileLibraryWindow (owner bridged).
- InternalConditionControl SetProfile -> SAM.Analytical.UI.Modify.SelectProfile.
- Drop a dead using SAM.Core.Windows.Forms from InternalConditionControl.
SAM.Analytical.Windows still has two consumers (InternalConditionLibraryForm
in InternalConditionControl + the UpdateAreaPerPerson Modify extension in
MapInternalConditionsControl), so its reference stays for now.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: InternalConditionLibraryForm + UpdateAreaPerPerson → WPF (wave 27)
- New SAM.Analytical.UI.WPF.InternalConditionLibraryWindow (browser: search,
Add/Duplicate/Remove/Import/Export, double-click edit via InternalConditionWindow
built on a temp AnalyticalModel wrapping the cluster+profile library, F12 JSON),
replacing the WinForms InternalConditionLibraryForm. Mirrors its public surface
(MultiSelect/Enabled/InternalConditionLibrary/ProfileLibrary/AdjacencyCluster/
GetInternalConditions).
- Port UpdateAreaPerPerson (+ inline UpdateOccupancy) into SAM.Analytical.UI.WPF.Modify.
- Move the IWin32Window EditInternalConditions overload from SAM.Analytical.UI to
SAM.Analytical.UI.WPF (extension call sites auto-rebind); repoint
InternalConditionControl.button_Select + MapInternalConditionsControl; drop their
dead SAM.Analytical.Windows(.Forms) usings.
NOTE: SAM.Analytical.Windows ref NOT dropped yet - a grep using the relative
"Windows.Query/Modify/Forms" name (namespace SAM.Analytical.UI) surfaced more
consumers in Modify/ (AddMissingObjects, Clean, DuplicateInternalCondition,
EditProperties, New, ReplaceApertureConstructions, ReplaceConstructions).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: port Update(Aperture)Constructions for Replace* (wave 27b)
Inline the retired SAM.Analytical.Windows.Modify.UpdateConstructions /
UpdateApertureConstructions (panel/aperture re-point by Guid + add remainder)
into SAM.Analytical.UI.Modify.ReplaceConstructions /
ReplaceApertureConstructions, dropping their Windows.Modify dependency.
(Construction path uses Analytical.Create.Panel - fully qualified, since the
UI namespace shadows SAM.Analytical.Create.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: SAM.Analytical.Windows DLL FULLY RETIRED (wave 28)
Migrate the last SAM.Analytical.Windows consumers and drop the DLL reference
from both SAM.Analytical.UI and SAM.Analytical.UI.WPF:
- AnalyticalModelWindow (WPF) replaces AnalyticalModelForm; repoint EditProperties.
- NewAnalyticalModelWindow (WPF, project name + template combo, merging
Query.Import for templates) replaces NewAnalyticalModelForm; repoint New.
- Port Query.AddMissingObjects + Query.Clean into SAM.Analytical.UI (WPF
MessageBox + MultipleSelectionTreeViewWindow, ProgressForms dropped); repoint
the Modify wrappers.
- Move DuplicateInternalCondition to SAM.Analytical.UI.WPF, merging the retired
Windows.Modify.Duplicate (name-dedup + InternalConditionWindow editor);
repoint AnalyticalModelControl.
- Consolidate the cluster-level UpdateConstructions/UpdateApertureConstructions
as public extensions in SAM.Analytical.UI.Modify (used by both the model-level
Update* and Replace* paths), dropping their Windows.Modify dependency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Retire SAM_Windows: SAM.Core.Windows DLL FULLY RETIRED (wave 29)
Migrate the last SAM.Core.Windows consumers and drop the DLL reference from
all four projects (SAM.Analytical.UI, SAM.Analytical.UI.WPF,
SAM.Core.Mollier.UI.WPF, SAM Analytical app):
- View-settings dialogs -> SAM.Core.UI.WPF: CopyViewSettings /
CopyViewSettingsCamera (ComboBoxForm -> ComboBoxWindow<T>), DuplicateViewSettings
(TextBoxForm -> TextBoxWindow).
- RenameSpacesControl IntegerOnly -> SAM.Core.UI.WPF.Query.ControlText_IntegerOnly.
- GeometryObjectModel text width -> SAM.Core.UI.WPF.Query.Width.
- DuplicateMaterial -> SAM.Core.UI.WPF.Modify.Duplicate.
- Drop dead `using SAM.Core.Windows.Forms` from 8 files.
With SAM.Weather.Windows (w24) and SAM.Analytical.Windows (w28), this retires
the last of the three SAM_Windows DLLs from SAM_UI - no project references any
SAM_Windows assembly. The SAM Analytical app already launches the WPF
AnalyticalWindow; its Core.Windows reference was an unused leftover.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Add missing SPDX/copyright headers to 12 changed files
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fab632c93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| private void Button_Add_Click(object sender, RoutedEventArgs e) | ||
| { | ||
| MaterialWindow materialWindow = new MaterialWindow(null, enums?.ToList()) { Owner = this }; |
There was a problem hiding this comment.
When a user clicks Add in MaterialLibraryWindow, this constructs MaterialWindow with a null material. The MaterialWindow.Material getter immediately returns null while its backing material is null, so even after the user presses OK, materialWindow.Material is null and the handler exits without adding anything. This makes the Add button in the material library a no-op; create a default material/type first or make the dialog build one from the entered fields.
Useful? React with 👍 / 👎.
Comment on lines
+153
to
+155
| case MaterialType.Transparent: | ||
| Material transparentMaterial = (TransparentMaterial)material; | ||
| result = new OpaqueMaterial(transparentMaterial.Guid, name, displayName, description, thermalConductivity, density, specificHeatCapacity); |
There was a problem hiding this comment.
When the edited material is MaterialType.Transparent, this branch returns an OpaqueMaterial. Pressing OK in the material editor therefore converts any transparent material into an opaque one, losing its transparent type and causing glazing/window-layer materials to be stored and displayed incorrectly after an edit. This branch should construct the transparent material type instead of OpaqueMaterial.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| adjacencyCluster = mechanicalSystemWindow.AdjacencyCluster; | ||
| mechanicalSystem = mechanicalSystemWindow.MechanicalSystem; | ||
|
|
||
| uIAnalyticalModel.JSAMObject = new AnalyticalModel(analyticalModel, adjacencyCluster); |
There was a problem hiding this comment.
When the model-tree Create action calls CreateMechanicalSystem, the caller ignores the returned MechanicalSystem, so the only persisted effect is this assignment to a new AnalyticalModel. However the new MechanicalSystemWindow only edits the system Id and never inserts the edited mechanicalSystem into adjacencyCluster, leaving the cluster unchanged; after OK, no mechanical system appears or persists. Add/replace the system in the cluster before rebuilding the model.
Useful? React with 👍 / 👎.
…) (#52) * Fix 4 pre-existing material/aperture/import bugs surfaced by Codex These predate the SAM_Windows->WPF migration (verified identical in the original WinForms MaterialControl/ApertureConstructionForm/Query.Import); PR #51 preserved them faithfully. Fixed here in a follow-up: - MaterialWindow: build TransparentMaterial (was OpaqueMaterial) for the Transparent case, so glazing materials are no longer silently converted. - MaterialWindow: derive material type from the combo box when adding, so the library Add button creates a material instead of returning null. - Query.Import: enumerate AdjacencyCluster.GetApertureConstructions() so aperture constructions embedded in a full model can be imported. - ApertureConstructionWindow: apply the selected aperture type when editing an existing construction (was kept from the original). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * MaterialWindow Add: enable type combo + default to Opaque (Codex #52) The previous Add fix read ComboBox_MaterialType.SelectedItem, but the combo was disabled in XAML and had no default selection, so on the Add flow the type resolved from null, fell through to the switch default, and Add was still a no-op. Now skip Undefined in the type list, and when adding a new material enable the combo and default-select Opaque so a type is always set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 543f4a81bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ConstructionLibrary result = new ConstructionLibrary(constructionLibrary); | ||
| constructionLibrary.GetConstructions().ForEach(x => result.Remove(x)); | ||
|
|
||
| GetConstructions(false)?.ForEach(x => result.Add(x)); |
There was a problem hiding this comment.
When the construction-library search box is non-empty, FillRows rebuilds rows with only matching constructions, but this getter removes every construction from the copied library and then adds only GetConstructions(false) from those visible rows. If a user searches to inspect/edit one construction and presses OK, EditConstructions consumes this property and ReplaceConstructions drops every hidden construction from the model. Keep an unfiltered backing collection, or merge row edits back into constructionLibrary without treating the filtered view as the full library.
Useful? React with 👍 / 👎.
| ApertureConstructionLibrary result = new ApertureConstructionLibrary(apertureConstructionLibrary); | ||
| apertureConstructionLibrary.GetApertureConstructions().ForEach(x => result.Remove(x)); | ||
|
|
||
| GetApertureConstructions(false)?.ForEach(x => result.Add(x)); |
There was a problem hiding this comment.
When the aperture-construction search filter is active, rows contains only matching entries, yet this getter removes all aperture constructions from the copied library and adds back only GetApertureConstructions(false) from the filtered rows. Pressing OK after searching therefore makes EditApertureConstructions replace the model's aperture-construction library with just the visible matches, silently deleting all hidden window/door constructions.
Useful? React with 👍 / 👎.
|
|
||
| InternalConditionLibrary result = new InternalConditionLibrary(internalConditionLibrary); | ||
| internalConditionLibrary.GetInternalConditions()?.ForEach(x => result.Remove(x)); | ||
| GetInternalConditions(false)?.ForEach(x => result.Add(x)); |
There was a problem hiding this comment.
With a search term active, DataGrid_InternalConditions.ItemsSource is the filtered list from RefreshView, but this getter clears the copied library and re-adds only GetInternalConditions(false) from that filtered ItemsSource. If a user searches the Internal Conditions dialog and clicks OK, the caller rebuilds the adjacency cluster from this property and removes every internal condition that was hidden by the search.
Useful? React with 👍 / 👎.
* Make SharpDX the default 3D viewport (#53 Phase E flip) Flip SAM_UI_VIEWPORT_SHARPDX to default ON in SharpDXViewportControl.ResolveEnabled: the SharpDX viewport is now the default 3D renderer. An explicit "0" is the escape hatch that falls back to the intact legacy Helix 3D path, kept for one release; the legacy Helix 3D + ortho-2D paths and the ToMedia3D/Create.Model3D conversion layer are removed in a follow-up PR once the flipped default proves stable. Gate evidence: the Phase B A/B already recorded in the port-plan doc (ToElement3D ~15-20x faster than ToMedia3D, full warm 3D well under 1 s) plus functional sign-off on a large (10k-space) model. Also records, in the port-plan doc, that the #53 Section B camera items are already at Helix parity (saved camera applied once on first load, live camera preserved after; Load-camera round-trips through SetCamera; projection not persisted by either path) - verified, no code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix stale colors: fall back to full regen for batched SharpDX recolor (Codex #54 P1) With SharpDX + batching both default-on after the flip, attribute-only edits took the in-place recolor fast path, but SharpDXViewportControl. RefreshAppearance only re-skins per-object GroupModel3Ds tracked in dictionary_Element3D - which the batched scene never populates - so it was a silent no-op while ViewportControl.RefreshAppearances still returned true, making AnalyticalWindow.TryRefreshSpaceAppearances skip the full-regeneration fallback and leave rendered colors stale. RefreshAppearance now returns bool (false for a batched scene), and RefreshAppearances propagates it so the caller falls back to a full regeneration. A true in-place batched recolor (recoloring an object's triangle range) remains a separate perf optimisation (#53 Section D). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b6cdea92c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
|
|
||
| materialLibrary.Add(material); |
There was a problem hiding this comment.
When a user double-clicks an existing material, edits it, and presses OK, this writes the edited material through Add while the dialog's MaterialLibrary getter later returns the backing materialLibrary, not the edited row. Other edit paths capture the selected material's unique id and call Replace; with Add here the edit can either be rejected as a duplicate or leave the old material alongside the edited copy, so material-library edits are not reliably persisted.
Useful? React with 👍 / 👎.
| continue; | ||
| } | ||
|
|
||
| if (!construction.TryGetValue(ConstructionParameter.DefaultPanelType, out string panelTypeString) || Core.Query.TryGetEnum(panelTypeString, out PanelType panelType_Temp)) |
There was a problem hiding this comment.
For constructions that already have DefaultPanelType, TryGetEnum(...) succeeds, but this condition adds PanelType.Undefined instead of the parsed panelType_Temp; failed parses fall into the else as well. The Change Type dialog therefore opens blank for typed constructions, and simply accepting it removes the existing default panel type. Add the parsed value when TryGetEnum succeeds.
Useful? React with 👍 / 👎.
Comment on lines
+264
to
+266
| int[] indices = SelectedIndices(); | ||
| int count = indices.Length != 0 ? indices.Length : (DataGrid_Values.Items?.Count ?? 1); | ||
| int startIndex = indices.Length != 0 ? indices[0] : 0; |
There was a problem hiding this comment.
The values grid allows extended selection, but Set Value collapses any non-contiguous selection to a contiguous range starting at the first selected index. For example selecting hours 1 and 5 makes count equal 2 and updates hours 1 and 2, leaving hour 5 unchanged; that silently corrupts profile schedules. Iterate the selected indices/ranges or disallow non-contiguous selections.
Useful? React with 👍 / 👎.
* In-place batched recolor for attribute-only edits (#53 Section D) Attribute-only edits (e.g. assigning an InternalCondition) change only space fill colours. In the batched scene those edits previously fell back to a full regeneration (PR #54) because the merged base meshes are grouped BY colour - an object can't be recoloured where it sits. RefreshAppearanceBatched now recolours in place: it degenerates an object's stale-colour triangles in the merged base mesh (collapsed to zero area, so they stop rendering) and attaches a small, correctly-coloured per-object fill mesh rebuilt from the object's already-updated SurfaceAppearances. The extracted mesh gets a single-object pick entry + octree and the object's overlay slices are repointed at it so hover/selection still highlight correctly. Base-triangle degeneration is batched (one index re-upload per affected mesh) and a repeat recolor replaces the previous extraction; Load tears all extractions down. Edges and text are untouched (the fast path doesn't change them). Falls back to a full regeneration only when an in-view object can't be rebuilt from its stored payload. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Drop inline octree build for extracted recolor fill meshes (review) A large multi-select recolor would call UpdateOctree() synchronously on the UI thread once per recoloured object, re-introducing the very stall the deferred Load octree pass exists to avoid. These are small per-object fill meshes, for which FindHits already falls back to a correct linear triangle test (the same fallback the base scene relies on in its deferred-octree window), so the inline build isn't needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d30e45d4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+585
to
+588
| foreach (Construction construction in constructions) | ||
| { | ||
| adjacencyCluster.AddObject(construction); | ||
| } |
There was a problem hiding this comment.
When importing a Construction whose Guid matches an existing panel's current construction, this only adds the construction object to the cluster. Panels embed their Construction (the new UpdateConstructions helper explicitly rebuilds panels by matching panel.Construction.Guid), so existing panels can keep referencing the old layers after an import. Use adjacencyCluster.UpdateConstructions(constructions) here so imported construction edits actually apply to panels that already use them.
Useful? React with 👍 / 👎.
Comment on lines
+590
to
+593
| foreach (ApertureConstruction apertureConstruction in apertureConstructions) | ||
| { | ||
| adjacencyCluster.AddObject(apertureConstruction); | ||
| } |
There was a problem hiding this comment.
When importing an ApertureConstruction whose Guid matches windows/doors already embedded in panels, this adds the construction object but does not replace the Aperture objects using it. The UpdateApertureConstructions helper in this patch removes/re-adds apertures by matching ApertureConstruction.Guid, which shows AddObject alone is not enough; imported glazing/frame edits will otherwise remain unused by existing apertures.
Useful? React with 👍 / 👎.
Comment on lines
+312
to
+313
| int startIndex = indices[0]; | ||
| int count = indices.Length; |
There was a problem hiding this comment.
When the values grid has a non-contiguous extended selection, these values collapse the selection to a contiguous range starting at the first selected index. For example selecting indices 1 and 5 makes count equal 2, so the remove path updates/removes indices 1 and 2 while leaving index 5 unchanged, corrupting the schedule the user selected. Iterate the selected indices/ranges or disallow non-contiguous selection for Remove.
Useful? React with 👍 / 👎.
ToSAM_GeometryObjectModel (both 3D and 2D) deep-cloned the entire AnalyticalModel on every regeneration via `new AnalyticalModel(analyticalModel)`, then read `.AdjacencyCluster` off the copy. But the AnalyticalModel.AdjacencyCluster getter already returns a fresh, isolated AdjacencyCluster clone - so the model clone was redundant, cloning the cluster twice (whole model incl. panels/spaces/ apertures + material/profile libraries + relations, then the cluster again). The cluster was the only thing taken from the cloned model, so read it directly off the source model. Same isolated cluster, one clone instead of two-plus-whole- model, on every 3D/2D regen. Part of #53 Section D. SAM.Analytical.UI builds clean. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
https://github.com/HoareLea/SAM_UI/blob/dd383b25a3f1806a7003893d13777179f938c73c/SAM.Analytical.UI/Windows/ConstructionLibraryWindow.xaml.cs#L123
If a user edits an editable grid value such as Description or Default Type and then types in Search before pressing OK, this clears the Row objects and rebuilds them from constructionLibrary; those edits only live on the rows until GetConstructions(false) is called, so the filter operation silently discards them and OK/Export saves the old construction metadata. Merge the current row state into the backing library before rebuilding, or filter over the existing row collection instead.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1788
to
+1792
| if (hitTestResult.ModelHit is MeshGeometryModel3D meshGeometryModel3D | ||
| && dictionary_PickBucket.TryGetValue(meshGeometryModel3D, out PickBucket pickBucket) | ||
| && hitTestResult.TriangleIndices != null) | ||
| { | ||
| Guid guid_Batched = pickBucket.Resolve(hitTestResult.TriangleIndices.Item1); |
There was a problem hiding this comment.
When the default batched SharpDX renderer is active, curve-only objects such as Segment3D/Polyline/BoundingBox render through LineGeometryModel3D, but this hit-test path only resolves MeshGeometryModel3D hits. Because the batched scene records line slices but no line pick map is consulted here, those visible line-only objects can never be hovered, selected, double-clicked, or used for context actions. Add line hit resolution (or a line pick bucket) alongside the mesh bucket lookup.
Useful? React with 👍 / 👎.
#57) * fix(floorplan): stop label solver hanging on degenerate sections A floor-plan view whose section plane sits at an elevation where spaces collapse to coincident slivers ("Level 0 [20.1m]") made FloorPlan.LabelSolver take ~128 s for 1703 labels on a 10k-space model - every other solve of the same labels was 100-455 ms. Root cause: the solver searches a disk of radius ~farthestPointDistance around each label's anchor (StartingDistance is 0, so max reach over the sweep = IterationCount * ShiftDistance = farthestPointDistance). When a section is collapsed that radius is tiny - far smaller than the label - so no candidate can move a label clear of its overlapping neighbours, and all 1703 labels exhaust the full 100-iteration x 8-direction sweep (each candidate running polygon inside/overlap tests) before falling back to their anchor. Detect that case (whole search reach < the label's own min dimension) and run a single iteration - placing the label at its anchor - instead of the full sweep. Real rooms have a reach of metres, far larger than a label, so this never triggers for them; their placement is unchanged. SAM.Analytical.UI builds clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * floorplan: detect sliver sections by min extent, not reach The first cut keyed the degenerate-section guard on farthestPointDistance, but a section collapsed by a wrong-elevation plane is often a long thin sliver: tiny area yet a large farthestPointDistance, so the reach test missed it and the label solver still hit ~58s on the 10k model. Identify a sliver by its minimum bounding-box extent instead: when the section is thinner than the label in its short dimension, a label cannot meaningfully sit inside it, so place it at the anchor (single iteration) rather than running the futile full sweep. Real rooms are metres across, so this never triggers for them. Pairs with the Solver2D consecutive-unplaced backstop (SAM #30). SAM.Analytical.UI builds clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * floorplan: log label-solver placement breakdown for diagnosis The degenerate-section thinness guard did not engage on the pathological "Level 0 [20.1m]" view (still ~108 s), and the Solver2D failure backstop did not fire either - meaning the labels are placing, just expensively. To target the fix instead of guessing the geometry, log how many labels the solver placed vs left at their anchor, plus how many sections were flagged degenerate, as FloorPlan.LabelSolver.Placement (only when SAM_UI_PERFORMANCE_LOG is on). Paired with the Solver2D wall-clock budget (SAM #30) which now caps the solve regardless of mechanism. SAM.Analytical.UI builds clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
4 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.
Summary
What is delivered and why (1–3 sentences).
Validation
How to verify or test the change.