diff --git a/modules/core/src/arithm_expr.cpp b/modules/core/src/arithm_expr.cpp index f949ec395d..6949b42181 100644 --- a/modules/core/src/arithm_expr.cpp +++ b/modules/core/src/arithm_expr.cpp @@ -314,8 +314,9 @@ int TExpr::emitBinary(TOp op, int a, int b, int rdepth, const Scalar& params) { // addWeighted a*alpha + b*beta + gamma (params = {alpha, beta, gamma}): ONE fused kernel (two v_fma). // Inputs are the same type T (cast to a common type if not). The kernel outputs T/f32 (small ints, - // f16/bf16, f32) or f64 directly; for any other requested rdepth it computes in the work type W and a - // final cast narrows it. + // f16/bf16, f32) or f64 directly, at its own natural work precision; for any other requested rdepth + // it computes at that natural precision and casts the RESULT to rdepth afterward (never the inputs + // up before the op, that would spend an extra cast and run the op at needlessly high precision). if (op == OP_ADDW) { int Tt = arginfo[a].depth; @@ -324,14 +325,17 @@ int TExpr::emitBinary(TOp op, int a, int b, int rdepth, const Scalar& params) Tt = promoteArith(arginfo[a].depth, arginfo[b].depth); a = maybeAddCast(a, Tt); b = maybeAddCast(b, Tt); } + if (Tt == CV_Bool) + CV_Error(Error::StsNotImplemented, "addWeighted: CV_Bool inputs are not supported, cast explicitly " + "first, e.g. cv::texpr(\"uint8({0})*{1} + uint8({2})*{3}\", {a, alpha, b, beta})"); if (rdepth == EW_DEPTH_NONE) rdepth = Tt; // default dtype = input depth TKernel k = getElemwiseFunc(OP_ADDW, Tt, Tt, EW_DEPTH_NONE, rdepth); int outD = rdepth; - if (!k.fptr) // no direct T->rdepth kernel: compute in W, cast + if (!k.fptr) // no direct T->rdepth kernel: compute at T's natural work depth, cast after { - outD = (Tt==CV_32U || Tt==CV_32S || Tt==CV_64U || Tt==CV_64S || Tt==CV_64F || rdepth==CV_64F) - ? CV_64F : CV_32F; + outD = (Tt==CV_32U || Tt==CV_32S || Tt==CV_64U || Tt==CV_64S || Tt==CV_64F) ? CV_64F : CV_32F; k = getElemwiseFunc(OP_ADDW, Tt, Tt, EW_DEPTH_NONE, outD); + CV_Assert(k.fptr && "ew: no kernel for this op/type combination"); } const int out = addTemp(outD); addInsn(OP_ADDW, a, b, 0, out, k, Scalar(params[0], params[1], params[2])); diff --git a/modules/core/test/test_arithm.cpp b/modules/core/test/test_arithm.cpp index 8b7a9eace7..ca06c21133 100644 --- a/modules/core/test/test_arithm.cpp +++ b/modules/core/test/test_arithm.cpp @@ -4276,6 +4276,57 @@ TEST(Core_Arithm, DISABLED_mul_overflow_28557) } +// https://github.com/opencv/opencv/issues/29880 +typedef testing::TestWithParam< tuple > Core_AddWeighted_regression29880; + +TEST_P(Core_AddWeighted_regression29880, dtype) +{ + const int sdepth = get<0>(GetParam()); + const int dtype = get<1>(GetParam()); + const int ddepth = dtype < 0 ? sdepth : dtype; + + cv::Mat src(4, 4, CV_MAKETYPE(sdepth, 1), cv::Scalar::all(1)), dst, dst64f; + cv::addWeighted(src, 2.0, src, 3.0, 4.0, dst, dtype); + ASSERT_EQ(ddepth, dst.depth()); + dst.convertTo(dst64f, CV_64F); + EXPECT_EQ(0, cv::countNonZero(dst64f != (ddepth == CV_Bool ? 1.0 : 9.0))); +} + +// sdepth excludes CV_Bool: addWeighted now rejects Bool sources outright, see below. +INSTANTIATE_TEST_CASE_P(/**/, Core_AddWeighted_regression29880, testing::Combine( + testing::Values(CV_8U, CV_8S, CV_16U, CV_16S, CV_16F, CV_16BF, CV_32F), + testing::Values(-1, CV_8U, CV_32F, CV_64F, CV_Bool))); + +// CV_Bool sources are disabled per https://github.com/opencv/opencv/pull/29883#issuecomment-5569942015: +// the user should cast explicitly instead. +typedef testing::TestWithParam Core_AddWeighted_boolInput_29880; + +TEST_P(Core_AddWeighted_boolInput_29880, throws) +{ + const int dtype = GetParam(); + cv::Mat src(4, 4, CV_MAKETYPE(CV_Bool, 1), cv::Scalar::all(1)), dst; + ASSERT_THROW(cv::addWeighted(src, 2.0, src, 3.0, 4.0, dst, dtype), cv::Exception); +} + +INSTANTIATE_TEST_CASE_P(/**/, Core_AddWeighted_boolInput_29880, + testing::Values(-1, CV_8U, CV_32F, CV_64F, CV_Bool)); + +// The dtype=CV_64F path computes at fp32 (addWeighted's native work precision for 8U..32F sources) +// and casts the RESULT up to fp64, rather than widening the sources to fp64 before the op; the two +// give different bit patterns for a generic alpha, so this pins down which one actually runs. +TEST(Core_Arithm, addWeighted_fp64_uses_fp32_intermediate_29880) +{ + const float srcVal = 100.f, alpha = 1.f/3, beta = 0.f, gamma = 0.f; + cv::Mat src(1, 1, CV_8UC1, cv::Scalar(srcVal)), dst; + cv::addWeighted(src, (double)alpha, src, (double)beta, (double)gamma, dst, CV_64F); + + const double fp32Then64 = (double)cv::saturate_cast(srcVal*alpha + srcVal*beta + gamma); + const double fp64Only = (double)srcVal*(double)alpha + (double)srcVal*(double)beta + (double)gamma; + ASSERT_NE(fp32Then64, fp64Only) << "chosen alpha does not distinguish the two code paths"; + EXPECT_EQ(fp32Then64, dst.at(0, 0)); +} + + TEST(Core_Arithm, min_empty) { cv::Mat A, B, C;