Fix bug in put/get of non-continuous Mat#19503
Conversation
aa4840d to
8b998db
Compare
| if (ar1.length != ar2.length) { | ||
| fail("Arrays have different sizes."); |
There was a problem hiding this comment.
@asmorkalov this was a copy/paste from another assertArrayEquals function - I'll change all of them to be like this
| for (int index = 0; index < inc; index++) { | ||
| if (incIdx(mat, indices)) { | ||
| return true; |
There was a problem hiding this comment.
Increasing by one multiple times is really slow (for HD images inc value is up to 1280).
We need "fast path" to handle "always contiguous" last dim (cols).
{
int cols_dim = mat->dims - 1;
size_t cols = mat->size[cols_dim];
CV_CheckLE(cols, (size_t)INT_MAX, ""); // 'int' only, size_t type is not supported for indexes
size_t new_cols_idx = (size_t)indices[cols_dim] + inc;
// only "cols" idx is allowed to be increased till the end
// do not allow to jump forward because we should support non-contiguous layouts too (submats)
CV_CheckLE(new_cols_idx, cols, "");
if (new_cols_idx == cols)
{
indices[cols_dim] = 0;
for (int dim = cols_dim - 1; dim >= 0; --dim)
{
int new_idx = indices[dim] + 1;
if (new_idx != mat->size[dim])
{
indices[dim] = new_idx;
return false;
}
else
{
indices[dim] = 0;
continue;
}
}
return true;
}
else
{
indices[cols_dim] = (int)new_cols_idx;
return false;
}
}
There was a problem hiding this comment.
Actually updating the indices doesn't require any iteration - we can just calculate where the new indices will end up.
But I get your point - we can optimize the copying of data if the trailing dimensions are contiguous. Will push a new optimized implementation.
| assertEquals(4, bytesNum); | ||
| assertTrue(Arrays.equals(new short[] {340, 341, 0, 0}, buff11)); | ||
|
|
||
| Mat m2 = new Mat(new int[]{ 5, 5, 5 }, CvType.CV_16S); |
There was a problem hiding this comment.
5, 5, 5
different numbers are preferable (to ensure that we don't mess with "size[]" array order in implementation):
5, 6, 8
Rebasing on the 3.4 branch |
724aafd to
775eda1
Compare
|
I spotted another bug in the calculation of elements remaining in the Mat: I will convert to draft while I fix this |
775eda1 to
e23b7aa
Compare
|
OK - I have reworked this a bit. |
|
@komakai Is it ready for review? |
e23b7aa to
5cf08b0
Compare
@asmorkalov Yeah (once the builds pass!) |
Pull Request Readiness Checklist
Fix for #19502