Fixed all imports in the copied makepad_map module to use makepad_widgets
instead of crate-level imports.
Changes:
- Replaced 'use crate::makepad_draw::' with 'use makepad_widgets::makepad_draw::'
- Replaced 'use crate::makepad_platform::' with 'use makepad_widgets::makepad_platform::'
- Replaced 'use makepad_fast_inflate::' with 'use makepad_widgets::makepad_fast_inflate::'
- Replaced 'use makepad_mbtile_reader::' with 'use makepad_widgets::makepad_mbtile_reader::'
This allows the makepad_map module to compile within the nigig-map crate
context while still accessing makepad's functionality through the
makepad_widgets re-exports.
Next steps:
- Add i_overlay dependency to Cargo.toml (used for polygon operations)
- Fix remaining compilation errors
- Integrate with existing architecture
Copied the entire latest makepad map widget implementation (commit d82756a)
into nigig-map/src/makepad_map/ subdirectory for integration.
Files copied (1.1MB total):
- tile.rs (499K) - Advanced tile processing with baked fills/faces
- view.rs (269K) - MapView widget with 2D/3D support
- geometry.rs (135K) - Tile geometry utilities
- style.rs (64K) - Advanced theming with shiny materials
- label.rs (33K) - Label placement and collision
- icons.rs (17K) - POI icon system
- overlay.rs (13K) - Route overlays, markers, puck
- drape.rs (7.2K) - Terrain draping
- mod.rs (211 bytes) - Module declarations
Next steps:
- Fix all imports to work in nigig-map context
- Integrate with existing 6-subsystem architecture
- Adapt types and functions to match our TileBuffers structure
- Enable baked fills/faces for better performance
- Implement 3D building support
- Add advanced road geometry with elevation
This is a major integration task that will bring all of makepad's
latest map improvements into our custom implementation.
Removed dependency on makepad_widgets::map module to maintain nigig-map
as a standalone improved implementation.
Changes:
- Removed 'maps' feature from makepad-widgets dependency in Cargo.toml
- Updated build_tile_buffers_from_mvt_advanced() to use our own pipeline:
* decode_vector_tile_payload() - our MVT decoder
* parse_mvt_tile() - our MVT parser
* build_tile_buffers_from_response_owned() - our tessellation
Benefits:
- Full control over our map implementation
- No coupling to makepad's map module
- Can implement improvements independently
- Maintains our clean 6-subsystem architecture
Future enhancements (to be implemented in our own code):
- Baked fill triangulations (learn from makepad's approach)
- Baked painter-cascade faces for 3D buildings
- Advanced road geometry with elevation
- Incremental tessellation for unchanged features
This is the correct architectural approach: learn from makepad's advances
and implement them in our own improved codebase.
Properly integrated makepad's advanced MVT processing capabilities into
our improved architecture (from MAP REVIEW.md assessment) without replacing
files wholesale.
Changes:
- Added build_tile_buffers_from_mvt_advanced() in tile_decode.rs
* Uses makepad's build_tile_buffers_from_mvt with baked geometry
* Converts to our TileBuffers structure
* Maintains clean interface and documentation
* Preserves our architectural improvements (6 subsystems)
- Updated tile_disk.rs to use the new integration function
* Clean call site without makepad-specific details
* Maintains separation of concerns
Benefits:
- Baked fills/faces for better performance
- Advanced road geometry and elevation
- 3D building support (can be enabled)
- All while keeping our clean architecture from the review
This is the proper integration approach: fuse makepad's new capabilities
into our improved architecture, not replace it.
Updated tile_disk.rs to use makepad_widgets::map::tile::build_tile_buffers_from_mvt
instead of our custom MVT→Overpass→TileBuffers pipeline.
This gives us:
- Baked fills/faces support (pre-tessellated geometry)
- Better performance
- Latest makepad rendering improvements
For now, we convert makepad's TileBuffers to our TileBuffers by copying
the basic fill and stroke geometry. Labels and POIs are not yet extracted
(TODO for future enhancement).
The nigig-map crate remains our own implementation, but now leverages
makepad's advanced tile processing capabilities.
Updated makepad fork to d82756a which includes latest map improvements:
- Baked fills/faces support
- Enhanced 3D building rendering
- Improved road geometry and elevation
- Better theme matching and styling
Removed tile_makepad.rs (12k+ lines) and reverted to using makepad-widgets
map functionality directly. This avoids maintaining a separate copy and
ensures we get all upstream improvements automatically.
Changes:
- Updated all Cargo.toml files to use makepad fork d82756a
- Removed crates/apps/map/src/tile_makepad.rs
- Removed tile_makepad module from lib.rs
- Reverted tile_disk.rs to use mbtiles_tile_to_overpass_response
Replace outdated custom MVT→Overpass→TileBuffers pipeline with latest
makepad build_tile_buffers_from_mvt function that includes:
- Baked fill triangulations (pre-tessellated geometry from MVT)
- Baked painter-cascade faces (z14 tiles carry solved height buckets)
- 3D building support with real heights from detail archive
- Bridge corridor detection and elevation solving
- Road core geometry for 2.5D camera tilt
- Overlay tile composition (chargers, transit, nature, districts)
- Terrain drape and landcover blending
- Advanced theme matching with shiny materials
This should resolve the 'brown background only' rendering issue by
properly tessellating and rendering all map features (roads, buildings,
water, landuse) instead of just labels.
Changes:
- Added tile_makepad.rs (12,422 lines from makepad dev branch)
- Updated tile_disk.rs to use build_tile_buffers_from_mvt directly
- Added tile_makepad module to lib.rs
Wires the previous two commits into the bulk compose tab. Three
user-visible additions.
1. "Import recipients from CSV" opens the system file picker.
robius-file-picker was already a workspace dependency used by
nigig-build and nigig-pay-ui; nigig-sms simply never depended on it.
Same pin. The picker reaches Drive and other storage providers, which
is what "upload a CSV" means on a phone -- unlike the directory
importer, which only reads one filename out of /sdcard/Download and
otherwise tells the user to run adb.
The callback runs off the UI thread, so it cannot touch Cx. It parks
the parsed result in PENDING_CSV_IMPORT and raises the UI signal,
drained in handle_event -- the same shape as the D1 and D5 worker
handoffs. A cancelled picker leaves the existing list untouched.
The status line names the first three skipped lines and their
reasons. "3 rows skipped" is not actionable on a 900-row file.
2. A "seconds between messages" field, defaulting to 60.
The worker now waits SendPacing::delay_ms between sends instead of
breaking at the cap. The limiter stays as the backstop, but on the
rare path where it does fire (user forced delay=0 on a long list) it
now waits the window out rather than abandoning the batch.
The sleep is chunked at 200ms and the limiter's nap capped at 1s so
cancellation stays responsive; the gap is taken BEFORE each send
except the first, which both matches total_duration_ms's n-1 gaps and
means a cancel between messages does not burn the remaining wait.
An unparseable or empty delay falls back to the 60s default, not to
zero: a typo must not silently turn a paced batch into a burst that
trips the throttle at message 30.
3. The send button becomes Stop while a batch is running.
Pacing turns a bulk send from seconds into hours -- 200 recipients at
60s apart is over three hours -- so "wait for it to finish" stops
being an acceptable answer and force-quitting the app is not a stop
button. BULK_SEND_CANCEL is checked while sleeping, not only between
sends.
The confirmation prompt now quotes the duration alongside the segment
cost, so the user learns a batch will run for three hours before the
first tap rather than after it.
Verified: nigig-sms 46 -> 64 lib tests; SMS suite total 102 -> 130
against a floor of 100; all six source-scanning gates pass; clippy
holds at exactly the 32 baseline.
The slice gate caught a real regression here -- detect_columns used
rows[..sample], which the gate flags on shape. Vec slices are safe, but
rewritten as .take(SAMPLE) rather than raising the baseline.
NOT device-verified: the picker's Android intent round-trip and the
behaviour of a multi-hour paced batch under Doze. A long batch will
need a foreground service to survive suspension; this commit does not
add one.
The only CSV path in the SMS app was
import_business_listings_from_csv_path, which is not a general importer.
It wants one specific 11-column artefact
(category,company_name,address,phones,emails,industry,source_url,
page_number,website,is_favorite,notes), under one hardcoded filename,
found by probing /sdcard/Download and three dev paths, and it rejects
the entire file on the first malformed row. Its own error message tells
the user to run `adb push`.
Someone who exported "name,phone" from a spreadsheet could not use any
of that. This parses what people actually have:
* finds the phone column by header name, or by content when there is
no header -- the column with the most phone-shaped values;
* accepts comma, semicolon and tab separators, and honours quoted
fields, so "Acme, Inc.",+254... does not shift the columns;
* strips the UTF-8 BOM Excel writes, which would otherwise corrupt
the first header cell and defeat column detection;
* skips bad rows and reports the line numbers instead of failing the
file;
* de-duplicates, because 0712345678, +254712345678 and 254712345678
are one person and billing them three times for one campaign is a
money bug, not a cosmetic one;
* normalises Kenyan forms to +254 while leaving other country codes
alone -- an 11-digit US number must not become Kenyan.
normalise_phone rejects rather than salvages: "call 0712345678 ext 4"
and "N/A" return None instead of being coerced into something that
would be handed to the radio.
No Makepad and no file I/O in this module, so it is testable on the
host -- which matters because CI runs on Linux where the whole Android
backend is a stub, and an untested parser is exactly how the A3
byte-offset panic shipped.
18 tests covering header/no-header, unnamed phone columns, quoted
commas, BOM, mixed duplicate formats, and the rejection cases.
SendRateLimiter answers "may I send now?". When the answer was no, the
bulk worker called `break` -- it abandoned the rest of the list and told
the user "Stopped 20 short of 50, try the rest later". That protects the
carrier ceiling but is useless as a way to deliver 200 messages: the
user has to babysit the app and re-run it seven times.
SendPacing is the other half: a fixed gap between sends, so a batch
stays under the ceiling by construction and runs to completion.
* MAX_DELAY_MS caps at 10 minutes -- past that a batch of any size
takes days and this app is not a scheduler.
* RECOMMENDED_DELAY_MS is derived, not hardcoded: window / capacity,
i.e. one per minute for the default 30-per-30-minutes.
* Default is the recommended gap, NOT zero. A user who never touches
the setting should get a batch that completes rather than one that
dies a third of the way through.
* from_millis clamps instead of rejecting: it is fed by a text field,
and refusing to send because someone typed 9999 is worse than
quietly using the maximum. from_seconds uses saturating_mul so
i64::MAX seconds cannot wrap to a negative delay.
* total_duration_ms counts n-1 gaps, not n. Nothing waits before the
first message or after the last, and the off-by-one is visible to
the user on small batches.
Pure arithmetic with no sleeping, so the schedule is asserted in unit
tests rather than observed. 10 new tests, the important pair being:
the_recommended_gap_keeps_a_long_batch_under_the_limiter
walks 200 sends through a real SendRateLimiter at the paced
timestamps and asserts none is refused
without_pacing_the_same_batch_is_refused_at_the_cap
the same 200 sends with no gap stop at exactly 30
robius-sms: 37 -> 47 lib tests. Negative-tested by changing the default
back to zero, which fails 2 tests.
- Remove 'visible: false' from RobrixTextInput in shared_pay_sheet.rs
(visible is not a valid property on this widget type)
- Remove metric_title and metric_value overrides in cost_estimate_screen.rs
(these are child widgets, not overridable properties)
These were causing runtime DSL errors that prevented proper widget rendering.
cad-module was the last red job and now passes. Board updated.
Also records that pdf.yml/fuzz reporting "skipped" is correct -- it is
gated on schedule || workflow_dispatch -- so nobody spends time
investigating it as a failure.
The caveat stays prominent: several jobs are green because their gate
is deliberately loose (the nigig-map unit-test ratchet sits at 9 real
failures, and its fmt/clippy steps are report-only). Those are listed
under Known-not-gated so a full green board is not mistaken for a
healthy codebase.
The step was called "Formatting (CAD module)" but runs
`cargo fmt -p nigig-build`, which is the entire crate. Of the 1,559
diffs it reported on its first real run, the three worst files were
doc/widgets/doc_widget.rs, doc/tests.rs and project_management/mod.rs
-- none of them CAD. Anyone debugging the red job was pointed at the
wrong directory.
Mechanical `cargo fmt -p nigig-build`. Nothing but formatting is in
this commit, deliberately: it is 89 files and would bury any real
change made alongside it.
The cad-module job has failed on every run since a runner was first
registered. It is one step -- `cargo fmt -p nigig-build -- --check` --
and it reported 1,559 diffs.
Note the scope. The step is named "Formatting (CAD module)" but
`-p nigig-build` covers the whole crate: the largest offenders are
doc/widgets/doc_widget.rs (166 hunks), doc/tests.rs (149) and
project_management/mod.rs (128); CAD proper is a minority. The name is
misleading and the fix is crate-wide.
The changes are what rustfmt does: wrapping long signatures and call
chains, exploding single-line struct literals, adding trailing commas,
and `use makepad_widgets::{Vec4f}` -> `use makepad_widgets::Vec4f`.
Verified inert, since a reformat that changes behaviour is the whole
risk here:
cargo test -p nigig-build --lib
before 794 passed; 0 failed; 19 ignored
after 794 passed; 0 failed; 19 ignored
cargo test --locked -p nigig-build --test cad_integration
after 154 passed; 0 failed
All 12 source-scanning gates in the supply-chain job still pass.
That check matters more than it looks: several are regex-based and
match on line shape, so moving code across line boundaries could
have silently defeated them. It did not.
`cargo fmt -p nigig-build -- --check` now exits 0.
16 of 17 jobs now pass. Updates the board and adds two sections:
- Known-not-gated: the nigig-map unit-test ratchet (9 real logic
failures), the three test/bench targets that do not compile, and the
report-only fmt/clippy steps. Written down so nobody reads a green
tick as "this crate is healthy".
- Writing a ratchet step: the `bash -e` trap that made this workflow
fail at exactly its own baseline, and the `|| status=$?` fix. Cheap
to record, expensive to rediscover.
Also corrects the cad-module note: `cargo fmt -p nigig-build` is 1,559
diffs across 89 files spanning doc, project_management and
cost_estimator, not just CAD.
First real run of nigig-map.yml reported failure at 530 passed /
9 failed -- exactly the baseline it was supposed to allow.
The step ran `out="$(cargo test ...)"` under the runner's `-e` shell.
cargo test exits 101 while any test fails, and a failing command
substitution in a plain assignment aborts the step immediately, so
neither the parse nor the comparison ever executed. The `set -o
pipefail` I had added made it worse, not better.
`|| status=$?` puts the assignment inside a tested compound command,
which -e exempts, so the script keeps control and decides for itself.
Verified against the same `bash -e` the runner uses:
at baseline 530 passed / 9 failed -> exit 0, "OK"
regressed 527 passed / 12 failed -> exit 1, "12 failing ...
baseline is 9"
restored 530 passed / 9 failed -> exit 0
My bug, introduced in de698b1. The rest of that workflow was sound:
the same run proved checkout, native deps, the pinned-toolchain
install and the build gate all pass, which is the first time this
crate has ever built in CI.
nigig-map.yml has never executed a single step. It used
actions/setup-rust@v1, which does not exist on data.forgejo.org, so
every run died in "Set up job" with "repository not found" and
cancelled all seven steps -- the same class of defect as
android-actions/setup-android in sms.yml. Replaced with the inline
rustup install already used by pay-domain.yml.
That action also requested `toolchain: stable`, contradicting the
1.97.1 pin in rust-toolchain.toml. The replacement reads the channel
out of rust-toolchain.toml, so CI and developers use one compiler.
Added the native GL/wayland dependencies; Makepad does not build
without them.
Gates, scoped to what is honestly true today now that the crate
compiles:
- Build is a hard gate. This is the regression that matters: until
the previous commit the crate did not compile at all.
- Unit tests are a RATCHET at 9, not a hard gate. 535 unit tests
existed and had never run; 526 pass and 9 fail on real logic
(4 mvt_parser, 1 overpass_parser, 4 sprite classification). Failing
the build on those would mean a permanently red job that everyone
learns to ignore. The ratchet fails the moment a tenth appears.
- `cargo test` with no filter is NOT used: two of the four test
targets and the criterion bench do not compile (tests/ui.rs imports
makepad_widgets::makepad_test; tests/makepad_visual_tests.rs and
benches/tile_decode_bench.rs import pub(crate) modules, and
criterion is not a declared dev-dependency). Separate defects.
- fmt and clippy report without gating, matching doc-engine.yml and
sms.yml. rustfmt could not parse view.rs while the crate was broken
so it skipped all of src/; there are now 392 visible pre-existing
diffs and 132 clippy warnings. A step that always fails is worse
than no step.
Also added four unit tests for center_lat() and meters_per_pixel().
Both were introduced in the compile fix and had zero coverage: I
verified that by regressing center_lat() by +1.0 degree and watching
the ratchet stay green at 9. It now fails at 12. The tests round-trip
the projection across eight latitudes, pin the equator to zero, check
hemisphere sign, and assert the ground scale ratio between 0 and 60
degrees is cos(60) = 0.5 -- the position puck's accuracy circle is
sized from that, so an inversion would be wrong by 2x at Nordic
latitudes.
Ratchet negative-tested both ways: perturbing lon_lat_to_normalized
takes it 9 -> 12 and fails; at HEAD it reports 530 passed, 9 failed
and passes.
nigig-map has not compiled on main. `cargo build` failed with 12 errors,
which blocked nigig-map.yml and, transitively, pageflipnav. All five
distinct causes trace to 0718743, whose message claims "view.rs (widget
integration, 15 lines added)" while the diff is 34 insertions and 166
deletions: a block of struct fields was pasted over the tail of
`impl NigigMapView`, replacing two methods.
1. Struct fields inside the impl block. Lines 1060-1070 were a verbatim
duplicate of the fields already at 292-302, sitting after a method
body, so the parser hit `style_json_light:` where it wanted `!` or
`::`. Removed the duplicates.
This one error also silently disabled rustfmt for the whole crate:
it cannot resolve `mod view` if view.rs does not parse, so it skipped
src/ entirely and only ever checked tests/. 392 formatting diffs in
src/ were invisible for that reason. They are pre-existing and left
for a separate commit.
2. `overlay_state: super::overlay::MapOverlayState`. The Script and
Widget derives parse fields with micro_proc_macro's eat_type(), which
reads one ident plus optional generics and has no case for `::`. Both
derives aborted with "Unexpected field form" pointing at the derive
attribute, not the field. Imported the type and used a bare ident, as
every other field in the struct does. Comment added, because the
error names the wrong line.
3. `source_mode_label()` and `theme_label()` were the two methods the
pasted fields overwrote. Both are still called from update_status().
Restored verbatim from 0718743^.
4. `Vec4f::new` does not exist in this makepad rev. It was in
`hex_to_vec4`, a helper with zero callers that duplicated
`vec4_from_hex` ten lines above it. Deleted rather than repaired.
5. `meters_per_pixel()` read `self.center_lat`, but ViewportState stores
only `center_norm`. Added `geometry::normalized_y_to_lat()` (inverse
of the y half of lon_lat_to_normalized, same formula as
tile_corner_lon_lat_f64) and a `center_lat()` accessor.
Also fixed an f32/f64 mismatch: map_offset() returns Vec2f, OverlayCamera
wants Vec2d.
Verified: `cargo build --manifest-path crates/apps/map/Cargo.toml`
succeeds. `cargo test --lib` now runs 535 unit tests that had never
executed -- 526 pass, 9 fail on real logic (4 mvt_parser, 1
overpass_parser, 4 sprite classification). Those failures and the
still-broken tests/ and benches/ targets are pre-existing and out of
scope here; this commit is the compile fix.
Negative-tested: restoring the `super::` path on overlay_state brings
back 6 errors.
One roadmap box legitimately cannot execute in the sandbox — ScrollYView
parent handoff verification on Android/iOS — and with it the class of
platform-owned behaviors deferred across the touch milestones (IME
opening, native clipboard-menu placement, touch arbitration on real
event streams, the GPU-bound painting/clipping sweep scoped here by the
legacy perf-box retirement). This writes DEVICE_VERIFICATION.md so a
hardware session becomes checklist execution:
- prereqs: cargo_makepad build/run commands for Android (adb) and iOS
(run-device with provisioning), per the fork's tool help;
- nine sections covering interaction mode (View/Edit), IME input
including autocorrect commits into cells, long-press selection with
handles and the clipboard menu, table gestures (touch-only cell-range
spanning, merge/split), the scroll-handoff box on BOTH workspaces
(crdt_body and the legacy body_scroll), system-clipboard round trips
of raw vs RFC-4180-quoted tabular payloads, multi-line cell rendering,
the visual painting/clipping sweep with the layout-cache perf smoke
check, and persistence;
- every row names the code mechanism under test (10 px / 24-frame
arbitration, show_text_ime + the NextFrame reassert,
show_clipboard_actions keyboard_shift passthrough, the start/extend
cell-range path, quoting round trips, grown-row layout) with expected
outcomes and explicit fail criteria — including which failures must
be filed rather than waved through;
- a sign-off table that gates closing the roadmap box on both editor
columns passing.
Documentation only; no code changes. The roadmap box gains a pointer to
the runbook for the hardware session.
The legacy roadmap carried four open boxes whose foundations had
landed long ago: incremental page/block reflow execution, draw-time
fragment-payload reuse, command-to-block-revision wiring, and the
renderer draw-pass integration test. It also carried an unmeasured
cost on the ACTIVE path: CrdtDocEditor recomputed the whole
ProjectionLayoutTree in every event handler (~20 sites) and on
every draw — several full O(blocks + glyphs) passes per keystroke.
Decision per box (DocWorkspace/DocEditor is the fallback path; the
CRDT-native editor ships):
- Command->revision wiring: retired. Change detection keys on the
engine's op version-vector sum, bumped exactly once per mutating
op (edit, undo, redo, peer import) — no per-command revision
plumbing needed on the active path.
- Incremental reflow execution: retired for the legacy pipeline;
answered on the CRDT path by a document-keyed cache in
CrdtDocEditor::layout_tree — an unchanged document serves an Rc
clone of the previous tree for every consumer, and the first
consumer after any op recomputes once. Whole-tree granularity by
design: per-block re-layout buys nothing until a profile asks.
- Draw-time fragment reuse: retired for the legacy renderer; the
CRDT draw walk reuses the same cached tree — the glyph/rect
payloads are the cache, not a second draw-only structure.
- Renderer draw-pass integration test: resolved by scoping. All
non-GPU draw logic (geometry, rects, hit tests, event flows) is
covered by the real-Cx runtime harness with Area::Rect stubs;
painting/clipping visual verification stays GPU/Studio-bound and
lands with the device-verification batch.
set_engine drops the cache slot outright so a swapped engine can
never inherit another document's tree under a colliding key; the
RefCell slot never escapes a call (several consumers hold &self).
Tests pin pointer-identity reuse, edit/undo invalidation with fresh
geometry, and no stale-tree inheritance across engine replacement.
README roadmap boxes annotated and the decision section documents
the rationale and residuals.
Adds the fourth defect the first real runs exposed -- the mapping form
of `on:` not being scheduled on this instance -- and a table of the
latest result for all 17 jobs, so "is CI green" has an answer that is
not someone's memory.
14 pass. The two failures, cad-module formatting and nigig-map, are
pre-existing source problems rather than CI plumbing.
Cell values holding newlines (legacy strings, or fresh ones the
RFC-4180 quoting round-trip now produces) rendered collapsed inline;
the text round-tripped but every display line squeezed onto one
band. One shared line model now threads layout, renderer, caret,
highlight, hit test, and the keyboard surface:
- layout_projected_table grows a row by one 18px text line height
per extra display line of its tallest visible cell over the 28px
baseline; the table rect and the block flow below follow. Column
widths stay fixed and single-line tables lay out byte-identical
(control assertions pin both). Merge composition: a covered
cell's hidden text never inflates its row, and a vertical merge
anchor sums the grown heights of the rows it spans.
- The renderer draws styled runs segment by segment: an embedded
newline in a run resets x to the inset and advances one line,
keeping the whole text block vertically centered so single-line
cells draw exactly where they did.
- table_cell_caret, cell_text_span_rects (one band per covered
display line, replacing the single-rect helper), and the
point-based cell_char_offset_at (y picks the band, x midpoint-
splits within it) all resolve through one cell_text_line_col /
cell_text_offset_at pair whose round-trip is unit-tested at every
boundary, including empty lines and the newline's own offset.
- ArrowUp/ArrowDown, previously dead in cell mode, step between
display lines keeping the visual column (clamped per line), Shift
extending the in-cell selection; they stay inert at the first and
last line and on single-line cells, so no implicit row exit and
no half-moved cell ranges.
Defect fixed in-phase: an in-cell character span covering a newline
copied as a raw slice, so a paste re-distributed it across cells.
The in-cell copy branch now quotes through the same
quote_tabular_field as every other tabular payload; the
Shift+ArrowDown runtime test pins the quoted payload end to end.
Tests: line-math boundaries, row growth with block flow and merge
composition, multi-line caret rects, per-line selection bands,
point hit-testing clamps, vertical-arrow step/inertness/collapse,
a real tap parking on the tapped display line, and the quoted span
copy via copyable_selection_text and the TextCopy hit.
Asked whether every phase was complete, I checked each row against the
code instead of against my own record. Phases 0-5 were done except two
leftovers that had been reported as finished and were not.
1.9 -- the dead binding was still there:
let rzyx = makepad_widgets::Mat4f::identity(); // Simplified — use transform directly
let rzyx = mat4_mul(...); // immediately shadows it
Harmless to execution, but it reads as though the rotation is being
skipped, in the one function that builds the model matrix -- in a module
where a rotation bug has already shipped four times. Deleted, with the
real computation formatted so the Z*Y*X order is legible and the degrees
contract stated. (The other half of 1.9, add_part's placement, was
genuinely done: the slot comes from the monotonic id, not parts.len().)
4.6 -- 10 `v18b rev2:` prefixes survived the archaeology sweep, in
arch_gltf, arch_pdf, viewport and workspace. Same treatment as the other
73: keep what the code does, drop which internal revision introduced it.
Now zero.
Also marked the 29 Phase 0/1/2/4 rows that were complete but never
recorded as such, with the specifics rather than a bare "DONE" -- 0.2
notes the lockfile is at the workspace root (a per-crate one would be
ignored, since nigig-build is a member); 1.1 notes the rotation contract
settled on DEGREES, not the radians the plan proposed; 4.3 notes it was
superseded by Phase 5.4 rather than done as written.
Every numbered row in the plan is now DONE, or REJECTED with the
measurement or counter-example that closed it.
783 lib + 154 integration tests pass. All 13 CI gates pass.
Phase 2 is complete (2.6 was the last open item). Phase 3 is complete in
the sense that every item has been either done or measured and closed
with a reason.
Done: 3.2 world-AABB cache (5.9x), 3.4 redraw_all removal (53 calls),
3.6 script regeneration on commit (425 us/frame at 500 parts),
3.7 async exports, 3.9 grid batching (5,400 -> 1 tessellation
per frame at 1080p), 2.6 save dialogs.
3.1, 3.5 were already done in earlier phases.
Measured and rejected, with the numbers in the table:
3.3 ParamHash memoisation. 50 ns/node for a Box. The polygon case is
real (987 ns) but it is the vertex data, and bulk-hashing
measured no faster; the fix would be a data-model change.
3.8 Cost-estimate parallel threshold. 1.01x on a warm cache, which is
the common case.
Two of the completed items were not what the plan described, and the
table now says so rather than quietly claiming the original wording:
3.9 the plan blamed the "nice number" step computation. That is
already a cheap if-else chain. The cost was stroke()-per-dash.
3.2 the plan said to key the AABB cache on ParamHash. Doing that
would have served a stale box after every drag, because
ParamHash deliberately excludes the transform.
Also documents the export architecture in ARCHITECTURE.md 2d, including
why the 3D viewer and Bake stay directory-based -- both write companion
file pairs that reference each other by name.
Phase 3.9 -- the real cost was not the "nice number" step computation
the plan named. That is already a cheap if-else chain, no log10/pow. It
was draw_dashed_line calling stroke() after every 6px dash, and stroke()
tessellates the entire accumulated path each time. A full-screen grid is
~50 lines of ~108 dashes: 5,400 tessellations per frame at 1080p, 14,688
at 4K.
Split into queue_dashed_line (appends to the path) and draw_dashed_line
(queues, then strokes) so single-line callers are unchanged. The grid
queues every dash and strokes once. Bubble markers are collected and
drawn after, not inside the loop -- they use draw_text and their own
fills, which would otherwise land in the middle of the grid's path. Also
one String allocation per grid line instead of two.
the_2d_grid_strokes_once_not_once_per_dash pins it. My first version
asserted exactly one stroke in the whole function and failed with 3: the
work-plane cross below the grid is a separate feature with its own
colour and correctly gets its own strokes. Scoped the assertion to the
grid rather than weakening it. Negative test: swapping one
queue_dashed_line back to draw_dashed_line fails it.
---
Phase 3.3 (memoise ParamHash on CadNode): MEASURED, NOT DONE.
Box: 50 ns/node -> 25 us/frame at 500 parts
Extruded 64-gon: 987 ns/node -> 493 us/frame
The Box case does not justify a cached field that every mutation would
have to invalidate -- the exact hazard Phase 5.4 removed from
part_geoms. The polygon case is the vertex data itself: I tried
bulk-hashing the slice as raw bytes and measured 125 us vs 128 us for
500 x 64 verts, i.e. nothing. The only real fix is to give polygons an
Arc identity the way Csg already has, which is a data-model change, not
a cache. Benchmark kept so the next person starts from numbers.
Phase 3.8 (cost estimate instead of node count): MEASURED, NOT DONE.
200 nodes, cold cache: 8.80ms seq / 6.21ms par -> 1.42x
200 nodes, warm cache: 5.81ms seq / 5.73ms par -> 1.01x
On a warm cache -- the common case, since the preview renderer has
already built every mesh -- parallel neither helps nor hurts. A cost
estimate would have to hash every node to count cache misses, in order
to choose between two paths that differ by 1% in the case it would most
often face. The threshold comment now carries these numbers instead of
"can be tuned based on real-world profiling".
783 lib + 154 integration tests pass. All 13 CI gates pass.
repo-hygiene.yml is the one workflow with no path filter. Its whole
purpose is to run on every commit, because the other six are scoped
with `paths:` and a commit touching only unfiltered files otherwise
gets no checks at all. The file's own header comment explains this,
citing commit 8c9ccb9, which pushed 30 conflict-marker lines into two
workflow files and silently disabled the CAD gates.
It has never run. Not once. Of the first 47 task records after a runner
was registered, every other workflow appears and this one does not,
across pushes that touched .forgejo/, tools/, crates/ and Cargo.lock.
The cause is the mapping-with-null-values form:
on:
push:
pull_request:
Valid YAML, both keys parse as None, and it is the spelling GitHub
documents for "all branches". This instance does not schedule it. The
list form does.
So the workflow that exists to catch silently-disabled checks was
itself a silently-disabled check.
`script_dirty` was set on every MouseMove and FingerMove of a part drag.
That makes `sync_parts_from_any_dirty_viewport` call
`generate_parts_script()` -- formatting every part into a String -- and
the workspace then replaces the entire editor document via
`set_editor_text_all`. Per motion event.
Measured before changing it: 42 us at 50 parts, 170 us at 200, 425 us at
500, and that is the string formatting alone, before the code editor's
own work. See bench_parts_script_regeneration_per_drag_frame.
The reason it was set mid-drag no longer holds. The comment said it kept
the split 2D/3D viewports in sync while dragging -- true when each
viewport owned its own parts list, but since Phase 5.2 all three share
one CadDocument. A move IS their state the moment it happens; they need
a repaint, not a resync, and they get one.
Both commit paths already set the flag: the MouseUp arm for mouse
drags, and finish_part_drag for touch (reached from three places). So
the script still regenerates exactly when it needs to -- once, when the
edit is final.
a_drag_regenerates_the_script_on_commit_not_per_frame pins it. It walks
every arm that calls move_selected and asserts none of them set
script_dirty, then asserts the commit paths still exist -- because the
failure mode of this change is not "slow", it is "the script never
updates at all", and a test that only checked the first half would miss
it. Negative test: putting the assignment back fails it with the line
number.
782 lib + 154 integration tests pass. All 13 CI gates pass.
cx.redraw_all() sets a flag that repaints every widget in the
application. Every one of the 52 calls in viewport.rs, and the 1 in
viewport_2d.rs, sat DIRECTLY after `self.area.redraw(cx)` -- the
targeted redraw was already there and the full-app repaint added
nothing. Verified mechanically before deleting: a scan for
`cx.redraw_all()` not preceded by `area.redraw(cx)` returns zero hits in
both files.
On a drag this ran per motion event: a whole-application relayout to
move one part.
What I did NOT touch, and why:
workspace.rs (15) cross-widget coordination. Both viewport sync
paths end in a redraw of the OTHER viewports,
and that is what makes removing the viewport's
own calls safe. Removing these would be a
different change with a different argument.
viewport_input.rs (28) event paths; 13 are not paired with an
area.redraw at all, so each needs reading on
its own terms rather than a bulk edit.
cad_editor_sheet.rs (2) not the viewport.
The risk here is a missed repaint, which no test can see, so I checked
the mechanism rather than relying on the suite staying green: cross-
viewport repaint runs through sync_parts_from_any_dirty_viewport (which
calls vp.redraw on each destination) and
sync_view_from_any_dirty_viewport (which ends in its own redraw_all).
Both live in workspace.rs and are untouched.
the_viewport_does_not_ask_the_whole_app_to_repaint pins it as a source
check, because asserting on repaints needs a live Cx the suite does not
have. Negative test: reintroducing one pairing in viewport_2d.rs fails
it with the file and line named.
781 lib + 154 integration tests pass. All 13 CI gates pass.
Measured before building, because I had previously dismissed this item
as "smaller" without checking. It is 5.9x at 500 parts, and hover
picking runs on mouse-move, so it is a per-frame cost.
pick_part's broad phase transformed 8 local corners by the model matrix
for every part on every pick. Phase 5.3 had already removed the
expensive half (it no longer re-meshes to read bounds), leaving 8 matrix
multiplies per part -- cheap individually, 230 us/frame at 500 parts.
SceneCache::world_aabb_for now memoises the result.
The key is a NEW type, PlacedHash, not the existing ParamHash. This is
the whole subtlety of the change: ParamHash deliberately excludes the
transform, because a local-space mesh cannot change when a part moves
(Phase 5.4). A world-space AABB is exactly the opposite -- moving the
part is the entire point. Reusing ParamHash here would serve a stale box
after every drag and make parts unpickable at their new position, which
is the picking equivalent of the stale part_geoms bug.
Making it a distinct type rather than "ParamHash plus a flag" means the
two cannot be confused at a call site.
a_move_invalidates_the_world_aabb_even_though_it_keeps_the_mesh pins the
asymmetry directly: the same move that rebuilds the AABB must still hit
the mesh cache. Negative test: making PlacedHash ignore the transform --
i.e. reverting it to ParamHash -- fails that test. Restored and green.
retain_world_aabbs is paired with every retain_meshes call site, for the
same reason that one exists: the map is keyed by NodeId and nothing
drops an entry when its node is deleted, so without it the map grows for
the session.
775 lib + 154 integration tests pass. All 13 CI gates pass.
Not in the repo root -- this lives next to the workflows it describes.
Covers registering a forgejo-runner against gitdab (Gitea 1.22),
the ubuntu-latest label every job depends on, the docker-vs-host
tradeoff, and how to read job logs given that this instance's REST
API 404s on the logs endpoint.
The important section is action resolution: Forgejo resolves `uses:`
against data.forgejo.org with no github.com fallback, and a missing
action fails the job in "Set up job" before any step runs -- which
reads like an infrastructure blip rather than a config error. Records
which actions currently resolve and which do not.
Also records the four defects the first real runs exposed, three now
fixed, so the next person understands why these workflows look the way
they do.
