Base/adam ef57e1f6#3
Open
giardiello wants to merge 12 commits into
Open
Conversation
fallback to look for the label in the framing_intent if the label in the framing_intent is also not set, fall back and display the id of the framing_decision as there needs to be something to select
Hotfix viewer label
Contexts in the FDL spec have no id field and label is optional. The viewer was using context labels as identifiers throughout the selection pipeline, which broke when a context had an empty label. The underlying C++ core API already identifies contexts by array index via fdl_doc_context_at(). Aligned the Python layer to match by using index-based selection across the viewer, imaging, and frameline generator packages. Labels are still used for display in dropdowns with the array index as fallback when empty.
Changed public API functions and get_fdl_components to accept context_index (int) instead of context_id (str). Labels are resolved to indices at test boundaries.
The settings class is named Settings not AppSettings. This caused the export image button to silently fail.
…_display Ensure contexts are treated via index across the python layer
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
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
Cross-binding work to make `round` optional in canvas-template creation,
defaulting to FDL spec §7.4.12 `{EVEN, UP}`. Changes the default mode
from ROUND to UP, and regenerates golden scenario fixtures accordingly.
Facade/API:
- fdl_api.yaml: give `round` param default `RoundStrategy()`
- Python: accept `round=None` in FDL.add_canvas_template / CanvasTemplate
- Node: `round: RoundStrategy = new RoundStrategy()` in fdl.ts
- C++: default-arg `{FDL_ROUNDING_EVEN_EVEN, FDL_ROUNDING_MODE_UP}` on
add_canvas_template, from_framing_intent, populate_from_intent
- Python FramingDecision ctor: move DimensionsFloat/PointFloat mutable
defaults to None-sentinels
Codegen:
- adapters.py: flip RoundStrategy aggregate literal EVEN,ROUND -> EVEN,UP
- python_context.py: new _resolve_python_default helper — constructor-call
defaults emit `foo=None` in signature + `if foo is None: foo = Ctor()`
unwrap in the body (avoid mutable default args)
- class.py.j2: render `unwrap_expr` blocks for init/builders/methods
- cpp_gen.py: plumb `global_fallback` through all param builders; inline
`{EVEN, UP}` as default for nullable round_strategy params
- node_gen.py: when a default is a `new X()` constructor call, import
the class as a value (not just a type) so the default compiles
- rounding.ts.j2: doc the new spec default
Test/infra:
- node scenarios.test.ts: tolerance-aware deep compare for nested
custom_attrs objects (mirror Python's __eq__ semantics)
- python testing/base.py: `generate_result` now reads FDL_REGEN_FIXTURES
env var, so refreshing golden .fdls doesn't require editing code
- binding.cc: expose fdl_canvas_template_has_round, update comments
- regenerated ~25 Scenarios_For_Implementers/*/Results/*.fdl fixtures
under the new {EVEN, UP} default
- uv.lock refresh
Snapshot commit for rollback — not a final, reviewed change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrite geometry_round as a 5-step algorithm: 1. Ceil effective_dims; absorb ceil delta into effective_anchor. 2. Round canvas_dims per strategy.even / strategy.mode. 3. Clamp effective / protection / framing against rounded canvas. 4. Shift all anchors by +canvas_delta/2. 5. Clamp anchors to [0, canvas - dim]. This fixes eight scenarios that previously emitted invalid FDL because canvas rounded DOWN while inner float dims (protection, framing) stayed above the rounded integer, violating the schema hierarchy invariant. Under the new rule inner dims <= effective_float by construction, so ceil(effective_float) >= inner automatically; canvas-clamping inner in step 3 covers the max_dims case where canvas is forced smaller than ceil(effective_float). strategy.even / strategy.mode no longer apply to effective_dims (pure ceil, may be odd when even=EVEN). Canvas keeps strategy-respecting rounding. Also clarify scaled_bounding_box and content_translation are unrounded floats by design (captured post-scale, pre-round). - native/core/src/fdl_geometry.cpp: new geometry_round. - native/core/src/fdl_template.cpp, fdl_constants.h: docstrings. - native/core/tests/vectors/geometry/: _round_like_cpp + golden vectors refreshed. - resources/FDL/Scenarios_For_Implementers/**/Results/*.fdl: 14 fixtures regenerated (8 previously-invalid + 6 where ceil replaces EVEN round). - native/bindings/python/tests/test_optional_rounding.py: assert EVEN strategy on canvas_dims instead of effective_dims.
Refines the round-pipeline step 5 so the integer effective dim adapts
to fit inside (canvas - anchor) instead of pulling the anchor back to
fit (canvas - dim). Anchor truthfulness is prioritised.
5a) Lower clamp: negative anchors -> 0 (unchanged behaviour, just
separated from the upper bound).
5b) Upper bound: shrink dim if anchor + dim overflows canvas.
- effective is an integer schema field, so use
floor(canvas - anchor) -> strict invariant
anchor + dim <= canvas always holds.
- protection / framing remain float; use raw
canvas - anchor.
5c) Re-establish hierarchy: the floor on effective in 5b can drop
it below previously-valid float inner dims. Clamp protection /
framing down to the (post-floor) effective, but only when they
actually exceed it (preserves the 0 = "unset" sentinel for
optional protection).
Mirrors the same algorithm in the Python golden vector generator and
refreshes the one template-vector expectation that lands in the upper
bound (right_bottom_alignment).
Empirical impact: 3 scenario fixtures change (Scen17 A/B/C, the
pad_to_max + bottom-aligned cases). In those, effective_anchor.y
preserves the float position computed by the pipeline, the integer
effective.height is one pixel smaller (floor of canvas - anchor), and
framing.height is clamped to match. All other 50 scenarios are
byte-identical with the previous commit.
Test status:
- C++ ctest: 134/134.
- Python native bindings: 457 pass, 4 skip.
- Node bindings: 325 pass.
- fdl_imaging FDL parity: passes; image-comparison failures remain
deferred (tracked separately).
OPEN DISCUSSION - Scen17 logic still under exploration:
1. Trade-off in step 5b/5c. The current choice prioritises anchor
truthfulness (anchor preserved exactly as the pipeline computed
it, sub-pixel and all) over geometric fidelity (the previous
anchor-clamp produced an exact-fit edge but moved the anchor by
up to delta/2). The 1-pixel effective shrink and the
sub-floor-pixel gap at the canvas edge are visible in the new
fixtures. Whether this is the right call vs. shifting the
anchor (committed previously) is still open.
2. Single-round-at-end exploration. The pipeline is already mostly
all-float internally; geometry_round is the only quantization
point. Question on the table is whether step 1's ceil and step
5b's floor-shrink should collapse into a single rule -- e.g.
skip the step-1 ceil entirely and just do
effective = floor(canvas - effective_anchor)
at the end -- so that the integer effective is computed once
from the float anchor + canvas, with no symmetric-delta
bookkeeping in the middle. That would eliminate the
"step 1 inflates, step 5 shrinks" tension entirely. Not
attempted yet; needs design + corpus diff before committing.
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
2 participants
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.