Merge pull request #14985 from andrey-golubev:gapi_fix_ocl_umat

* G-API: fix GOCLExecutable issue with UMat lifetime

Add tests on initialized/uninitialized outputs for all
backends

* Use proper clean-up procedure for magazine

* Rename InitOut test and reduce tested sizes

* Enable output allocation test

modules/gapi/src/api/gbackend.cpp

@@ -103,7 +103,7 @@ void bindInArg(Mag& mag, const RcDesc &rc, const GRunArg &arg, bool is_umat)
                 auto& mag_umat = mag.template slot<cv::UMat>()[rc.id];
                 mag_umat = to_ocv(util::get<cv::gapi::own::Mat>(arg)).getUMat(ACCESS_READ);
 #else
-                util::throw_error(std::logic_error("UMat is not supported in stadnalone build"));
+                util::throw_error(std::logic_error("UMat is not supported in standalone build"));
 #endif // !defined(GAPI_STANDALONE)
             }
             else

modules/gapi/src/backends/ocl/goclbackend.cpp

@@ -150,6 +150,26 @@ void cv::gimpl::GOCLExecutable::run(std::vector<InObj>  &&input_objs,
     // has received from user (or from another Island, or mix...)
     // FIXME: Check input/output objects against GIsland protocol
 
+    // NB: We must clean-up m_res before this function returns because internally (bindInArg,
+    //     bindOutArg) we work with cv::UMats, not cv::Mats that were originally placed into the
+    //     input/output objects. If this is not done and cv::UMat "leaves" the local function scope,
+    //     certain problems may occur.
+    //
+    //     For example, if the original output (cv::Mat) is re-initialized by the user but we still
+    //     hold cv::UMat -> we get cv::UMat that has a parent that was already destroyed. Also,
+    //     since we don't own the data (the user does), there's no point holding it after we're done
+    const auto clean_up = [&input_objs, &output_objs] (cv::gimpl::Mag* p)
+    {
+        // Only clean-up UMat entries from current scope, we know that inputs and outputs are stored
+        // as UMats from the context below, so the following procedure is safe
+        auto& umats = p->slot<cv::UMat>();
+        // NB: avoid clearing the whole magazine, there's also pre-allocated internal data
+        for (auto& it : input_objs)  umats.erase(it.first.id);
+        for (auto& it : output_objs) umats.erase(it.first.id);
+    };
+    // RAII wrapper to clean-up m_res
+    std::unique_ptr<cv::gimpl::Mag, decltype(clean_up)> cleaner(&m_res, clean_up);
+
     for (auto& it : input_objs)   magazine::bindInArg (m_res, it.first, it.second, true);
     for (auto& it : output_objs)  magazine::bindOutArg(m_res, it.first, it.second, true);
 

modules/gapi/test/common/gapi_core_tests.hpp

@@ -123,6 +123,7 @@ struct BackendOutputAllocationTest : TestWithParamBase<>
 };
 // FIXME: move all tests from this fixture to the base class once all issues are resolved
 struct BackendOutputAllocationLargeSizeWithCorrectSubmatrixTest : BackendOutputAllocationTest {};
+GAPI_TEST_FIXTURE(ReInitOutTest, initNothing, <cv::Size>, 1, out_sz)
 } // opencv_test
 
 #endif //OPENCV_GAPI_CORE_TESTS_HPP

modules/gapi/test/common/gapi_core_tests_inl.hpp

@@ -1310,8 +1310,7 @@ TEST_P(BackendOutputAllocationTest, CorrectlyPreallocatedOutput)
     EXPECT_EQ(out_mat_gapi_ref.data, out_mat_gapi.data);
 }
 
-// FIXME: known issue with OCL backend - PR #14985
-TEST_P(BackendOutputAllocationTest, DISABLED_IncorrectOutputMeta)
+TEST_P(BackendOutputAllocationTest, IncorrectOutputMeta)
 {
     // G-API code //////////////////////////////////////////////////////////////
     cv::GMat in1, in2, out;
@@ -1479,6 +1478,39 @@ TEST_P(BackendOutputAllocationTest, LargerPreallocatedSizeWithSmallSubmatrix)
     EXPECT_NE(out_mat_gapi.data, out_mat_gapi_submat.datastart);
 }
 
+TEST_P(ReInitOutTest, TestWithAdd)
+{
+    in_mat1 = cv::Mat(sz, type);
+    in_mat2 = cv::Mat(sz, type);
+    cv::randu(in_mat1, cv::Scalar::all(0), cv::Scalar::all(100));
+    cv::randu(in_mat2, cv::Scalar::all(0), cv::Scalar::all(100));
+
+    // G-API code //////////////////////////////////////////////////////////////
+    cv::GMat in1, in2, out;
+    out = cv::gapi::add(in1, in2, dtype);
+    cv::GComputation c(cv::GIn(in1, in2), cv::GOut(out));
+
+    const auto run_and_compare = [&c, this] ()
+    {
+        // G-API code //////////////////////////////////////////////////////////////
+        c.apply(cv::gin(in_mat1, in_mat2), cv::gout(out_mat_gapi), getCompileArgs());
+
+        // OpenCV code /////////////////////////////////////////////////////////////
+        cv::add(in_mat1, in_mat2, out_mat_ocv, cv::noArray());
+
+        // Comparison //////////////////////////////////////////////////////////////
+        EXPECT_EQ(0, cv::countNonZero(out_mat_gapi != out_mat_ocv));
+        EXPECT_EQ(out_mat_gapi.size(), sz);
+    };
+
+    // run for uninitialized output
+    run_and_compare();
+
+    // run for initialized output (can be initialized with a different size)
+    initOutMats(out_sz, type);
+    run_and_compare();
+}
+
 } // opencv_test
 
 #endif //OPENCV_GAPI_CORE_TESTS_INL_HPP

modules/gapi/test/cpu/gapi_core_tests_cpu.cpp

@@ -448,4 +448,12 @@ INSTANTIATE_TEST_CASE_P(BackendOutputAllocationLargeSizeWithCorrectSubmatrixTest
                                 Values(cv::Size(50, 50)),
                                 Values(-1),
                                 Values(CORE_CPU)));
+
+INSTANTIATE_TEST_CASE_P(ReInitOutTestCPU, ReInitOutTest,
+                        Combine(Values(CV_8UC3, CV_16SC4, CV_32FC1),
+                                Values(cv::Size(640, 480)),
+                                Values(-1),
+                                Values(CORE_CPU),
+                                Values(cv::Size(640, 400),
+                                       cv::Size(10, 480))));
 }

modules/gapi/test/cpu/gapi_core_tests_fluid.cpp

@@ -267,6 +267,14 @@ INSTANTIATE_TEST_CASE_P(BackendOutputAllocationLargeSizeWithCorrectSubmatrixTest
                                 Values(-1),
                                 Values(CORE_FLUID)));
 
+INSTANTIATE_TEST_CASE_P(ReInitOutTestFluid, ReInitOutTest,
+                        Combine(Values(CV_8UC3, CV_16SC4, CV_32FC1),
+                                Values(cv::Size(640, 480)),
+                                Values(-1),
+                                Values(CORE_FLUID),
+                                Values(cv::Size(640, 400),
+                                       cv::Size(10, 480))));
+
 //----------------------------------------------------------------------
 // FIXME: Clean-up test configurations which are enabled already
 #if 0

modules/gapi/test/gpu/gapi_core_tests_gpu.cpp

@@ -393,6 +393,14 @@ INSTANTIATE_TEST_CASE_P(DISABLED_BackendOutputAllocationLargeSizeWithCorrectSubm
                                 Values(-1),
                                 Values(CORE_GPU)));
 
+INSTANTIATE_TEST_CASE_P(ReInitOutTestGPU, ReInitOutTest,
+                        Combine(Values(CV_8UC3, CV_16SC4, CV_32FC1),
+                                Values(cv::Size(640, 480)),
+                                Values(-1),
+                                Values(CORE_GPU),
+                                Values(cv::Size(640, 400),
+                                       cv::Size(10, 480))));
+
 //TODO: fix this backend to allow ConcatVertVec ConcatHorVec
 #if 0
 INSTANTIATE_TEST_CASE_P(ConcatVertVecTestGPU, ConcatVertVecTest,