Skip to content

[G-API] ND RMat Views#18720

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
rgarnov:rg/nd_rmat_view
Nov 16, 2020
Merged

[G-API] ND RMat Views#18720
opencv-pushbot merged 1 commit intoopencv:masterfrom
rgarnov:rg/nd_rmat_view

Conversation

@rgarnov
Copy link
Copy Markdown
Contributor

@rgarnov rgarnov commented Nov 3, 2020

Summary

  • Added missing multidimensional steps (strides) for RMat::View

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2020.3.0:16.04
build_image:Custom Win=openvino-2020.3.0
build_image:Custom Mac=openvino-2020.3.0

test_modules:Custom=gapi
test_modules:Custom Win=gapi
test_modules:Custom Mac=gapi

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

@rgarnov rgarnov changed the title (WIP) [G-API] ND RMat Views [G-API] ND RMat Views Nov 3, 2020
@rgarnov rgarnov requested a review from dmatveev November 3, 2020 15:52
@rgarnov
Copy link
Copy Markdown
Contributor Author

rgarnov commented Nov 3, 2020

@TolyaTalamanov fyi

@rgarnov rgarnov force-pushed the rg/nd_rmat_view branch 3 times, most recently from 47fc882 to eb334fc Compare November 5, 2020 15:28
dmatveev
dmatveev reviewed Nov 6, 2020
View reviewed changes
modules/gapi/include/opencv2/gapi/rmat.hpp Outdated

template<typename T = uchar> T* ptr(int y = 0, int x = 0) {
return reinterpret_cast<T*>(m_data + m_step*y + x*CV_ELEM_SIZE(type()));
return reinterpret_cast<T*>(m_data + step()*y + x*CV_ELEM_SIZE(type()));
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is step() here in the 2d case? what is it in, hm, 3d case? would this function work in 3d or 4d case?

Copy link
Copy Markdown
Contributor Author

@rgarnov rgarnov Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step() returns step value which should be applied when iterating on the first (with biggest order) dimension of the View. So it's a "width" step in 2d case as it was before, semantics unchanged, it's a "height" step in 3d case and "plane" step in 4d case.

Copy link
Copy Markdown
Contributor Author

@rgarnov rgarnov Nov 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding applicability in non-2d cases it's a good question if it even should be called in these cases with x != 0 and x != 0, maybe I should place an assert on that. Or maybe it's worth to generalize this method for any number of dimensions

@rgarnov rgarnov force-pushed the rg/nd_rmat_view branch 2 times, most recently from 5bf2514 to 22039e6 Compare November 10, 2020 09:38
@rgarnov
Copy link
Copy Markdown
Contributor Author

rgarnov commented Nov 10, 2020

@dmatveev updated

@rgarnov
Copy link
Copy Markdown
Contributor Author

rgarnov commented Nov 12, 2020

@dmatveev updated, please take a look

dmatveev
dmatveev approved these changes Nov 16, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, thanks! 👍

@dmatveev
Copy link
Copy Markdown
Contributor

dmatveev commented Nov 16, 2020

@alalek this one can be merged

@opencv-pushbot opencv-pushbot merged commit d0b6637 into opencv:master Nov 16, 2020
@alalek alalek mentioned this pull request Nov 27, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmatveev dmatveev dmatveev approved these changes

@TolyaTalamanov TolyaTalamanov TolyaTalamanov approved these changes

Assignees

@dmatveev dmatveev

Labels

category: g-api / gapi

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

5 participants

@rgarnov @dmatveev @TolyaTalamanov @opencv-pushbot @alalek