Fix bugs in arithm_op() for InputArray (src == dst) case.

[eightco]

Fix bugs in arithm_op() for InputArray (src == dst) case.

It is a well-known issue that InputArray (src == dst) can cause problems in opencv.
So I try to fix this issue through this PR.

The cause of this problem is the recreate of dst by call create().

There are many ways to solve this problem, but most of them are difficult solutions.
Store Mat itself inside Input/OutputArray, not the pointer to it, but it would noticeably increase the overhead and also It is possible to generate a side effect bug on any code that uses InputArray.

So it is difficult to solve the problem for every part that can be (src == dst) by modifying InputArray
In most cases, we can prevent the (src == dst) case with CV_Assert(src1.getObj() != dst.getObj())
However, in the case of Mat's += operations, we must allow src == dst.

Change the internal state of InputArray to solve this problem is difficult and dangerous.
So I propose to make a temporary Mat for src on the stack when src == dst at the beginning of the arithm_op function.
I think this method can avoid this problem with small overhead and the possibility of side effects being small.

The reasons are as follows.

Anyway, in the arithm_op function, src will be converted via getMat() or getUMat() to be used inside the operation function.
Therefore, if we create a temporary Mat (or UMat) for src at the beginning of the function, the overhead would be small.

If src is Mat (or UMat), it is only a ref count increment and more use of stack memory
Otherwise It may take time to convert to Mat, but in the case of arithm_op function it must be converted by GetMat() (or GetUMat()) function to be used inside this function anyway.
So this is not an additional overhead.

After the above operation, even if dst is recreated in the case of src == dst, psrc is safe because it refers to the temporary Mat.
This temporary Mat is created on the stack and will disappear when the function ends.

The following issues can be solved with this modification.

issue #9688

    Mat x(1, 1, CV_64FC1);
    Mat p(10, 4, CV_64FC1);
    x.setTo(0);
    p.setTo(Scalar::all(5));
    for (int i = 0; i < p.rows; i++)
        x += p.row(i);
    ASSERT_DOUBLE_EQ(50.0, x.ptr<double>()[0]);
    ASSERT_DOUBLE_EQ(50.0, x.ptr<double>()[1]);
    ASSERT_DOUBLE_EQ(50.0, x.ptr<double>()[2]);
    ASSERT_DOUBLE_EQ(50.0, x.ptr<double>()[3]);

issue #8385

    Mat x(1, 2, CV_8UC1, Scalar::all(3)), y;
    cv::add(x, 2.0, x);
    cv::multiply(x, x, x, 1, CV_16UC1);
    EXPECT_EQ(25, (int)x.ptr<ushort>()[0]);
    EXPECT_EQ(25, (int)x.ptr<ushort>()[1]);

I want to solve this problem.
Please let me know if this PR has a problem or if there are other good solutions

[alalek]

Please add some simple test based on your code.

[alalek]

Use this method of InputArray:

bool isUMat() const

[alalek]

Actually there is more common problem: #8352
Not sure if such common problems should be fixed via local hacks.

Using of "inplace" operations with re-allocation of destination matrix is something strange operation (make no sense, because there is no real inplace processing).
However this approach is used quite often, especially as pre-processing (cvtColor(), filtering via GaussianBlur(), etc).

[eightco]

#8341 #8149 are also the same cause, so it can be fixed by applying the same method as this PR.
but your comment also makes sense.
So there is way to throw an exception if recreation happens in an inplace operation.

However, if this local hacks does not fit your standard.
It is also good to reject and close this PR.

[vpisarev]

@eightco, thank you for the contribution! We discussed this PR within the core team. Arithmetic operations are very cheap, so the implicit allocation of a temporary array is not very good, it may effectively double the execution time. Especially given that in most cases those implicit operations work correctly. It's just that those 2 reported issues are about the case when the input (and the output at the same time) matrix is mis-interpreted as a scalar, and the code behaves incorrectly. I think, we should just detect such cases and throw an error instead of implicit allocation of a temporary array.