Skip to content

Conversation

odatsk
Copy link

@odatsk odatsk commented Sep 4, 2020

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 OpenCV (BSD) 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

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

Please take a look on comments below.

@@ -0,0 +1,8 @@
# Default ignored files
Copy link
Member

Choose a reason for hiding this comment

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

.idea

Please remove IDE related files from the Git.
Configure "global .gitignore" for handling that in the future.

Rebase / squash all commits into one after that (to cleanup Git history).

@@ -1,2 +1,3 @@
set(the_description "Structured Light API")
ocv_define_module(structured_light opencv_core opencv_imgproc opencv_calib3d opencv_phase_unwrapping OPTIONAL opencv_viz WRAP python java objc)

Copy link
Member

Choose a reason for hiding this comment

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

Remove unnecessary changes from PR

@@ -0,0 +1,55 @@
<?xml version="1.0"?>
Copy link
Member

Choose a reason for hiding this comment

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

modules/structured_light/images

Perhaps should go into modules/structured_light/samples/data instead.



#ifndef OPENCV_structured_light_HPP
#define OPENCV_structured_light_HPP
Copy link
Member

Choose a reason for hiding this comment

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

capital letters
add _SLMONO_


namespace cv{
namespace structured_light{
class CV_EXPORTS StructuredLightMono : public virtual Algorithm
Copy link
Member

Choose a reason for hiding this comment

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

virtual

Do we really need this here?

namespace structured_light{

//compute atan2 for object and reference images
void computeAtanDiff(InputOutputArrayOfArrays src, OutputArray dst);
Copy link
Member

Choose a reason for hiding this comment

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

Avoid indentation in namespaces.


int calibrate( int argc, char **argv )
{
VideoCapture cap(CAP_PVAPI);
Copy link
Member

Choose a reason for hiding this comment

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

PVAPI

Can other backends work here?

void StructuredLightMono::removeShadows(InputOutputArrayOfArrays refs, InputOutputArrayOfArrays imgs)
{
vector<Mat>& refs_ = *(vector<Mat>*)refs.getObj();
vector<Mat>& imgs_ = *(vector<Mat>*)imgs.getObj();
Copy link
Member

Choose a reason for hiding this comment

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

This code is not safe.
Checks should be added.

@@ -0,0 +1,242 @@
#include <opencv2/structured_light/slmono.hpp>
Copy link
Member

Choose a reason for hiding this comment

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

Please use short OpenCV license header (in all new .hpp/.cpp files except samples):

// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html.

https://github.com/opencv/opencv/wiki/Coding_Style_Guide#file-structure

@@ -0,0 +1,252 @@
#include "opencv2/structured_light/slmono_utils.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

Include "precomp.hpp" first in all src/*.cpp files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants