New label placement - #8
Merged
Merged
Conversation
Capture conflict-free reference scores (COPD orig+r1/r2/r3 + gallery, 13 configs) from the prototype pipeline as the regression anchor for packaging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
- Boost.Geometry rtree box-fit compiles via Rcpp+BH and is correct (~7.5s build). - Label injection point is R/mark_label.R my_place_labels (already a local fork). - polylabelr::poi confirmed for poles. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Port the six pipeline kernels, reorganised/renamed into per-stage files: geometry.h (BH rtree BoxFit + segcross/segbox), boxfit/assign/candidates/ refine/polish/foreign.cpp. hungarian, oneMoveSweep, twoMoveBnB, foreignLength verified bit-identical to the prototype; radialCandidates + forcePolish now use the exact polygon R-tree instead of the integral image (all cases still 0/0/0, within ~1% of golden). Package now compiles with LinkingTo BH. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
placeLabels() + prep glue (candidate pool + boundary-slot fallback, row/index maps, GEO geometry columns) + min-cost seed (.reorderBase) + isotonic .place1d. Pure given per-label box half-sizes; drives the six C++ kernels. Verified on 13 configs: all conflict-free, within 5% of golden reference. Golden used as a reference gate (not exact) per updated policy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Replace my_place_labels() (ggforce Minkowski placement) with the mascarade boundary-seed placer. Runs in makeContent (draw stage) in the panel's mm space, where box sizes are real text metrics and polygons are in mm; poles + box-fit R-tree are recomputed per draw (~20ms). Re-places on resize automatically. generateMask/fancyMask/mark_shape unchanged. Verified: example plot renders, placement box-overlap-free, proportionally equivalent to the prototype. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
- tests/testthat/helper-score.R: layoutScore (bb/ll/lb/len), scoreBetter, .buildTestGeom. - test-placement.R: conflict-free on example data, across box scales (view-change), single-label edge case, end-to-end fancyMask render. - Harden my_place_labels: clean non-finite vertices, guarantee >=3-pt rings, pole centroid fallback, anchor/pole fallback on failure (draw-stage polygons can be cropped/dropped/degenerate). K==1 guard in placeLabels. Full suite: 76 pass, 0 fail, 1 skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
.Rbuildignore excludes all dev scratch; NEWS 0.4.0 entry; version bump to 0.4.0. R CMD check Status: OK (76 tests pass). Boundary-seed label placer fully integrated across generateMask -> fancyMask -> geom_mark_shape draw stage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
- Delete simplify_outer() + its test (unreachable after the placement replacement). - placeLabels: name the do.call geometry args (bind by name, not position); drop the never-set hard_ll param (inline TRUE); name the radial/polish tuning constants. - my_place_labels: drop the ignored ghosts/simp_ratio params. - Deprecate+document the now-ignored public simp_ratio (fancyMask, geom_mark_shape). - Fix false 'built once / reused across draws' comments (geometry.h, boxfit.cpp, placeLabels.R): the box-fit is rebuilt per draw. De-archaeologise twoMoveBnB comment. - Add transitively-relied-on includes (<algorithm>, <utility>). - Wire the golden as a real no-worse regression gate on the example data (was unused). Tests: 68 pass, 0 fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
The R-tree box-fit and foreignLength walk every polygon edge, and mask rings have 50-800 vertices -> placement was ~6.8s on COPD. Restore simplify_outer and apply it (simp_ratio) to the polygons used for placement inside my_place_labels: COPD placement 6.8s -> 1.9s (3.7x), layout equal-or-better, still conflict-free (simplified ring encloses the original, so box-fit stays conservative). Drawn connectors still use the full-resolution polygons. simp_ratio un-deprecated and re-documented for its new role. Restore test-simplify_outer.R. 77 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
The placer optimizes each leader as box-edge -> cluster pole, but the drawing projected onto the nearest polygon-boundary point instead -- a different endpoint that could look inconsistent with the optimized layout. my_place_labels now returns the poles (attr 'poles'); my_make_label targets them, and straight() clips at the box. Added a note that supporting elbow leaders would require modelling the elbow geometry inside the placer (a future parameter). 77 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Root cause: my_make_label buffered all cluster polygons in a single polyoffset() call, which reorders (and can merge) its output. But my_place_labels pairs polygon i with rects[i]/anchors[i] and returns results indexed to line up with labels/con.gp, so the reordering silently mismatched each label's text/colour with a different cluster's placement and pole. The old projection-based leader hid this by re-projecting onto the correctly-indexed original polygon; targeting the pole (previous commit) exposed it. - Buffer each polygon individually so p_big stays 1:1 and in label order. - straight() now attaches the leader at the box exit of the centre->pole ray (matching .leaderEdges, exactly what the placer scores) instead of a corner. - fancyMask draws con.border='none' so the leader is a single straight segment (no borderline overhang / 'elbow'), consistent with the placement model. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
The min-cost seed placed its two fallback columns just inside the plot limits (xlim +/- max half-width), so at the default expansion wide labels were squeezed against or over the panel edge. Anchor them to the cluster cloud instead: left column at (min polygon x - column max half-width), right column mirror-image, so the boxes clear the clusters regardless of the plot limits. Columns may overflow the panel in x and y is never clamped (labels overflow rather than pile up). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Replaces the centre-exit leader with an anchor leader whose start point on the
label box is decoupled from the box centre, exposed via con.type:
cl corners + horizontal ledge (single sign-quadrant corner)
cm corners + edge-midpoints, no ledge (ggforce get_end_points rule: corner
only when the pole is fully to the side in x AND y)
none no connector
New con.padding = minimum visible leader length (distance outside the mask) +
the boundary-seed offset; NULL scales with label height.
- placeLabels: .anchorPoint + .firstHit; .geoCols sets ex,ey from the anchor;
the boundary seed hangs boxes off min/max(poly x) -/+ con.padding with a
bottom-corner (cl) / edge-midpoint anchor; radial candidates are filtered by
visible length >= con.padding; forcePolish (centre-exit only) is dropped.
- Leaders are drawn anchor -> first mask boundary (visible part), + ledge for cl.
- Removed the now-dead straight/elbow/with_borderline/get_end_points helpers and
the unused ggforce_points_to_path.R.
- Tests check cl and cm are conflict-free; golden regenerated for cl.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
- my_make_label now applies con.cap (gap at the cluster end of each leader); previously the parameter was silently ignored. - Remove the now-dead forcePolish C++ kernel (src/polish.cpp) and its exports, and the unused con_border/anchor_mod args from my_make_label. - Boundary-seed leader now starts at the box's true near edge (not the padded line) so the cl ledge meets it; still conflict-free (verified on example+pbmc, both styles) since column boxes are separated in y by gap > pad. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Restores the continuous force-directed polish dropped during the anchor-leader integration. The only change from the original kernel is the leader-start rule: polish.cpp's lead() now mirrors R's .anchorPoint() (con_type 0 = cl corner, else cm/none 8-point rule) instead of the centre-exit leader, so its hard conflict guard matches the drawn/scored geometry. placeLabels runs it after the two-move and re-derives the anchors from the polished centres; feasibility is preserved (the discrete solution is kept and only conflict-free moves are accepted). Cuts total leader length markedly (example cl golden 26.5 -> 19.8); still 0/0/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
con.padding (minimum visible leader length) is removed. The keep-out gap between labels and clusters is now the pre-existing label.buffer, which dilates each cluster polygon; fancyMask defaults it to 2 mm (was 0). Leaders are drawn to the TRUE cluster outline, not the dilated one: - my_place_labels takes both the true and dilated polygon sets. Box-fit keep-out (rtree) is built from the dilated set; poles, leader ends (.firstHit) and foreign-routing use the true set. Seed columns hang off the dilated extent. - Dropped the visibility filter and con.padding seed offset from placeLabels. Golden regenerated; cl/cm still conflict-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
…ective The optimizer's length objective now also penalizes how far a label box extends past the viewport (summed over x and y, 1:1 with distance), so the discrete stage prefers in-panel candidates over off-panel boundary-seed fallbacks -- cutting the label clipping on dense plots (e.g. COPD, 43 clusters). The foreignLength kernel is renamed effectiveLength and now returns the whole effective length (base leader length + foreign-cluster crossing + overflow) instead of just the foreign component; src/foreign.cpp -> src/effective.cpp, exports regenerated. Where a label has no feasible in-panel slot it can still overflow (preferred over a conflict). Golden unchanged (example fits in-bounds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
forcePolish previously rejected any move whose box left the viewport, which trapped off-panel boundary-seed labels: every intermediate step back toward the panel was still partly outside and got rejected. Drop the hard bounds check and add the same 1:1 viewport-overflow term to the energy. Now the pole distance and the overflow term both pull an off-panel label inward (the polish can walk it in step by step until a cluster/box blocks it), and a label may cross the edge only when doing so lowers total energy. Conflict-freeness is still hard-guarded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
radialCandidates required each box fully in-bounds, so a crowded wide label (e.g. T_senescence-associated against the T-cell blob) had no conflict-free in-panel slot and fell back to the distant boundary seed. Drop the in-bounds gate: candidates now only need to be cluster-free, and the effective-length overflow term ranks them, so such a label takes a minimally-clipped slot next to its own cluster instead of the far margin column. (xlo..yhi are now unused in the kernel but kept in the signature.) Golden unchanged (example fits in-bounds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Bump the overflow penalty from 1:1 to 3x in both the effective-length ranking (effective.cpp) and the polish energy (polish.cpp), pulling crowded edge labels further in-panel. Golden unchanged (example fits in-bounds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Measure viewport overflow against xhi - label.buffer (and xlo + label.buffer, etc.), so labels keep the same keep-out gap from the panel edge that they keep from clusters. Done by shrinking the xlim/ylim my_place_labels passes to placeLabels (those bounds now feed only the overflow terms), so no kernel change. Pulls near-edge labels inward; genuinely cluster-blocked labels (e.g. FCGR3A+ Mono, whose cluster hugs the right edge) still overflow and need limits.expand. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Same algorithm, reformatted: split the point-in-polygon and leader-inside-polygon math into named helper functions, one statement per line, a named OVERFLOW_WEIGHT constant, and clearer variable names. Behavior identical (golden unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
…tyle rule - lengthInsidePolygon (hand-rolled edge-split + midpoint point-in-polygon) is replaced by bg::intersection(leader, polygon) + bg::length -- the leader-inside- cluster arc via standard Boost.Geometry primitives. Cluster polygons are built once as bg polygons (bg::correct) with a cached bbox broad-phase reject. Behavior identical (golden unchanged). - OVERFLOW_WEIGHT moved to src/constants.h, included by effective.cpp and polish.cpp (was a literal 10.0 duplicated in both). - CLAUDE.md: production code should be readable/follow conventions; always brace if/for/while bodies and put the body on its own line (R and C++). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
twoMoveBnB assumed a conflict-free start: its length branch-and-bound prunes the
length-increasing tail, so a conflict resolvable only by a LONGER candidate was
never reached and input conflicts survived. Now, per label:
* conflict-free label -> unchanged length B&B (blocker partner + length bound);
* conflicted label -> drop the pruning and search all pairs (this label's c1
x every other label's c2, with the current mutual conflict subtracted from the
delta), driving input conflicts out. Only conflicted labels pay the extra cost.
Adds an artificial test: two labels overlap and the only fix is a longer, hence
previously length-pruned, candidate -- fails on the old kernel, passes now.
Example placement unaffected (conflict-free into two-move; golden unchanged).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
oneMoveSweep reformatted to match twoMoveBnB: braces on every if/for/while and the body on its own line. Cosmetic only -- identical tokens/logic (example golden and the two-move conflict test unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Readability pass (no behavior change): rename the cryptic locals -- bbf/lbf/llf/ conf -> boxesOverlap/leaderBoxConflict/leadersCross/anyConflict, lv -> lengthCost, cnt3 -> countConflicts, byl -> candidatesFor, ord -> order, the b/x/l conflict counts -> boxBox/leaderLeader/leaderBox, and the bBB/bXX/... best-move trackers -> bestDBox/bestDCross/... ; break the multi-clause predicate lambdas across lines, put one statement per line (incl. the best-move assignment), and keep lines <=100 cols. Example golden and the two-move conflict test unchanged; full suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Convert the comment doc-strings on the placeLabels.R helpers (.pavaDec, .place1d, .anchorPoint, .firstHit, .geoCols, .reorderBase, placeLabels) and the mark_label.R placement functions (simplify_outer, my_place_labels, my_make_label) to roxygen: title, @PARAM, @return, and @Keywords internal + @nord (internal, no man page). Cross-references to @nord helpers use code spans, not [foo()] auto-links, so roxygenise resolves cleanly. Documentation only -- example golden unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Add roxygen (//') doc-strings -- title, @PARAM, @return, @Keywords internal -- to every [[Rcpp::export]] kernel (buildBoxFit, hungarian, radialCandidates, oneMoveSweep, twoMoveBnB, effectiveLength, forcePolish); compileAttributes copies them into RcppExports.R. Remove @nord from the R placement internals so their docs (and the C++ ones) now generate real internal man pages -- letting R CMD check enforce documented arguments rather than suppressing it. checkDocFiles reports no undocumented arguments; full test suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Removes the significant duplication (TODO) between oneMoveSweep and twoMoveBnB: the box/leader conflict predicates (boxesOverlap, leaderBoxConflict, leadersCross, anyConflict) and the per-label countConflicts, previously copy-pasted as lambdas in both, now live once in a file-local CandidateSet struct. lengthCost stays local to twoMoveBnB (it alone squares the length). Pure extraction -- example golden and the two-move conflict test unchanged. Also clarifies the two-move doc-string: the partner is whatever label sits in the target slot; it need not itself be in conflict. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
- "box" places the label like "direct" (same anchor rule) and additionally outlines the label's bounding box at draw time (four edges drawn with the connector gp). Added to geom_mark_shape()/fancyMask() match.arg and the internal con_type paths; placement is identical to "direct" so only the draw-time grob building in mark_label.R changes. - Moved the per-value descriptions of con.type out of the @PARAM entry into a geom_mark_shape() @details section (ledge/direct/box/none); the @PARAM now just lists the options and points to Details. fancyMask() references it. - Test: leader-style loop now covers ledge/direct/box (title de-staled from the old cl/cm names). Full suite green (105/105); "box" renders via fancyMask and geom_mark_shape and draws the box edges (verified by grob inspection). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
labelboxGrob laid out a label box as top-margin / text / sep / description / bottom-margin. grobHeight(text) is the ascent (cap) box and excludes the descent, so for a label with no description the code parked a full font descent in the sep row to make room for descenders -- but on top of the bottom margin, so a label without descenders (most cluster names) got ~1mm of dead space underneath and looked bottom-heavy. Most visible now that con.type = "box" outlines the box. Reserve only the descent the bottom margin does not already cover (max(0, font_descent - bottom_margin)): with the default 2mm margin a no- descender label is symmetric (2mm top / 2mm bottom around the cap box), descenders still sit within the margin, and a zero-margin label still reserves the full descent so nothing clips. grobHeight equals the cap height exactly for the default font across devices (pdf/cairo/ragg/svglite), so the label grob's own box is already the cap-height box -- no separate box computation or text repositioning needed; con.type = "box" and placement just read grobHeight/grobWidth. Full suite green (105/105). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
The placer minimises viewport overflow but will spill a label box off-panel (where it gets clipped) when a crowded layout leaves no room. Detect that at draw time -- any label box extending beyond the panel bounds -- and emit a cli warning suggesting the two fixes the user controls: a smaller label.fontsize, or wider plot limits (e.g. ggplot2::expansion() on a scale). Fires only on real overflow (verified: no warning on the normal example renders; fires with correct singular/plural wording when labels are forced off-panel). Full suite green (105/105). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFyDJ2r7jeLmUuZgQKejon
Rename the "direct" leader/label-mark style to "line" throughout the geom, placement kernels, and documentation. Also refresh the geom_mark_shape examples (add a description example) and give the geom a `description` default aesthetic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwyxAC7kSfr21WExAnHQE
Introduce `balance_wrap()`, a soft-target minimum-raggedness word wrapper, and expose it as a new `label.maxwidth` parameter on `geom_mark_shape()` and `fancyMask()`. When `label.width` (a strict fixed width) is unset, the label and description are wrapped to this soft target so line widths are even and close to it, avoiding a short dangling final line and orphaned words. A line may slightly exceed the target to prevent an orphan, and an over-long single word is never broken (the effective target grows to the widest word). The target also lowers the effective `label.minwidth` so the box never exceeds it. Break points match `wrap_text()` (spaces and after-hyphens, never before). fancyMask uses a positive line height when wrapping is active so multi-line labels do not overlap. Adds tests for the wrapper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwyxAC7kSfr21WExAnHQE
Follow-up to the label.maxwidth review: - balance_wrap() now trims the single-token / empty early return, matching wrap_text() so a label with surrounding whitespace is not mis-sized. - Replace three idiomatic inline if/else expressions with braced statements (per CLAUDE.md brace style) in balance_wrap() and fancyMask(). No behavioural change beyond the trim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwyxAC7kSfr21WExAnHQE
Merge the strict-width and soft-target code paths in labelboxGrob() into a single `width` argument that behaves as the old `max.width` did: balance_wrap the text to a soft target, lower the effective min.width, and size the box to the wrapped content rather than to a fixed dimension. Drop the now-unused greedy `wrap_text()`. Remove `label.maxwidth` from the geom and fancyMask() entirely, keeping `label.width` as the sole public parameter (breaking rename), and override the inherited ggforce doc that still described it as a strict fixed width. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
Extract three named internal helpers from my_place_labels() -- prepPolygons() (ring cleaning + simplification), polesOfInaccessibility() (pole solve with centroid fallback), and warnOnOverflow() (off-panel spill warning) -- and rewrite the body with descriptive names, one statement per line, braces on every branch, and no <<- closure mutation. Behavior is unchanged (verified by the geom_mark_shape draw-path tests and a matching render). Fix the "did not full fit" -> "did not fully fit" typo in the overflow warning, and document the two 1e-3 (1 um) constants: the non-collinear seed triangle for degenerate rings and the floating-point tolerance on the overflow check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
Extract five named internal helpers from my_make_label() -- expandSinglePoints(),
computeAnchors(), ringArea(), dilatePolygons(), and buildConnectorGrob() -- and
rewrite the body as a short top-to-bottom pipeline with descriptive names, one
statement per line, and braces on every branch. The dense connector loop (parallel
xs/ys/gi arrays with a manual counter) becomes a per-label segmentsFor() that
returns {x, y, label} segments flattened into one polylineGrob. Behavior is
unchanged (verified by the geom_mark_shape draw-path tests and matching renders
across ledge/line/box connector styles).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
Detect degenerate cluster rings -- a point, a line, or a >=3-point ring enclosing zero area (collinear / repeated-vertex slivers), including NA-cropped rings -- in makeContent.shape_enc() via the new degenerateRing(), and drop them entirely: no outline, no label, with a cli warning. generateMask() never emits these, but a real cluster can collapse this way after axis-limit cropping or a negative expand. Fold the drop into the existing ggforce-drop prune so a single keep-set is applied once to every positionally-indexed structure (polygons, labels, dims, anchors, mark rows via the new pruneMark(), mark$gp, con.gp), which is where colour-alignment bugs lived before. Bind `surviving` to sorted id order so polygons[[k]] stays aligned with surviving[k]. The prune stays guarded, so the happy path is untouched. This removes the need to expand single-point clusters into jittered clouds downstream: delete expandSinglePoints() and rehome its stats::runif import to generateMask.R (its other user). prepPolygons()'s eps-triangle guard is downgraded to a defensive net. Adds a unit test for degenerateRing and an integration test asserting a dropped cluster leaves survivor colours aligned. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
…lots Convert bare noun-phrase roxygen titles to action titles across the label-placement helpers (e.g. computeAnchors, ringArea, polesOfInaccessibility, my_place_labels, and the placeLabels.R sweeps / polish / layout accessors), and trim a few over-detailed bodies. Rename the two slot generators for a consistent vocabulary: .seedSlots -> .sideSlots and .seedColumn -> .sideColumn (the left/right boundary columns), and .radialPlacements -> .radialSlots. seedLayout() stays -- it now reads as the initial layout built from side slots, pairing with addRadialCandidates() adding radial slots. Also reword the .sideSlots doc: these are the starting placement, not a "fallback". All three are internal (@nord); no API, test, or NAMESPACE impact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
…ract
The label-placement layer used "ring" and "polygon" interchangeably for the same
single-loop list(x, y) object. Standardize on "polygon" (matching the dominant
identifier vocabulary and the public generateMask() "polygon table" API): rename
ringArea -> polygonArea and degenerateRing -> degeneratePolygon, and switch the
prose/comments in mark_label.R, placeLabels.R, and mark_shape.R to "polygon".
Add a boundary note in my_place_labels() recording the fact this glosses over: each
input is a single ring; any mask holes are resolved upstream in generateMask(). The
generated RcppExports.R / src C++ contract ("one ring per cluster") is intentionally
left as-is.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
Now that makeContent.shape_enc() drops degenerate polygons up front (degeneratePolygon()), the downstream defensive handling for them is dead. Turn it into assertions that fail loudly if the invariant is ever violated: - prepPolygons(): the eps-triangle rebuilt for sub-3-vertex polygons -> stopifnot( length(x) >= 3). The non-finite filter stays (a valid polygon can still carry NA vertices from axis cropping). - my_place_labels(): the poles/bounds finiteness half of the anchor-fallback trigger -> stopifnot(all(is.finite(poles)), all(is.finite(bounds))). Poles are finite by construction and a panel's npc->mm size is always finite; only a collapsed (zero-size) panel stays a real guard. The assert also keeps the `<= 0` comparison NA-safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
When placement cannot run -- a zero-size panel or the C++ solver failing -- my_place_labels() now warns once and draws no labels (skipPlacement) instead of dropping every label onto a computed "anchor" point that is invisible anyway. That retires the entire anchor machinery, whose only consumer was that fallback: - Delete placeOnAnchors(), computeAnchors(), the `anchors` parameter, and the anchor_x/anchor_y plumbing through my_make_label() and makeContent.shape_enc(). - Remove the x0/y0 aesthetics and the anchor.x/anchor.y grob parameters from geom_mark_shape / shapeEncGrob, plus the `data$xmin <- data$x0` transform hack. These were package-local additions (not ggforce), shipped in v0.3.5; real placement uses poles of inaccessibility, never anchors. NEWS entry to follow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
…ductions label.buffer is a padding, so it must be non-negative -- assert delta >= 0 in my_make_label() before dilating. With that guaranteed, dilatePolygons() can drop its empty-offset and split-offset branches: dilating a single connected polygon by a non-negative offset always yields exactly one piece, now enforced by stopifnot(length(dilated) == 1). Remove the poi guards in polesOfInaccessibility() -- the tryCatch and centroid fallback. Polygons reaching it are non-degenerate, so poi() is expected to succeed and return a finite point; if it doesn't that's a bug and should surface (a non-finite result is still caught by the caller's stopifnot(all(is.finite(poles)))). Mark the degenerate-coordinate reductions in makeContent.shape_enc() as LEGACY (inherited from ggforce's mark_hull): they are largely redundant now that degeneratePolygon() drops such parts, but are kept because they also feed the drawn mark and predate the degeneracy drop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
label.width (soft wrap cap) and label.minwidth (box-width floor) are orthogonal bounds. The `min.width <- min(min.width, width)` line only existed to stop ggforce's 50mm default min.width from bloating boxes when a small width is set. Default label.minwidth to 0 instead -- which fancyMask already forced everywhere -- so the knobs no longer interact: remove the hack, and drop the now-redundant label.minwidth = 0 overrides in fancyMask. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYxn2tp7jUnQnYfbMrLhjQ
The NA form is soft-deprecated in favor of the explicit expect_no_error(); bump the testthat minimum to 3.1.5, which introduced it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHu5JKSSK5G9sCN62LXLdJ
Drop the K==1 short-circuit in my_place_labels() that dropped a lone label on its cluster's pole with an invisible leader. A single cluster now flows through placeLabels() like every other case, producing an external label with a real leader. The pipeline already handled K==1 (it is per-label throughout), and test-placement.R already asserted this behavior via a direct placeLabels() call. Also refresh now-stale comments: the zero-length-leader note cited the removed single-label / anchor fallbacks, and the placeLabels docstring framed K==0/K==1 as special cases rather than the natural empty/one-row result of the one-row-per-cluster contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012sTksGqUQMWYD7ddMrxwGR
It asserted conflict-free placement across three char_frac values, but on the example data every scale takes the identical path: the Hungarian seed branch, a conflict-free seed, and the sweeps as no-ops. It flipped no size-dependent branch (the crowded packLen path needs capacity < m, which never happens here; the conflicted two-move branch is covered by the synthetic kernel test), so it added no coverage over the default-scale test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012sTksGqUQMWYD7ddMrxwGR
The constant had only one user (effectiveLengthImpl in effective.h), so a separate header just added an include. Move the definition next to its use and remove constants.h. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012sTksGqUQMWYD7ddMrxwGR
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.