Repository navigation
Add LINEMOD wrappers for rgbd module - #2042
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds OpenCV RGB-D LINEMOD support across native exports, C# interop bindings, modality and detector wrappers, LINEMOD data types, persistence APIs, and integration tests. ChangesRGB-D LINEMOD support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant LinemodDetector
participant NativeMethods
participant Detector
Application->>LinemodDetector: AddTemplate or Match
LinemodDetector->>NativeMethods: Marshal sources and invoke binding
NativeMethods->>Detector: Execute LINEMOD detector operation
Detector-->>NativeMethods: Return templates, matches, and quantized images
NativeMethods-->>LinemodDetector: Return native results
LinemodDetector-->>Application: Return managed LINEMOD values
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/OpenCvSharp/Modules/rgbd/LinemodTypes.cs (1)
6-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
LinemodTemplate's generated equality is broken by theFeaturesarray.Records auto-generate
Equals/GetHashCodefrom their properties, butLinemodFeature[]compares by reference, not by content. Two templates with identical feature data will report as unequal (and hash differently) unless they share the exact same array instance — surprising for arecordwhose whole point is usually value semantics.♻️ Suggested fix: override equality members explicitly
-public sealed record LinemodTemplate(int Width, int Height, int PyramidLevel, LinemodFeature[] Features); +public sealed record LinemodTemplate(int Width, int Height, int PyramidLevel, LinemodFeature[] Features) +{ + public bool Equals(LinemodTemplate? other) => + other is not null && + Width == other.Width && Height == other.Height && PyramidLevel == other.PyramidLevel && + Features.AsSpan().SequenceEqual(other.Features); + + public override int GetHashCode() + { + var hash = new HashCode(); + hash.Add(Width); hash.Add(Height); hash.Add(PyramidLevel); + foreach (var f in Features) hash.Add(f); + return hash.ToHashCode(); + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenCvSharp/Modules/rgbd/LinemodTypes.cs` around lines 6 - 7, Update LinemodTemplate’s equality implementation to compare Features element-by-element and compute a matching content-based hash, while retaining Width, Height, and PyramidLevel in value comparison. Override the generated equality members explicitly so identical feature data is equal even when stored in different array instances.src/OpenCvSharp/Modules/rgbd/LinemodDetector.cs (1)
9-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
SetSafeHandleconstruction logic.The private
LinemodDetector(IntPtr ptr), the parameterlessLinemodDetector(), and the modalities/tPyramidconstructor each build their ownOpenCvPtrSafeHandlewith the same delete delegate. Route the public constructors through the private one to keep the disposal wiring in one place.♻️ Suggested refactor
- public LinemodDetector() - { - NativeMethods.HandleException(NativeMethods.rgbd_linemod_Detector_newEmpty(out var ptr)); - SetSafeHandle(new OpenCvPtrSafeHandle(ptr, true, - static p => NativeMethods.HandleException(NativeMethods.rgbd_linemod_Detector_delete(p)))); - } + public LinemodDetector() : this(CreateEmpty()) { } + + private static IntPtr CreateEmpty() + { + NativeMethods.HandleException(NativeMethods.rgbd_linemod_Detector_newEmpty(out var ptr)); + return ptr; + } public LinemodDetector(IEnumerable<LinemodModality> modalities, IEnumerable<int> tPyramid) + : this(CreateNative(modalities, tPyramid, out var mods)) { - ArgumentNullException.ThrowIfNull(modalities); - ArgumentNullException.ThrowIfNull(tPyramid); - var mods = modalities.ToArray(); - var steps = tPyramid.ToArray(); - if (mods.Length == 0 || steps.Length == 0) - throw new ArgumentException("At least one modality and pyramid sampling step are required."); - var pointers = mods.Select(m => m.SmartPtr).ToArray(); - NativeMethods.HandleException(NativeMethods.rgbd_linemod_Detector_new( - pointers, pointers.Length, steps, steps.Length, out var ptr)); - SetSafeHandle(new OpenCvPtrSafeHandle(ptr, true, - static p => NativeMethods.HandleException(NativeMethods.rgbd_linemod_Detector_delete(p)))); foreach (var mod in mods) GC.KeepAlive(mod); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenCvSharp/Modules/rgbd/LinemodDetector.cs` around lines 9 - 36, Centralize disposal-handle setup in the private LinemodDetector(IntPtr ptr) constructor. Update the parameterless and modalities/tPyramid constructors to delegate to it after creating the native detector pointer, removing their duplicated SetSafeHandle/OpenCvPtrSafeHandle construction while preserving validation, native creation, and GC.KeepAlive behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/OpenCvSharp/Modules/rgbd/LinemodModality.cs`:
- Around line 52-78: Add ThrowIfDisposed() at the start of Modality.Process,
Read, and Write, before accessing Handle or invoking native methods; apply the
same check to the ColorGradient.Parameters and DepthNormal.Parameters getters.
Preserve the existing argument validation, native calls, and KeepAlive behavior.
---
Nitpick comments:
In `@src/OpenCvSharp/Modules/rgbd/LinemodDetector.cs`:
- Around line 9-36: Centralize disposal-handle setup in the private
LinemodDetector(IntPtr ptr) constructor. Update the parameterless and
modalities/tPyramid constructors to delegate to it after creating the native
detector pointer, removing their duplicated SetSafeHandle/OpenCvPtrSafeHandle
construction while preserving validation, native creation, and GC.KeepAlive
behavior.
In `@src/OpenCvSharp/Modules/rgbd/LinemodTypes.cs`:
- Around line 6-7: Update LinemodTemplate’s equality implementation to compare
Features element-by-element and compute a matching content-based hash, while
retaining Width, Height, and PyramidLevel in value comparison. Override the
generated equality members explicitly so identical feature data is equal even
when stored in different array instances.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 88a59fac-f496-4328-83bd-1d867924e459
📒 Files selected for processing (9)
AGENTS.mdsrc/OpenCvSharp/Internal/PInvoke/NativeMethods/rgbd/NativeMethods_rgbd_linemod.cssrc/OpenCvSharp/Modules/rgbd/LinemodDetector.cssrc/OpenCvSharp/Modules/rgbd/LinemodModality.cssrc/OpenCvSharp/Modules/rgbd/LinemodTypes.cssrc/OpenCvSharpExtern/include_opencv.hsrc/OpenCvSharpExtern/rgbd.cppsrc/OpenCvSharpExtern/rgbd_linemod.htest/OpenCvSharp.Tests/rgbd/LinemodTest.cs
Owner
Author
|
Addressed the CodeRabbit review in b0f367c: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cv::linemod::Detector,Modality,ColorGradient,DepthNormal, andQuantizedPyramidAGENTS.mdthat shares the existing Copilot development instructionsThis implements the LINEMOD portion of #2018. The KinFu family remains for follow-up PRs.
Testing
cmake --build src\build --config Release -j 8dotnet build src\OpenCvSharp\OpenCvSharp.csproj -c Release --no-restoredotnet test test\OpenCvSharp.Tests\OpenCvSharp.Tests.csproj -c Release --no-restore --filter FullyQualifiedName~LinemodTestSummary by CodeRabbit