Skip to content

Port SegmentationImage to the new image archetype style#6928

Merged
emilk merged 7 commits intomainfrom
emilk/segmentation-image
Jul 18, 2024
Merged

Port SegmentationImage to the new image archetype style#6928
emilk merged 7 commits intomainfrom
emilk/segmentation-image

Conversation

@emilk
Copy link
Copy Markdown
Member

@emilk emilk commented Jul 17, 2024

What

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@emilk emilk added 🚜 refactor Change the code, not the functionality include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Jul 17, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 17, 2024

Deployed docs

Commit Link
0e91315 https://landing-5zpji93d0-rerun.vercel.app/docs

@emilk emilk marked this pull request as draft July 17, 2024 15:34
@Wumpf Wumpf self-requested a review July 17, 2024 15:58
@emilk emilk marked this pull request as ready for review July 17, 2024 16:00
return Err(ImageConstructionError::BadImageShape(tensor_data.shape));
}

let (blob, data_type) = match tensor_data.buffer {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe now is the time to add a conversion function that is shared between segmentation_image_ext and depth_image_ext?

reinterpret_cast<const uint8_t*>(pixels), resolution_, get_data_type(pixels)} {}

/// New segmentation image from tensor data.
/// Row-major.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these are pretty sporadic, would be nice to write something nicer imho. Same on DepthImage, but didn't pay enough attention last time.

  • what is row major?
  • what does "borrows" mean? That's not a common concept in C++ and the implication (crashes at random) needs to be clarified

Copy link
Copy Markdown
Member Author

@emilk emilk Jul 18, 2024

Choose a reason for hiding this comment

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

I'll add a TODO to #6844

@emilk emilk mentioned this pull request Jul 18, 2024
1 task
@emilk emilk merged commit a8bf4dd into main Jul 18, 2024
@emilk emilk deleted the emilk/segmentation-image branch July 18, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 🚜 refactor Change the code, not the functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants