Add map notes with alignment and sharp text rendering

[alcatron]

Add an Add Note editor action and a dialog for multiline text, left/centre/right alignment, standard colours with a custom picker, fonts, move, clone and delete. New notes default to black Vera Sans at 9 pt and are saved as label-only map nodes with editor hints.

Render note glyphs at twice the density while retaining logical dimensions, coordinates and clickable bounds. Preserve foreground ordering and composite furniture once. Allocate both new canvases before suppressing normal glyphs; if either allocation fails, retain the logical-resolution map and readable note text. Submitted and loaded notes resolve missing/unreadable font files, invalid font headers or unavailable slots to a built-in font before text sizing/drawing, including poller/live rendering. Regression images assert readable fallback glyphs.

Save configurations through checked atomic replacement under a destination-directory mutex shared with registration and deletion. Send the loaded page revision with editor mutations and reject stale pages with HTTP 409 and reload guidance. Compare the exact request-read revision again under the mutex before replacement so concurrent edits and repairs cannot overwrite newer content. Update the database title cache only after saving the configuration. Create-only writes protect new and cloned map filenames. Clones retain the shared directory mutex through database registration and remove their own file and partial records if registration fails or throws. Raw configuration edits do not revalidate their old page token after saving; concurrent CAS failures return HTTP 409, with move/delete/note-property actions preserving the helper-selected status. Clone IDs are bound in both registration queries. Repair stages complete copies of legacy backgrounds/icons under the config mutex, retaining originals so rejected or failed writes cannot break existing references. The initial controls and JavaScript revision use one WeatherMap snapshot. Successful saves reload the complete editor through a clean view URL, preserving filenames and return destinations while removing stale revision/action parameters. A file lost during ReadConfig is rejected when its revision is null, with HTTP 404. Missing targets and save failures return errors with neutral retry guidance. Regression tests exercise concurrent creators/readers, stale repairs, raw fragment edits, both canvas allocation failures and process-isolated upgrade stubs.

New notes preserve literal backslashes through a reversible encoding while retaining real line breaks and legacy label semantics. Unavailable numeric font slots use the built-in fallback. Failed new-map creation stops the controller with the original error status and retry guidance. Existing configs can be registered from a read-only directory without requiring creation of a new mutex. Editor snapshot tokens and commit checks include all loaded INCLUDE dependencies; external dependency locks reject contention without deadlocks, and read-only dependencies remain usable.

Editor image/capture dimensions and note coordinate limits use effective logical dimensions after background sizing and post-processing. An independent preview preserves the configured canvas and original map objects. Symlink writers hold logical and resolved-target mutexes.

Legacy asset repair confines sources to the plugin tree and verifies raster decoding before copying. Font fallback also verifies that GD can parse the font. Save and Enter submission restore the active dialog action.

Resolve symlink INCLUDE filenames to physical targets and check save revisions after all destination/dependency locks are held. Dimension previews use an independent image map. Existing repair destinations must decode as regular, non-symlinked raster files.

Mutation routes require loaded revisions; positioned notes exclude marked templates without coordinates. Clone failures log internal details server-side and show generic retry guidance.

Canonical source/destination identity preserves CAS across aliases. INCLUDE snapshots detect alias retargeting; new-destination copies bind the source root revision. Cleanup attempts are independent; the config remains when map-row removal is uncertain, and failed unlink is logged. Root target identity is captured at read time and checked in page tokens and commits.

Validation: affected branch PHP 8.3 and browser regressions pass in isolated Cacti fixtures. The combined changes pass 292 PHP tests (5859 assertions) and 16 browser regression programs. Changed-line coverage passes at 100% of measured production lines; existing controller/entry-point exemptions remain. Syntax, whitespace, translation and manifest checks pass. These revisions have not been deployed to the live installation.

Based on current develop, including the merged icon picker and gallery resize. Shared editor files overlap with the other feature PRs.

[Copilot]

🟡 Changes recommended

Note deletion can silently remove connected links, and save/render consistency issues remain.

6 open findings

• Route note deletion through guarded connected-link confirmation · New
• Propagate configuration write and close failures · New
• Add newly created notes to the render layer index · New
• Add browser regression coverage for the new note UI · New
• Add the standard GPL v2 header to the new test · New
• Use tabs for PHP test method indentation · New

What changed in this PR

Adds editable map notes and high-density text rendering while preserving logical map dimensions.

Changes:

• Adds note creation, styling, movement, cloning, deletion, and validation.
• Renders eligible note text at 2× resolution.
• Adds localization and engine regression coverage.

File	Description
weathermap-cacti-plugin-editor.php	Adds note controls and dialog.
js/​editor.js	Implements note editor interactions.
lib/​editor.notes.php	Validates and persists note data.
lib/​editor.actions.php	Adds note save endpoint and metadata.
lib/​editor.inc.php	Registers note helpers and translations.
lib/​WeatherMap.notes.php	Implements high-density note rendering.
lib/​WeatherMap.class.php	Integrates note composition and thumbnail scaling.
lib/​WeatherMapNode.class.php	Adds multiline alignment and rendering state.
tests/​Unit/​EditorNotesEngineTest.php	Wraps the engine regression test.
tests/​Support/​EditorNotesEngineRegression.php	Tests note persistence, rendering, and validation.
locales/​po/​cacti.pot	Adds translatable note strings.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The built-in live view displays high-density PNGs at twice their logical dimensions, and palette labels are not extractable for translation.

2 open findings

• Propagate configuration write and close failures
• Add browser regression coverage for the new note UI

4 resolved since last review

• Route note deletion through guarded connected-link confirmation
• Add newly created notes to the render layer index
• Use tabs for PHP test method indentation
• Add the standard GPL v2 header to the new test

Previously missed (4)

In code that hasn't changed since last review

Constrain live-view images to logical map dimensions

lib/​WeatherMap.class.php:4186

The high-density output changes the PNG's intrinsic size, but the built-in live-view caller renders ?action=liveviewimage without width/height attributes (weathermap-cacti-plugin.php:206). A 600×300 map with a note is therefore displayed at 1200×600 in live view, contrary to the logical-size guarantee. Update that rendering path to constrain the image to the map's logical dimensions (and add a regression for it).

Document function parameters and return value in PHPDoc

lib/​WeatherMap.notes.php:29

This new function's PHPDoc does not follow the repository-required shape because its parameter and return value are undocumented. Add typed @param and @return entries.

This issue also appears on line 60 of the same file.

Indent else-block statement consistently

lib/​WeatherMapNode.class.php:803

The statement inside this else block is not indented one level deeper, unlike the surrounding PHP control structures. Add one tab so the branch structure remains clear.

Use literal gettext calls for translatable palette labels

weathermap-cacti-plugin-editor.php:390

Passing $label dynamically to __esc() prevents gettext extraction. The regenerated POT contains none of these eight plain colour names, so palette labels cannot be translated. Use literal gettext calls for each array value and regenerate locales/po/cacti.pot.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

High-density rendering incorrectly excludes notes beneath non-overlapping higher-layer nodes, and note deletion displays an inaccurate confirmation.

2 open findings

• Propagate configuration write and close failures
• Add browser regression coverage for the new note UI

Previously missed (2)

In code that hasn't changed since last review

Use note-specific confirmation for deleting notes

js/​editor.js:587

The note-specific wmEditorText.noteDelete message added by this change is never used. Calling delete_node() shows the generic node confirmation and appends “The Cacti device and graphs will remain,” which is inaccurate for a label-only note. Use a note-aware confirmation (while still warning about any connected links) and omit the device message.

Scan all layers to preserve non-overlapping sharp notes

lib/​WeatherMap.notes.php:43

This only scans the single highest real z-layer. For example, a note at the default layer 600 plus a non-overlapping node at layer 700 starts with an empty $notes list and returns no sharp notes, so the note loses high-density rendering even though the later node does not overlap it. This also contradicts the stated behavior that later nodes block only annotations they overlap. Traverse the sorted layers in draw order so eligible notes are collected before later nodes are checked for overlap, while retaining the conservative handling for links and overlapping objects.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

High-density composition can reorder higher map layers, and localized interfaces may select the wrong default font.

3 open findings

• Reapplying furniture incorrectly changes foreground z-order · New
• Propagate configuration write and close failures
• Add browser regression coverage for the new note UI

Previously missed (6)

In code that hasn't changed since last review

Select bundled fonts using stable identifiers, not localized labels

js/​editor.js:1748

