Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/FDL_Apply_Template_Logic.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,14 @@ The scale factor is then determined by the `fit_method`:

Apply the scale factor to **all** dimensions and anchors uniformly.

> **Precision note**: The scale factor is kept internally as a ratio
> (numerator / denominator) to avoid IEEE 754 precision loss. The computation
> uses `(value x numerator) / denominator` instead of `value x (num / den)`.

**Normalize and scale** (per value):

```
scaled_value = (source_value * source_squeeze * scale_factor) / target_squeeze
scaled_value = (source_value * source_squeeze * scale_numerator) / (scale_denominator * target_squeeze)
```

This converts from source pixel space through square-pixel space to target
Expand Down
17 changes: 12 additions & 5 deletions docs/FDL_Template_Implementer_Guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,13 @@ calculate_scale_factor(fit_norm, target_norm, fit_method):

Apply the scale factor to **all** dimensions and anchors uniformly, then round.

> **Precision note**: The scale factor `target / fit` may not be exactly
> representable in IEEE 754 (e.g., `3840/6560 = 24/41`). To avoid rounding
> errors, the implementation keeps the scale factor as a ratio
> (numerator / denominator) and computes `(value x numerator) / denominator`
> instead of `value x (numerator / denominator)`. This ensures intermediate
> products are exact for integer inputs that fit within 2^53.

