From b8293cb8bab65a6e42d95b6e1c39b76bcc1af45e Mon Sep 17 00:00:00 2001 From: Jackson Sun Date: Mon, 10 Aug 2026 16:34:45 -0400 Subject: [PATCH] fix(IBA): keep nearest resample off the Highway path resample_hwy() ignores its `interpolate` argument and always bilerps, but resample_() routed nearest sampling to it whenever Highway was enabled, so ImageBufAlgo::resample(dst, src, false) returned filtered pixels instead of the source pixel the caller asked for. Measured against a 128x128 nearest resample of testsuite/common/grid.tif, 40.9% of pixels were wrong, with a maximum error of 0.608. Latent today because enable_hwy defaults to 0, but it becomes a live behavior change as soon as Highway is turned on by default. Also fixes an MSVC build failure in the same subsystem. The two hwy_*_perpixel_op_rgba_rgb_roi() lambdas derived a tag type with decltype(d), where d is captured from the enclosing function; MSVC yields a reference type there, and D::template Rebind cannot be named through a reference. Naming the tag directly drops the dependency on how decltype treats a captured entity, and says what the tag is rather than what it matches. Signed-off-by: Jackson Sun --- src/libOpenImageIO/imagebufalgo_hwy_pvt.h | 4 +-- src/libOpenImageIO/imagebufalgo_test.cpp | 35 +++++++++++++++++++++++ src/libOpenImageIO/imagebufalgo_xform.cpp | 8 +++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/libOpenImageIO/imagebufalgo_hwy_pvt.h b/src/libOpenImageIO/imagebufalgo_hwy_pvt.h index a2e2161a71..774904d443 100644 --- a/src/libOpenImageIO/imagebufalgo_hwy_pvt.h +++ b/src/libOpenImageIO/imagebufalgo_hwy_pvt.h @@ -1100,7 +1100,7 @@ hwy_binary_perpixel_op_rgba_rgb_roi(ImageBuf& R, const ImageBuf& A, (void)ba; using DstLaneT = HwyLaneTypeT; - auto d_dstlane = hn::Rebind(); + auto d_dstlane = hn::Rebind>(); hn::Vec dr, dg, db, da; hn::LoadInterleaved4(d_dstlane, reinterpret_cast(r_row @@ -1189,7 +1189,7 @@ hwy_ternary_perpixel_op_rgba_rgb_roi(ImageBuf& R, const ImageBuf& A, (void)ca; using DstLaneT = HwyLaneTypeT; - auto d_dstlane = hn::Rebind(); + auto d_dstlane = hn::Rebind>(); hn::Vec dr, dg, db, da; hn::LoadInterleaved4(d_dstlane, reinterpret_cast(r_row diff --git a/src/libOpenImageIO/imagebufalgo_test.cpp b/src/libOpenImageIO/imagebufalgo_test.cpp index 42367ae3d2..6c94e134ae 100644 --- a/src/libOpenImageIO/imagebufalgo_test.cpp +++ b/src/libOpenImageIO/imagebufalgo_test.cpp @@ -739,6 +739,40 @@ test_zover() // Test ImageBuf::resample +// resample_hwy() ignores its `interpolate` argument and always bilerps, so +// nearest sampling has to stay off that path. Highway is off by default, so a +// reference image would not exercise this; compare the two settings instead. +void +test_resample_hwy_nearest() +{ + std::cout << "test resample nearest is not routed to Highway\n"; + + int prev_hwy = 0; + OIIO::getattribute("enable_hwy", prev_hwy); + + // Neighbouring pixels must differ, or bilerp and nearest would agree and + // the test would pass on broken code. + ImageSpec srcspec(64, 48, 4, TypeFloat); + ImageBuf src(srcspec); + for (ImageBuf::Iterator it(src); !it.done(); ++it) + for (int c = 0; c < 4; ++c) + it[c] = float(((it.x() + it.y() + c) % 7) * 0.125f); + + ImageSpec dstspec(37, 29, 4, TypeFloat); + ImageBuf without_hwy(dstspec), with_hwy(dstspec); + OIIO::attribute("enable_hwy", 0); + OIIO_CHECK_ASSERT(ImageBufAlgo::resample(without_hwy, src, false)); + OIIO::attribute("enable_hwy", 1); + OIIO_CHECK_ASSERT(ImageBufAlgo::resample(with_hwy, src, false)); + OIIO::attribute("enable_hwy", prev_hwy); + + OIIO_CHECK_EQUAL(memcmp(with_hwy.localpixels(), without_hwy.localpixels(), + dstspec.image_bytes()), + 0); +} + + + void test_resample() { @@ -1811,6 +1845,7 @@ main(int argc, char** argv) test_over(TypeFloat); test_over(TypeHalf); test_zover(); + test_resample_hwy_nearest(); test_resample(); test_compare(); test_isConstantColor(); diff --git a/src/libOpenImageIO/imagebufalgo_xform.cpp b/src/libOpenImageIO/imagebufalgo_xform.cpp index b469da6173..9c9b89ec9a 100644 --- a/src/libOpenImageIO/imagebufalgo_xform.cpp +++ b/src/libOpenImageIO/imagebufalgo_xform.cpp @@ -1272,6 +1272,9 @@ static bool resample_hwy(ImageBuf& dst, const ImageBuf& src, bool interpolate, ROI roi, int nthreads) { + // This routine has no nearest case -- it always interpolates -- so + // resample_() must not route nearest sampling here. + OIIO_ASSERT(interpolate); using SimdType = std::conditional_t, double, float>; using D = hn::ScalableTag; @@ -1435,7 +1438,10 @@ resample_(ImageBuf& dst, const ImageBuf& src, bool interpolate, ROI roi, int nthreads) { #if OIIO_USE_HWY - if (OIIO::pvt::enable_hwy && HwySupports(dst, roi) + // Interpolating only: resample_hwy() ignores its `interpolate` argument + // and always bilerps, so sending nearest here silently returns filtered + // pixels instead of the source pixel the caller asked for. + if (interpolate && OIIO::pvt::enable_hwy && HwySupports(dst, roi) && HwySupports(src, ROI())) return resample_hwy(dst, src, interpolate, roi, nthreads);