Skip to content

Feat/improve 2d3d reproj#130

Open
yzhou-bcom wants to merge 4 commits into
masterfrom
feat/improve-2d3d-reproj
Open

Feat/improve 2d3d reproj#130
yzhou-bcom wants to merge 4 commits into
masterfrom
feat/improve-2d3d-reproj

Conversation

@yzhou-bcom
Copy link
Copy Markdown
Contributor

No description provided.

@yzhou-bcom yzhou-bcom requested a review from jim-bcom May 26, 2026 14:38
Copy link
Copy Markdown
Contributor

@jim-bcom jim-bcom left a comment

Choose a reason for hiding this comment

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

Approved, just some minor questions.

/// @param[in] maskCollection input list of masks
/// @param[out] semanticIds estimated semantic ids and probabilities for each input cloud point
/// @return FrameworkReturnCode::_SUCCESS (semantic id estimated successfully) otherwise FrameworkReturnCode::_ERROR_ (failure)
virtual FrameworkReturnCode estimate(const std::vector<SRef<SolAR::datastructure::CloudPoint>>& points,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some question to better understand:

  • Why has this method been added?
  • Why this one takes points as const ? In the current implementation, the other methods calls this one and uses semanticIds to update its pointCloud. Is it because the first takes a whole point cloud whereas here it can be a subset of cloud points...?
  • For the other method, aren't we interested in getting semanticIds as well ? It could be optional, for example, the argument could be added as a pointer to the vector, with nullptr as a default value?
estimate(SRef<SolAR::datastructure::PointCloud> pointCloud,
            std::vector<std::vector<std::pair<int16_t, float>>>* semanticIds = nullptr);

If it's null, you create and use it internally, otherwise, you use the given instance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this new method has been added to be able to output multiple semantic Ids for the same cloud point. previously, only the most probable semantic Id is set to cloud point via cloudPoint->setSemanticId()

the new method does not call setSemanticId() inside and the input list of points will not be modified

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants