Feature/optional rounding v2#50
Open
giardiello wants to merge 2 commits intoascmitc:devfrom
Open
Conversation
Align FDL rounding with spec 7.4.12 by treating the rounding strategy as
applicable only to canvas.dimensions (and the integer-typed
canvas.effective_dimensions for schema consistency). Inner geometry
(protection, framing dims/anchors) stays floating-point so sub-pixel
precision is not lost.
- Remove the NONE sentinel entirely; an omitted "round" field now
returns the spec default {EVEN, UP} directly (no magic third mode).
- Skip round when maximum_dimensions + pad_to_maximum (per spec 7.4.12).
- After crop, re-round effective_dims with the template strategy,
enforce hierarchy (effective >= ceil(max inner dims)), and apply
half-delta compensation to effective_anchor so the rounded effective
area stays centered on the original float area, clamped to canvas.
- Surface fdl_canvas_template_has_round in C ABI and bindings.
- Regenerate 52 scenario golden FDLs and C++ template vector affected
by the new anchor compensation and inner-float geometry.
Tests: 134/134 C++, 457 Python (4 skipped).
Made-with: Cursor
giardiello
commented
Apr 23, 2026
| alignment_method_horizontal: HAlign = HAlign.CENTER, | ||
| alignment_method_vertical: VAlign = VAlign.CENTER, | ||
| round: RoundStrategy = RoundStrategy(), | ||
| round: RoundStrategy | None = None, |
Author
There was a problem hiding this comment.
This needs to be added to the autogeneration for the bingings rather than being hand written
giardiello
commented
Apr 23, 2026
| alignment_method_horizontal: HAlign, | ||
| alignment_method_vertical: VAlign, | ||
| round: RoundStrategy, | ||
| round: RoundStrategy | None = None, |
Author
There was a problem hiding this comment.
This also needs to not be hand written and needs adding to the autogeneration
giardiello
commented
Apr 23, 2026
| // rounded effective area within the canvas: x in [0, canvas_w - eff_w]. | ||
| double const delta_w = geo.effective_dims.width - float_eff.width; | ||
| double const delta_h = geo.effective_dims.height - float_eff.height; | ||
| double const max_x = std::max(0.0, geo.canvas_dims.width - geo.effective_dims.width); |
Author
There was a problem hiding this comment.
Need to double check the clamping to canvas and ensure this is actually needed
Consolidate all schema-integer rounding and anchor compensation into a
single step at the end of the CanvasTemplate apply pipeline (post-crop),
replacing the prior two-pass design (Phase 5 round + Phase 9b
effective-only re-round). This removes redundant intermediate
quantization, fixes a latent asymmetry where canvas rounding was not
absorbed into anchor positions, and makes the behavior easier to reason
about end to end.
fdl_geometry_round now does everything the schema-integer step requires:
1. Round canvas_dims; shift ALL anchors by +canvas_delta/2 to keep
content centered within the rounded canvas. Previously the
canvas rounding delta was discarded, introducing a small
directional bias whenever canvas_dims wasn't already integer.
2. Round effective_dims; enforce hierarchy (effective >= ceil(max
inner dims), effective <= canvas).
3. Shift effective_anchor by -effective_delta/2 to keep the rounded
effective rectangle centered on its pre-round extent.
4. Clamp all anchors to [0, canvas - dim] so symmetric absorption
degrades gracefully to one-sided at canvas edges.
The skip_round gate for pad_to_maximum + maximum_dimensions is no
longer needed: when that branch is active, canvas_dims is set from
max_dims (already integer) before the final round, so its rounding
delta is zero and no anchor shift occurs on that axis — matching spec
7.4.12's "round has no effect" semantics automatically. Inner
effective_dims is still integerized as the schema requires.
geometry_round_effective_post_crop is removed from the internal
header, internal implementation, and the C ABI (wrapper +
fdl_core.h declaration). It was only ever called from fdl_template.cpp
and was not exposed in Python or Node bindings.
Test vectors updated:
- Geometry round vectors: all 3 cases now reflect canvas_delta/2
shifts on all anchors plus -effective_delta/2 on effective_anchor;
inner (protection, framing) dims and anchors stay float.
- Template vectors: all 10 cases regenerated. Notable knock-on
changes now exposed: scaled_bounding_box reports the true float
pre-round canvas (previously the post-Phase-5-round integer), and
output_effective_dims is now consistent with output_canvas_dims
rounding (the old JSON had an effective_dims value that the old
pipeline wouldn't actually produce).
- generate_geometry_vectors.py: adds an inline _round_like_cpp helper
that reproduces the new C++ semantics so regenerations stay in sync
without requiring matching changes to the Python reference
implementation.
Python and Node bindings are intentionally untouched; they are
autogenerated from the C++ core and will pick up the new semantics
when regenerated.
Tests: 134/134 C++.
Made-with: Cursor
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.