This default-font lookup depends on the English option label. When Vera 9 is already registered, wm_style_font_options() uses its numeric font ID instead of the bundled: value; in a locale that translates the label, neither condition matches and the note silently defaults to font 3 rather than Vera Sans 9 pt. Expose and select a stable bundled-font identifier (for example, a data attribute) instead of matching localized display text.

Use cacti_count() instead of raw count()

lib/​WeatherMap.notes.php:85

Use the repository's cacti_count() wrapper for this new production code instead of raw count(), as required by the plugin's compatibility convention.

Use cacti_count() instead of raw count()

lib/​WeatherMapNode.class.php:371

New or edited Cacti plugin code should use the repository's count wrappers rather than raw count(). Replace this with cacti_count($noteLines) to follow the plugin-wide compatibility convention.

Add required PHPDoc to named helper

tests/​Support/​EditorNotesEngineRegression.php:34

This new named helper is missing the repository-required PHPDoc shape (description, @param entries, and @return). Add a block documenting the assertion value, message, and void return before the function.

This issue also appears on line 152 of the same file.

Use cacti_count() in test code

tests/​Support/​EditorNotesEngineRegression.php:119

Use cacti_count() rather than raw count() in this new test code as well, matching the repository's required Cacti compatibility wrapper convention.

Add required PHPDoc to test method

tests/​Unit/​EditorNotesEngineTest.php:31

The new test method is missing the PHPDoc block required for every PHP function/method in this repository. Add a one-line description and @return void while retaining the process-isolation attributes.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Layer composition can break z-ordering, and move/delete failures remain silent.

9 open findings

• Reapplying furniture incorrectly changes foreground z-order
• Propagate configuration write and close failures
• Note moves ignore failed configuration writes · New
• Note deletions silently ignore configuration write failures · New
• Refresh only initialized selectmenu widgets · New
• Replace raw count() with cacti_count() · New
• Use cacti_count() in production code · New
• Use cacti_count() in added PHP code · New
• Add browser regression coverage for the new note UI

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Rendering order, repeated compositing, font defaults, and non-atomic failed writes can produce incorrect output or data loss.

12 open findings

• Direct truncating writes can destroy map configuration on failure · New
• Reapplying furniture incorrectly changes foreground z-order
• Propagate configuration write and close failures
• Font option deduplication breaks Vera Sans 9 default selection · New
• Furniture bitmap is composited twice, darkening antialiased edges · New
• Refresh only initialized selectmenu widgets
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Use cacti_count() in added PHP code
• Use cacti_count() in production code
• Replace raw count() with cacti_count()
• Add browser regression coverage for the new note UI

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

High-density rendering composites map furniture twice, and missing note fonts can disrupt editor JavaScript generation.

6 open findings

• Propagate configuration write and close failures
• Fallback when note font definition is unavailable · New
• Font option deduplication breaks Vera Sans 9 default selection
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Add browser regression coverage for the new note UI

7 resolved since last review

• Direct truncating writes can destroy map configuration on failure
• Reapplying furniture incorrectly changes foreground z-order
• Furniture bitmap is composited twice, darkening antialiased edges
• Refresh only initialized selectmenu widgets
• Use cacti_count() in added PHP code
• Use cacti_count() in production code
• Replace raw count() with cacti_count()

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

It combines rendering, filesystem synchronization, database cleanup, and editor concurrency changes that warrant final human integration review.

0 open findings

3 resolved since last review

• Perform revision check after acquiring target and dependency locks
• Lock resolved symlink target for included file dependencies
• Use an independent image map for preview clones

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Note-marked template nodes render incorrectly, and clone failures may disclose internal exception details.

0 open findings

Previously missed (2)

In code that hasn't changed since last review

Skip marked template nodes without valid drawable coordinates

lib/​WeatherMap.notes.php:94

Template nodes have x === null, but a template carrying SET editor_note 1 currently enters the sharp-note list. The normal pre-render loop skips it, while the new sharp path pre-renders it with null coordinates and draws its label near the origin; it also doubles the output bitmap even though the template should not render. Require a real position before treating a marked node as a drawable note.

Prevent database exception details from reaching the management UI

weathermap-cacti-plugin-mgmt.php:2043

