Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork56.4k
OpenCV 3D module rendering function PR#24065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
* bmp specified BI_BITFIELDS should take care RGBA bit mask* support xrgb bmp file
add test case for "tf/reduce_sum"* fix output dimension of reduce_sum* add test case and fix bug* add more cases* style:change files name* fix a little bug
Add regression test for QRcode encoding and decodingCo-authored-by: APrigarina <ann73617@gmail.com>
Signed-off-by: Crayon-new <1349159541@qq.com>
Onnx conformance tests* Add ONNX conformance tests* dnn(onnx): make conformance stubs a part of repositoryCo-authored-by: Alexander Alekhin <alexander.a.alekhin@gmail.com>
Signed-off-by: Crayon-new <1349159541@qq.com>
savuor left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is a good progress. There's a couple of things to fix:
- My comments
- Broken CI tests
modules/3d/include/opencv2/3d.hpp Outdated
| */ | ||
| CV_EXPORTS_WvoidsaveMesh(const String &filename, InputArray vertices, InputArray normals, InputArrayOfArrays indices); | ||
| /* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This comment does not render to docs.
You can always check the generated documentation for the PRhere (Builders -> default -> PR number -> Docs -> Upload docs -> Preview).
modules/3d/src/rendering.hpp Outdated
| @@ -0,0 +1 @@ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We don't need empty files, please remove
modules/3d/test/test_rendering.cpp Outdated
| { | ||
| if (debugLevel >0) | ||
| { | ||
| Mat groundTruth =imread("../../../opencv_extra/opencv_extra/testdata/rendering/example_image_depth_1.png"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This file path will work on your machine only.
We have a special function to obtain a path:
// The file path is OPENCV_TEST_DATA_PATH + "/rendering/example_image_depth_1.png"std::string dataPath = cvtest::TS::ptr()->get_data_path() + "/rendering/example_image_depth_1.png";The test executables should be run with env variableOPENCV_TEST_DATA_PATH set totestdata folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
findDataFile() should be used.
modules/3d/src/rendering.cpp Outdated
| std::vector<Vec3f> verticesVector = vertices.getMat(); | ||
| std::vector<Vec3f> indicesVector = indices.getMat(); | ||
| std::vector<Vec3f> colorsVector = colors.getMat(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should allow an empty vertex colors array. In that case all the triangles will be drawn by the same color (let it be full white).
modules/3d/include/opencv2/3d.hpp Outdated
| **/ | ||
| CV_EXPORTSvoidtriangleRasterize(InputArray vertices, InputArray indices, | ||
| InputArray colors, InputArray cameraMatrix,int width,int height,bool shadingMode, | ||
| OutputArray depth_buf, OutputArray color_buf); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK, the thing happened tocameraMatrix is not what I was talking about. It should be changed:
- The way of passing camera pose by lookAt, position and up vector is not an OpenCV-style, we use camera pose matrix for that (in your code this is called
lookAtMatrix - The intrinsic camera parameters are also passed in different way, by 3x3 intrinsic matrix
I'll rewrite the doc string, and you are to change the function signature according to it.
Renders a set of triangles on a depth and/or RGB image. The output images are not cleared before the rendering and can be used for masking or with pre-filled Z-buffer.Triangles can be flat-shaded or have interpolated colors between vertices. In flat-shaded mode a color of 1st triangle vertex is used.If no colors are given, the triangles are drawn in white (1.0, 1.0, 1.0).@param vertices vertices coordinates array. Should contain values of CV_32FC3 type or a compatible one (e.g. cv::Vec3f, etc.)@param indices triangle vertices index array, 3 per triangle. Each index indicates a vertex in a vertices array. Should contain CV_32SC3 values@param colors per-vertex colors of CV_32FC3 type, can be empty.@param cameraPose a 4x3 or 4x4 float matrix containing camera pose@param fovY field of view in vertical direction, given in degrees@param zNear minimum Z value to render, everything closer is clipped@param zFar maximum Z value to render, everything farther is clipped@param width Frame width@param height Frame height@param useFlatShading Enables or disables flat shading *@param depthBuf a width x height array of floats containing resulting Z buffer. Reused if not empty. Created and filled by zFar values if a user-provided array is empty. To disable Z buffer output, pass cv::noArray() here.*@param colorBuf a width x height array of CV_32FC3 representing the final rendered image. Reused if not empty. Created and filled by zeroes if a user-provided array is empty. To disable color output, pass cv::noArray() here.CV_EXPORTS void triangleRasterize(InputArray vertices, InputArray indices, InputArray colors, InputArray cameraPose, float fovY, float zNear, float zFar,int width, int height, bool useFlatShading, OutputArray depthBuf=noArray(), OutputArray colorBuf=noArray());modules/3d/src/rendering.cpp Outdated
| Vec3f position = camera.row(0); | ||
| Vec3f lookat = camera.row(1); | ||
| Vec3f upVector = camera.row(2); | ||
| float fovy = camera.at<float>(3,0), zNear = camera.at<float>(3,1), zFar = camera.at<float>(3,2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please change this according to new function signature
modules/3d/src/rendering.cpp Outdated
| Matx44f mvpMatrix = perspectMatrix * lookAtMatrix; | ||
| Mat depth_buf_mat = depth_buf.getMat(); | ||
| Mat color_buf_mat = color_buf.getMat(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
- Both depthBuf and colorBuf
OutputArrayarguments should be checked for size, type,empty()andneeded() - If they are empty, they should be created with given
widthandheight - If color buffer is not needed then the color rendering should be disabled
- If depth buffer is not needed then we still need it during rendering, just don't pass it outside
modules/3d/src/rendering.cpp Outdated
| returnVec3f(a[0] / length, a[1] / length, a[2] / length); | ||
| } | ||
| Matx44flookAtMatrixCal(const Vec3f& position,const Vec3f& lookat,const Vec3f& upVector) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As I said earlier, this function makes more sense in test/perf files rather than here.
As an option, we can expose it to public interface
modules/3d/src/rendering.cpp Outdated
| if (ACcrossAB.dot(ACcrossAP) >=0 && ABcrossAC.dot(ABcrossAP) >=0) | ||
| { | ||
| float beta =norm(ACcrossAP) /norm(ACcrossAB); | ||
| float gamma =norm(ABcrossAP) /norm(ABcrossAC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
norm(ACcrossAB) == norm(ABcrossAC), this can be optimized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If the triangle is degenerate or close to, we'll get division by zero problem, this should be avoided
modules/3d/src/rendering.cpp Outdated
| if (ACcrossAB.dot(ACcrossAP) >=0 && ABcrossAC.dot(ABcrossAP) >=0) | ||
| { | ||
| float beta =norm(ACcrossAP) /norm(ACcrossAB); | ||
| float gamma =norm(ABcrossAP) /norm(ABcrossAC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The same about div by zero
VIT track(gsoc realtime object tracking model)opencv#1088add an onnx model for opencv vttrack PRopencv#24201
* add converted conformance gemm tests* update to modern googlenet with new outputs* correct filename* replace model link with the one from org gdrive* drop googlenet 2023
samples/opengl/opengl.cpp Outdated
| #include<windows.h> | ||
| #defineWIN32_LEAN_AND_MEAN1 | ||
| #defineNOMINMAX1 | ||
| #include<windows.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
avoid unnecessary changes
modules/3d/src/rendering.cpp Outdated
| @@ -0,0 +1,212 @@ | |||
| #include"precomp.hpp" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Missing license header
modules/3d/src/rendering.cpp Outdated
| namespacecv { | ||
| structTriangle | ||
| { | ||
| Vec4f vertices[3]; | ||
| Vec3f color[3]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
avoid indentation in namespaces (all files)
modules/3d/test/test_rendering.cpp Outdated
| { | ||
| if (debugLevel >0) | ||
| { | ||
| Mat groundTruth =imread("../../../opencv_extra/opencv_extra/testdata/rendering/example_image_depth_1.png"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
findDataFile() should be used.
| structDrawData | ||
| { | ||
| ogl::Arrays arr; | ||
| ogl::Buffer indices; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Don't use tabs in source code.
Follow OpenCV coding style guidelines.
* add expand conformance tests* fix clip-vit-base-head model and its output
Model and test data for the fix of shared const inputs
dnn: download_models script improvementsopencv#1084Added new features:* Use argument parser and new arguments: * `--cache` directory will be tried before downloading from internet * `--dst` change destination directory * `--list` list all model names * `--cleanup` remove all `tar.gz` files afterwards* Add hierarchy to model structure, now there are regular models which can be downloaded directly and archived models which should be downloaded and then required files will be extracted. Archived files will be checked for existence before downloading whole archive - it allows to populate them from cache and avoid storing archives on disk.* Filters are now case-insensitive and use _contains_ rule for matching, multiple filters can be used in the command line, results will be united.* GDrive object simplified to separate method - allows to reduce repeated download code. Better solution would be to extract all download-store-verify functionality to separate class(es), but I decided to postpone this step for the future improvements.* Use `pathlib.Path` for most paths.I have doubts regarding filter arguments: should we keep them positional arguments or change to keyword arguments (e.g. `--filter`)?Examples:```.sh# Populate cache and remove archives afterwards (~4Gb)./download_models.py --dst /data/dnn --cleanup# Download models to the specified folder using cache./download_models.py --cache /data/dnn --dst $HOME/opencv_extra/testdata/dnn# List all models having face in their name./download_models.py --list face```
Extend performance test models
Updated performance test sanity data for PR opencv/opencv:24333.
Add QRCode with byte data and umlaut
Merge withopencv#24386Also noted that random seed is added in this pr, so previous data is regenerated to keep consistency.
…-dnnRemoved torch7 models
savuor commentedOct 27, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Reopened as#24459 |
Triangle rasterization function#24459#24065 reopened since the previous one was automatically closed after rebaseConnected PR with ground truth data: [#1113@extra](opencv/opencv_extra#1113)### Pull Request Readiness ChecklistSee details athttps://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request- [x] I agree to contribute to the project under Apache 2 License.- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV- [x] The PR is proposed to the proper branch- [x] There is a reference to the original bug report and related work- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name.- [x] The feature is well documented and sample code can be built with the project CMake
Pull Request for GSoC 2023 Project5 - Simple Triangle Rendering
This is a pull request for OpenCV 3d module in 5.x branch, which is the work of GSoC 2023 project 5. The changes are demonstrated down below.