From 3700f79b901c3d6eaf7076eed8223fb2edb4d212 Mon Sep 17 00:00:00 2001 From: vathib Date: Fri, 7 Aug 2026 00:55:23 +0200 Subject: [PATCH] segmentation: fix mask upsampling to pixel-centre convention _crop_resize_mask() maps output pixels back to the decoder mask grid with an align-corners convention: sx = x * (valid_w - 1) / (dst_w - 1) That is not the convention the mask is defined on. SAM and SAM 2 map the decoder mask to image resolution with F.interpolate(..., align_corners=False), and _preprocess_image() builds the encoder input with plain scaling (src = y / scale). The mapping is therefore not the inverse of what produced the mask. Align-corners additionally ties it to valid_w -- the mask column count rounded to the nearest integer, 170 for a 170.17-column valid region -- which stretches the mask outwards. Measured with a NumPy replay of the function on a synthetic 256x256 logit mask whose edges sit at exactly known positions, upsampled to 1021x1536, the render darktable produces for a 4024x6048 raw at the default render cap (SEG_RENDER_DEFAULT, object.c), at scale 1/6: the SAM 2.1 decoder case, mask_scale = ctx->scale * dec_h / input_size = 256/1536. "expected" is edge / scale, the position the model's own convention implies. Plain bilinear path, along the 1021 px axis: edge in mask expected current error 20 120.0 118.0 -2.0 60 360.0 360.0 +0.0 100 600.0 601.0 +1.0 140 840.0 842.0 +2.0 168 1008.0 1011.0 +3.0 The error is affine, not a constant offset: a fixed -3.0 output px term plus a stretch reaching +6.0 px across the frame. No constant shift absorbs it. Along the 1536 px axis, where valid_h = 256 is exact, the sweep is -2 to +2; the extra pixel on the short axis comes from the rounding of valid_w. The mask is vectorised at render resolution and mapped back to full resolution, so on that 24 Mpix file the +3 px is about 12 px of contour displacement at the far edge, and the stretch across the frame is about 24 px. The call site always enables the joint-bilateral path -- the guide is the encoded render, sigma_range is 0.1 -- so the plain bilinear path above isolates the sampling rather than describing production. Re-measured with the guide on, the guide partly hides the error when the image has a strong luma step exactly at the contour: the sweep collapses to 0 except +3 px at the far edge. On a soft, low-contrast or textureless guide it does not, and the full sweep comes back. The corrected mapping measures 0.00 px at every one of the 60 points tested: five edges per axis, both axes, six guide regimes. Use the pixel-centre convention, whose inverse is exact: output pixel x covers [x, x+1[, its centre maps to (x + 0.5) * scale, so the sample index is (x + 0.5) * scale - 0.5. The guide coordinate mapping in the joint-bilateral branch inverted the same formula and is corrected with it: it has to be the exact inverse of the sampling above, or the range weights are read at guide pixels the mask values do not come from. Fixing only the sampling would leave the two mutually inconsistent. The (dst_w > 1) and (valid_w > 1) guards existed only to avoid dividing by (dst_w - 1) and (valid_w - 1); the new formulas contain no division by a dimension, so they go with it. Two consequences worth naming. The new mapping reaches sample index 169.58 on the 1021 px axis, past valid_w - 1 = 169, so the last four output columns and the last three rows now replicate the last fully-valid mask sample instead of interpolating into mask column 170, which is 17% image and 83% encoder padding. The align-corners mapping landed exactly on valid - 1 and never reached the clamp. Replicating is the intended behaviour here; interpolating into the padded column would pull the mask towards the background. masks/object.c feeds the upsampled mask back into the automatic prompt refinement (peak point and bounding box), so moving the contour by 2 to 3 px moves the points fed to passes 2..n by 1 to 2 encoder px. The change is not confined to post-processing. Nothing already on disk is affected: the encoder embedding cache stores only what is upstream of this function, prev_mask is copied from the raw decoder output, and existing masks are stored as vectorised control points and never re-derived. Only newly created masks land differently. The function is shared by both supported architectures and runs before any further processing, but the magnitude is architecture-dependent: the SegNext decoder outputs a 1024x1024 mask (mask_scale 2/3 on the same render), where the error stays at 0.25 output px and is invisible. The measurable gain is on SAM 2.1, whose decoder mask is 6x coarser than the render. The correctness argument applies to both. Known limitation, deliberately left out: _preprocess_image() downscales the encoder input with the naive mapping (src = y / scale), and the prompt coordinates are mapped the same way (points[i].x * ctx->scale), consistent with the reference implementation. Together they leave a constant 0.5 * (1/ctx->scale - 1) = 0.25 output px offset in the full chain, about 1 px at full resolution, against the 12 px of stretch removed here. Correcting it changes what the encoder is fed -- every existing mask and every cached embedding -- so it belongs in a separate change. No new tunable. --- src/common/ai/segmentation.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/common/ai/segmentation.c b/src/common/ai/segmentation.c index 4d30f9bb3826..8c3134ed794b 100644 --- a/src/common/ai/segmentation.c +++ b/src/common/ai/segmentation.c @@ -225,17 +225,26 @@ static void _crop_resize_mask(const float *const restrict src, } } + // inverse of the sampling mapping below, hoisted out of the pixel loops + const float inv_scale = (scale > 1e-6f) ? 1.0f / scale : 0.0f; + DT_OMP_FOR(shared(range_lut)) for(int y = 0; y < dst_h; y++) { - const float sy = (dst_h > 1) ? (float)y * (float)(valid_h - 1) / (float)(dst_h - 1) : 0.0f; + // pixel-centre convention: output pixel y covers [y, y+1[, whose + // centre y + 0.5 maps to (y + 0.5) * scale in mask space, i.e. + // sample index (y + 0.5) * scale - 0.5. the previous align-corners + // mapping was not the inverse of that and stretched the mask + // outwards; see the commit message for figures. the clamp is + // load-bearing: it keeps fy >= 0 and makes (int)sy a floor + const float sy = MAX(((float)y + 0.5f) * scale - 0.5f, 0.0f); const int y0 = MIN((int)sy, valid_h - 1); const int y1 = MIN(y0 + 1, valid_h - 1); const float fy = sy - (float)y0; for(int x = 0; x < dst_w; x++) { - const float sx = (dst_w > 1) ? (float)x * (float)(valid_w - 1) / (float)(dst_w - 1) : 0.0f; + const float sx = MAX(((float)x + 0.5f) * scale - 0.5f, 0.0f); const int x0 = MIN((int)sx, valid_w - 1); const int x1 = MIN(x0 + 1, valid_w - 1); const float fx = sx - (float)x0; @@ -255,11 +264,13 @@ static void _crop_resize_mask(const float *const restrict src, // reference luma at the output pixel (guide is at dst resolution) const int luma_ref = _luma709(&guide_rgb[(y * guide_w + x) * 3]); - // map the 4 source-grid corners back into guide coords - const float gx0 = (valid_w > 1) ? (float)x0 * (float)(dst_w - 1) / (float)(valid_w - 1) : 0.0f; - const float gx1 = (valid_w > 1) ? (float)x1 * (float)(dst_w - 1) / (float)(valid_w - 1) : 0.0f; - const float gy0 = (valid_h > 1) ? (float)y0 * (float)(dst_h - 1) / (float)(valid_h - 1) : 0.0f; - const float gy1 = (valid_h > 1) ? (float)y1 * (float)(dst_h - 1) / (float)(valid_h - 1) : 0.0f; + // map the 4 source-grid corners back into guide coords -- exact + // inverse of the sampling mapping above, so that the range weights + // are read at the guide pixels the mask values actually come from + const float gx0 = ((float)x0 + 0.5f) * inv_scale - 0.5f; + const float gx1 = ((float)x1 + 0.5f) * inv_scale - 0.5f; + const float gy0 = ((float)y0 + 0.5f) * inv_scale - 0.5f; + const float gy1 = ((float)y1 + 0.5f) * inv_scale - 0.5f; const int ix00 = MIN(MAX((int)(gx0 + 0.5f), 0), guide_w - 1); const int ix01 = MIN(MAX((int)(gx1 + 0.5f), 0), guide_w - 1);