tier4 / AutowareArchitectureProposal.iv

AutowareArchitectureProposal is meta repository of this repository.
Apache License 2.0
139 stars 110 forks source link

Why are we calculating centroid point when we will be calculating cv::minEnclosingCircle and it outputs centroid in 2D? #145

Closed yucedagonurcan closed 3 years ago

yucedagonurcan commented 3 years ago

Hello,

Like I asked already on header, I don't think we need to calculate centroid in 3D (and loop through all the points), because cv::minEnclosingCircle will give the centroid in 2D already and we can calculate the centroid in 3D by:

// calc min and max z for cylinder length
  double min_z = 0;
  double max_z = 0;
  for (size_t i = 0; i < cluster.size(); ++i) {
    if (cluster.at(i).z < min_z || i == 0) min_z = cluster.at(i).z;
    if (max_z < cluster.at(i).z || i == 0) max_z = cluster.at(i).z;
  }
// calc circumscribed circle on x-y plane
  cv::Mat_<float> cv_points((int)cluster.size(), 2);
  for (size_t i = 0; i < cluster.size(); ++i) {
    cv_points(i, 0) = cluster.at(i).x;  // x
    cv_points(i, 1) = cluster.at(i).y;  // y
  }
  cv::Point2f center;
  float radius;
  cv::minEnclosingCircle(cv::Mat(cv_points).reshape(2), center, radius);
  centroid.x = center.x;
  centroid.y = center.y;
  centroid.z = std::max((max_z - min_z), ep)/2;

https://github.com/tier4/Pilot.Auto/blob/0f3dee10dd4c22fddbee44662bd520e5a6d87ef7/perception/object_recognition/detection/shape_estimation/src/model/normal/cylinder.cpp#L39-L46

https://github.com/tier4/Pilot.Auto/blob/0f3dee10dd4c22fddbee44662bd520e5a6d87ef7/perception/object_recognition/detection/shape_estimation/src/model/normal/cylinder.cpp#L64

yukkysaito commented 3 years ago

thank you for comment. As you said, there is no need to calculate centroid. Basically, there is no place where the Z value is used, so it will not affect. I'll fix it here.

yukkysaito commented 3 years ago

This is reflected in the internal version, so I think it will be fixed in v0.9.0.