Fix: text label handling renewed to avoid floating geometry#127
Conversation
|
While I was working on the issue with the floating text, I came up with this solution. It differs from the original acceptance criteria in #121, but I found that this approach is the simplest and most intuitive for the user experience. What do you think @jasonmadigan ? |
There was a problem hiding this comment.
thanks for digging into this!
concern is scope: #121 only needs "skip labels in disabled cells". the helpers you already wrote, _label_layout_cell and _label_in_enabled_cell, are essentially that fix, but nothing in generate_bin calls them yet. wiring those into label filtering + the tests would close #121 in ~30 lines.
the full rewrite changes behaviour for all labels, and as-is breaks recessed ones: the clip slab extrudes upward from floor_z while the recessed cutter extrudes downward, so their intersection is a 0.01mm sliver (measured 0.16mm3 vs 8.13mm3 on main). tests pass because they only assert non-empty + wall ratio.
suggestion: slim this PR to the disabled-cell exclusion check (your existing helpers + tests), and file the per-surface rewrite as its own issue where the recessed handling and label-splitting design can be worked through properly. i'd be glad to see that as a follow-up.
| return ix, grid_y - 1 - iy_ui | ||
|
|
||
|
|
||
| def _label_in_enabled_cell(config: GenerateRequest, x_mm: float, y_mm: float) -> bool: |
There was a problem hiding this comment.
these two helpers are the core of what #121 needs, but nothing in generate_bin calls them. wiring _label_in_enabled_cell into the label loop as a skip check (plus the existing tests for it) is roughly the whole fix for this issue.
jasonmadigan
left a comment
jasonmadigan
left a comment
There was a problem hiding this comment.
Thanks @gr3enk, nice fix. Verified the disabled-cell skip works and the suite is green. There's a fractional grid_y edge in the label row indexing we'll pick up in a follow-up, no need for another round here. Merging.
2 participants
This PR changes how text labels are handled so that floating geometry no longer occurs. With this change, text labels are now placed/clipped on the lowest level. This applies to both tool cutouts and disabled cells which are connected via a base plate.
closes #121