This catch also handles exceptions thrown by sql_save() and the database helpers, so returning $error->getMessage() can expose SQL/schema or connection details to the management UI. Log the original exception server-side and show a translated generic clone-failure message instead; preserve specific user-facing failures through a dedicated safe exception type if needed.

🧠 Review effort: Balanced

[alcatron]

Addressed in 25de595. Both additional review-summary findings are addressed. Sharp-note eligibility requires real numeric X/Y coordinates, so marked template nodes remain non-rendering and do not cause a doubled bitmap; a rendering regression verifies this. Management cloning logs original database and cleanup exceptions server-side and shows only translated generic retry guidance. Registration-failure regressions verify the displayed message is generic and diagnostics are logged. Shared mutation routes also now require loaded page revisions. Validation: 292 PHP tests (5834 assertions), 16 browser regression programs; measured coverage, syntax, translation, manifest and whitespace checks pass. The live installation is unchanged.

[Copilot]

🟡 Changes recommended

Alias writes can bypass revision CAS, and failed clone cleanup can silently leave partial database records.

1 open finding

• Canonicalize paths to preserve stale-write protection · New

Previously missed (2)

In code that hasn't changed since last review

Handle failed cleanup to prevent silent partial registration

weathermap-cacti-plugin-mgmt.php:2040

These cleanup calls ignore false, even though the registration inserts above explicitly treat that return value as failure. If any DELETE fails without throwing, the clone file is removed and the user sees a generic failure, but partial map/auth/settings records remain silently. Use a database transaction for registration and rollback it on failure, or at minimum verify every cleanup result and surface/log incomplete cleanup.

Use cacti_count() instead of count()

lib/​WeatherMap.notes.php:72

Repository guidance requires Cacti's counting wrappers in new or edited PHP code. Use cacti_count($bounds) here instead of the raw count() builtin.

🧠 Review effort: Balanced

[alcatron]

Addressed in 72b3bf6. Both additional review-summary findings are addressed. Every cleanup DELETE return value is checked and failures are logged server-side; all three cleanup operations are attempted when they return false. A regression injects failed cleanup and verifies all three incomplete-removal diagnostics. The generic UI message still hides internal details. The font probe now uses cacti_count($bounds). Validation: 292 PHP tests (5845 assertions), 16 browser regression programs; measured coverage, syntax, translation, manifest and whitespace checks pass. The live installation is unchanged.

[Copilot]

🟡 Changes recommended

Clone failure cleanup can leave either broken database registrations or orphaned configuration files.

1 open finding

• Avoid deleting map files when database cleanup fails · New

1 resolved since last review

• Canonicalize paths to preserve stale-write protection

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Server-side byte counting conflicts with the UI’s character limit and rejects valid Unicode notes.

0 open findings

1 resolved since last review

• Avoid deleting map files when database cleanup fails

Previously missed (1)

In code that hasn't changed since last review

Use consistent character limits for UTF-8 notes

lib/​editor.notes.php:79

The textarea advertises a 2,000-character limit, but strlen() enforces 2,000 bytes. Valid UTF-8 notes can therefore pass browser validation and then receive HTTP 400 (for example, 1,001 CJK characters are below maxlength but exceed 2,000 bytes). Count UTF-8 characters here, or make the client and server limits explicitly use the same unit.

🧠 Review effort: Balanced

[bmfmancini]

Reviewed the diff across WeatherMap.notes.php, editor.notes.php, editor.actions.php and the node-rendering hooks. Request handlers now go through wm_editor_read_config()/wm_editor_write_config() — mutation paths require a loaded page revision token validated with hash_equals against snapshotRevision(), returning 404/409/422 with HTTP status rather than silently proceeding; stale editor submissions are rejected client- and server-side. Note inputs are strictly validated (alignment whitelist, #hex colour, bounded text length, control-char rejection, coordinate range clamped to effective background dimensions, font resolved through wm_note_font_id() which falls back to built-in font 3 on unreadable file, bad TTF signature or missing imagettftext). The 2x-density rendering path allocates both canvases before any logical-suppression state change and recovers cleanly on allocation failure; the overlap-filtered wm_sharp_notes() only affects bounded maps (<= 4M px) with plain TrueType label nodes. eval() appears only inside Browser test-harness files that load the plugin's real editor.js/form_submit code; prod code has none. All five checks pass (CodeQL, JS analyze, PHP 8.2/8.3/8.4 integration). Approving.