```
geometry = fdl_geometry_normalize_and_scale(geometry,
source_squeeze = input_squeeze,
Expand All @@ -477,16 +484,16 @@ geometry = fdl_geometry_round(geometry, round_strategy)

**The Normalize-and-Scale Formula** (per ASC FDL spec 7.4.5):

For dimensions:
For dimensions (internally computed as ratio for precision):
```
width_out = (width_in x source_squeeze x scale_factor) / target_squeeze
height_out = height_in x scale_factor
width_out = (width_in x source_squeeze x scale_numerator) / (scale_denominator x target_squeeze)
height_out = (height_in x scale_numerator) / scale_denominator
```

For anchor points:
```
x_out = (x_in x source_squeeze x scale_factor) / target_squeeze
y_out = y_in x scale_factor
x_out = (x_in x source_squeeze x scale_numerator) / (scale_denominator x target_squeeze)
y_out = (y_in x scale_numerator) / scale_denominator
```

**Rounding Configuration** (`RoundStrategy`):
Expand Down
84 changes: 84 additions & 0 deletions native/bindings/python/tests/test_template_width_rounding.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# SPDX-FileCopyrightText: 2024-present American Society Of Cinematographers
# SPDX-License-Identifier: Apache-2.0
"""
Regression test for floating-point precision bug in template application.

Source: 6560x2747 framing, 6560x3100 canvas
Template: 3840x1608 target, fit_method=width, fit_source=framing_decision.dimensions
Round: even=even, mode=down

The scale factor 3840/6560 = 24/41 has no exact IEEE 754 binary representation.
Computing value * (target/fit) produces 3839.999... which with round(even=even,
mode=down) yields 3838 instead of the correct 3840.
"""

import uuid
from pathlib import Path

import pytest

from fdl import read_from_file

RESOURCES = Path(__file__).parents[4] / "resources" / "FDL"
FDL_PATH = RESOURCES / "Simple_FDL2.0.1_Failure_Example.fdl"


@pytest.fixture()
def template_result():
"""Load the failing FDL and apply the template, returning the result."""
fdl = read_from_file(FDL_PATH)

template = None
for t in fdl.canvas_templates:
if t.label == "Custom Template":
template = t
break
assert template is not None, "Template 'Custom Template' not found"

context = None
for c in fdl.contexts:
if c.label == "Custom Template":
context = c
break
assert context is not None, "Context 'Custom Template' not found"

canvas = None
for cv in context.canvases:
if cv.id == "1":
canvas = cv
break
assert canvas is not None, "Canvas '1' not found"

fd = None
for f in canvas.framing_decisions:
if f.framing_intent_id == "A":
fd = f
break
assert fd is not None, "Framing decision with intent_id 'A' not found"

deterministic_uuid = uuid.UUID("12345678-1234-5678-1234-567812345678")
new_canvas_id = deterministic_uuid.hex[:30]

result = template.apply(
source_canvas=canvas,
source_framing=fd,
new_canvas_id=new_canvas_id,
new_fd_name="",
source_context_label=context.label,
context_creator="Test",
)
return result


def test_output_canvas_dimensions(template_result):
"""Output canvas width must be 3840 (not 3838) and height 1608."""
canvas = template_result.canvas
assert canvas.dimensions.width == 3840, f"Expected canvas width 3840, got {canvas.dimensions.width}"
assert canvas.dimensions.height == 1608, f"Expected canvas height 1608, got {canvas.dimensions.height}"


def test_output_framing_decision_dimensions(template_result):
"""Output framing decision width must be 3840 (not 3838) and height 1608."""
fd = template_result.framing_decision
assert fd.dimensions.width == 3840, f"Expected framing width 3840, got {fd.dimensions.width}"
assert fd.dimensions.height == 1608, f"Expected framing height 1608, got {fd.dimensions.height}"
8 changes: 8 additions & 0 deletions native/core/include/fdl/fdl_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,9 @@ fdl_dimensions_scale(fdl_dimensions_f64_t dims, double scale_factor, double targ
* Normalize and scale in one step.
*
* Equivalent to fdl_dimensions_scale(fdl_dimensions_normalize(dims, input_squeeze), ...).
* When higher precision is needed (e.g., template application), use the internal
* ratio-based variant which keeps numerator/denominator separate to avoid
* IEEE 754 rounding errors in the scale factor.
*
* @param dims Dimensions to transform.
* @param input_squeeze Source anamorphic squeeze factor.
Expand Down Expand Up @@ -448,6 +451,9 @@ FDL_API int fdl_point_is_zero(fdl_point_f64_t point);
/**
* Normalize and scale a point in one step.
*
* When higher precision is needed (e.g., template application), use the internal
* ratio-based variant which keeps numerator/denominator separate.
*
* @param point Point to transform.
* @param input_squeeze Source anamorphic squeeze factor.
* @param scale_factor Scale multiplier.
Expand Down Expand Up @@ -523,6 +529,8 @@ FDL_API fdl_geometry_t fdl_geometry_fill_hierarchy_gaps(fdl_geometry_t geo, fdl_
* Normalize and scale all 7 fields of the geometry.
*
* Applies anamorphic normalization and scaling to all dimension and anchor fields.
* Internally uses ratio-based arithmetic (denominator=1.0) to preserve precision.
* For template application, use the internal ratio variant directly.
*
* @param geo Geometry to transform.
* @param source_squeeze Source anamorphic squeeze factor.
Expand Down
3 changes: 2 additions & 1 deletion native/core/src/fdl_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ constexpr double kCenterDivisor = 2.0;

/** @name Identity / default values
* @{ */
constexpr double kIdentitySqueeze = 1.0; /**< No anamorphic distortion (1:1 squeeze). */
constexpr double kIdentitySqueeze = 1.0; /**< No anamorphic distortion (1:1 squeeze). */
constexpr double kIdentityDenominator = 1.0; /**< Identity denominator for ratio-based scaling (num / 1.0 = num). */
/** @} */

/** @name Banker's rounding constants
Expand Down
53 changes: 35 additions & 18 deletions native/core/src/fdl_geometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
*
* - **Gap-filling**: Propagates populated dimensions upward to fill missing layers.
* Protection is never auto-filled from framing (by spec).
* - **Normalize+scale**: Applies anamorphic correction and uniform scaling.
* - **Normalize+scale**: Applies anamorphic correction and uniform scaling using
* ratio-based arithmetic (multiply before divide) to preserve precision for
* scale factors that are not exactly representable in IEEE 754.
* - **Round**: Rounds all 7 fields (4 dimensions + 3 anchors) per strategy.
* - **Offset**: Translates anchors for alignment, tracking theoretical positions.
* - **Crop**: Clips dimensions to visible portion within canvas bounds,
Expand All @@ -27,16 +29,20 @@ namespace fdl::detail {
namespace {

/**
* @brief Normalize a point for squeeze then apply uniform scale.
* @param pt Input point.
* @param squeeze Source anamorphic squeeze factor.
* @param scale Uniform scale factor.
* @param target_squeeze Target anamorphic squeeze factor.
* @return The normalized and scaled point.
* @brief Normalize and scale dimensions using ratio (num/den) for precision.
*/
fdl_point_f64_t point_normalize_and_scale(fdl_point_f64_t pt, double squeeze, double scale, double target_squeeze) {
fdl_dimensions_f64_t dims_normalize_and_scale_ratio(
fdl_dimensions_f64_t dims, double input_squeeze, double num, double den, double target_squeeze) {
return {(dims.width * input_squeeze * num) / (den * target_squeeze), (dims.height * num) / den};
}

/**
* @brief Normalize and scale a point using ratio (num/den) for precision.
*/
fdl_point_f64_t point_normalize_and_scale_ratio(
fdl_point_f64_t pt, double squeeze, double num, double den, double target_squeeze) {
fdl_point_f64_t const normalized = fdl_point_normalize(pt, squeeze);
return fdl_point_scale(normalized, scale, target_squeeze);
return {(normalized.x * num) / (den * target_squeeze), (normalized.y * num) / den};
}

/**
Expand Down Expand Up @@ -118,17 +124,28 @@ fdl_geometry_t geometry_fill_hierarchy_gaps(fdl_geometry_t geo, fdl_point_f64_t
};
}

fdl_geometry_t geometry_normalize_and_scale(
fdl_geometry_t geo, double source_squeeze, double scale_factor, double target_squeeze) {
fdl_geometry_t geometry_normalize_and_scale_ratio(
fdl_geometry_t geo,
double source_squeeze,
double scale_numerator,
double scale_denominator,
double target_squeeze) {

return {
fdl_dimensions_normalize_and_scale(geo.canvas_dims, source_squeeze, scale_factor, target_squeeze),
fdl_dimensions_normalize_and_scale(geo.effective_dims, source_squeeze, scale_factor, target_squeeze),
fdl_dimensions_normalize_and_scale(geo.protection_dims, source_squeeze, scale_factor, target_squeeze),
fdl_dimensions_normalize_and_scale(geo.framing_dims, source_squeeze, scale_factor, target_squeeze),
point_normalize_and_scale(geo.effective_anchor, source_squeeze, scale_factor, target_squeeze),
point_normalize_and_scale(geo.protection_anchor, source_squeeze, scale_factor, target_squeeze),
point_normalize_and_scale(geo.framing_anchor, source_squeeze, scale_factor, target_squeeze),
dims_normalize_and_scale_ratio(
geo.canvas_dims, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
dims_normalize_and_scale_ratio(
geo.effective_dims, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
dims_normalize_and_scale_ratio(
geo.protection_dims, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
dims_normalize_and_scale_ratio(
geo.framing_dims, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
point_normalize_and_scale_ratio(
geo.effective_anchor, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
point_normalize_and_scale_ratio(
geo.protection_anchor, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
point_normalize_and_scale_ratio(
geo.framing_anchor, source_squeeze, scale_numerator, scale_denominator, target_squeeze),
};
}

Expand Down
23 changes: 14 additions & 9 deletions native/core/src/fdl_geometry.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* @file fdl_geometry.h
* @brief Internal geometry operations on the 4-layer FDL dimension hierarchy.
*
* Provides gap-filling, normalize+scale, rounding, offset, and cropping
* operations used by the template application pipeline.
* Provides gap-filling, ratio-based normalize+scale, rounding, offset, and
* cropping operations used by the template application pipeline.
*/
#ifndef FDL_GEOMETRY_INTERNAL_H
#define FDL_GEOMETRY_INTERNAL_H
Expand All @@ -27,15 +27,20 @@ namespace fdl::detail {
fdl_geometry_t geometry_fill_hierarchy_gaps(fdl_geometry_t geo, fdl_point_f64_t anchor_offset);

/**
* @brief Normalize and scale all 7 fields (4 dimensions + 3 anchors) of the geometry.
* @param geo Input geometry.
* @param source_squeeze Source anamorphic squeeze.
* @param scale_factor Uniform scale factor to apply.
* @param target_squeeze Target anamorphic squeeze.
* @brief Normalize and scale using a ratio (numerator/denominator) for precision.
*
* Computes (value * numerator) / denominator instead of value * (num/den) to
* preserve precision for integer inputs.
*
* @param geo Input geometry.
* @param source_squeeze Source anamorphic squeeze.
* @param scale_numerator Numerator of the scale ratio.
* @param scale_denominator Denominator of the scale ratio.
* @param target_squeeze Target anamorphic squeeze.
* @return Normalized and scaled geometry.
*/
fdl_geometry_t geometry_normalize_and_scale(
fdl_geometry_t geo, double source_squeeze, double scale_factor, double target_squeeze);
fdl_geometry_t geometry_normalize_and_scale_ratio(
fdl_geometry_t geo, double source_squeeze, double scale_numerator, double scale_denominator, double target_squeeze);

/**
* @brief Round all 7 fields of the geometry using the given strategy.
Expand Down
3 changes: 2 additions & 1 deletion native/core/src/fdl_geometry_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ fdl_geometry_t fdl_geometry_fill_hierarchy_gaps(fdl_geometry_t geo, fdl_point_f6

fdl_geometry_t fdl_geometry_normalize_and_scale(
fdl_geometry_t geo, double source_squeeze, double scale_factor, double target_squeeze) {
return fdl::detail::geometry_normalize_and_scale(geo, source_squeeze, scale_factor, target_squeeze);
return fdl::detail::geometry_normalize_and_scale_ratio(
geo, source_squeeze, scale_factor, fdl::constants::kIdentityDenominator, target_squeeze);
}

fdl_geometry_t fdl_geometry_round(fdl_geometry_t geo, fdl_round_strategy_t strategy) {
Expand Down
22 changes: 22 additions & 0 deletions native/core/src/fdl_pipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ double calculate_scale_factor(
}
}

scale_ratio_t calculate_scale_ratio(
fdl_dimensions_f64_t fit_norm, fdl_dimensions_f64_t target_norm, fdl_fit_method_t fit_method) {

double const w_ratio = target_norm.width / fit_norm.width;
double const h_ratio = target_norm.height / fit_norm.height;

switch (fit_method) {
case FDL_FIT_METHOD_FIT_ALL:
return (w_ratio <= h_ratio) ? scale_ratio_t{target_norm.width, fit_norm.width}
: scale_ratio_t{target_norm.height, fit_norm.height};
case FDL_FIT_METHOD_FILL:
return (w_ratio >= h_ratio) ? scale_ratio_t{target_norm.width, fit_norm.width}
: scale_ratio_t{target_norm.height, fit_norm.height};
case FDL_FIT_METHOD_WIDTH:
return {target_norm.width, fit_norm.width};
case FDL_FIT_METHOD_HEIGHT:
return {target_norm.height, fit_norm.height};
default:
return {0.0, fdl::constants::kIdentityDenominator};
}
}

double output_size_for_axis(double canvas_size, double max_size, bool has_max, bool pad_to_max) {

if (has_max && pad_to_max) {
Expand Down
28 changes: 28 additions & 0 deletions native/core/src/fdl_pipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@

namespace fdl::detail {

/**
* @brief Scale factor represented as numerator / denominator.
*
* Keeping the ratio split avoids the precision loss inherent in computing
* a single double when the quotient is not exactly representable in IEEE 754.
* The scaling pipeline computes (value * numerator) / denominator instead of
* value * (numerator / denominator), which is exact for integer inputs that
* fit within 2^53.
*/
struct scale_ratio_t {
double numerator;
double denominator;
};

/**
* @brief Calculate scale factor based on fit method.
* @param fit_norm Normalized fit-source dimensions.
Expand All @@ -24,6 +38,20 @@ namespace fdl::detail {
double calculate_scale_factor(
fdl_dimensions_f64_t fit_norm, fdl_dimensions_f64_t target_norm, fdl_fit_method_t fit_method);

/**
* @brief Calculate scale ratio (numerator/denominator) based on fit method.
*
* Returns the numerator and denominator separately so that downstream
* scaling can multiply before dividing, preserving precision.
*
* @param fit_norm Normalized fit-source dimensions.
* @param target_norm Normalized target dimensions.
* @param fit_method Fit method (width, height, fill, fit_all).
* @return Scale ratio with separate numerator and denominator.
*/
scale_ratio_t calculate_scale_ratio(
fdl_dimensions_f64_t fit_norm, fdl_dimensions_f64_t target_norm, fdl_fit_method_t fit_method);

/**
* @brief Determine output canvas size for a single axis.
* @param canvas_size Scaled canvas size along this axis.
Expand Down
10 changes: 8 additions & 2 deletions native/core/src/fdl_template.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "fdl_constants.h"
#include "fdl_doc.h"
#include "fdl_enum_map.h"
#include "fdl_geometry.h"
#include "fdl_pipeline.h"

#include <algorithm>
#include <cmath>
Expand Down Expand Up @@ -484,10 +486,14 @@ fdl_template_result_t apply_canvas_template(
// --- Calculate scale factor ---
fdl_dimensions_f64_t const fit_norm = fdl_dimensions_normalize(fit_dims, input_squeeze);
fdl_dimensions_f64_t const target_norm = fdl_dimensions_normalize(target_dims, target_squeeze);
double const scale_factor = fdl_calculate_scale_factor(fit_norm, target_norm, fit_method);
auto const scale_ratio = fdl::detail::calculate_scale_ratio(fit_norm, target_norm, fit_method);
double const scale_factor = scale_ratio.numerator / scale_ratio.denominator;

// --- Phase 5: Scale and round ---
geometry = fdl_geometry_normalize_and_scale(geometry, input_squeeze, scale_factor, target_squeeze);
// Use ratio-based scaling: (value * numerator) / denominator preserves precision
// for integer inputs, avoiding IEEE 754 rounding errors in the scale factor.
geometry = fdl::detail::geometry_normalize_and_scale_ratio(
geometry, input_squeeze, scale_ratio.numerator, scale_ratio.denominator, target_squeeze);
geometry = fdl_geometry_round(geometry, rounding);

// Extract scaled values BEFORE crop
Expand Down
Loading
Loading