Add LINEMOD wrappers for rgbd module#2042
Open
shimat wants to merge 2 commits into
Open
Conversation
Contributor
📝 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 OpenCVLINEMOD
Application->>LinemodDetector: AddTemplate or Match
LinemodDetector->>NativeMethods: Marshal sources and invoke binding
NativeMethods->>OpenCVLINEMOD: Execute LINEMOD detector operation
OpenCVLINEMOD-->>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
Comment on lines
+52
to
+78
| /// <summary>Forms a quantized image pyramid from a source image.</summary> | ||
| public QuantizedPyramid Process(Mat src, Mat? mask = null) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(src); | ||
| src.ThrowIfDisposed(); | ||
| mask?.ThrowIfDisposed(); | ||
| NativeMethods.HandleException(NativeMethods.rgbd_linemod_Modality_process( | ||
| Handle, src.Handle, mask?.Handle ?? OpenCvSafeHandle.Null, out var ptr)); | ||
| GC.KeepAlive(src); GC.KeepAlive(mask); | ||
| return new QuantizedPyramid(ptr); | ||
| } | ||
|
|
||
| /// <summary>Loads modality parameters from a file node.</summary> | ||
| public void Read(FileNode node) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(node); | ||
| NativeMethods.HandleException(NativeMethods.rgbd_linemod_Modality_read(Handle, node.Handle)); | ||
| GC.KeepAlive(node); | ||
| } | ||
|
|
||
| /// <summary>Writes modality parameters to file storage.</summary> | ||
| public void Write(FileStorage storage) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(storage); | ||
| NativeMethods.HandleException(NativeMethods.rgbd_linemod_Modality_write(Handle, storage.Handle)); | ||
| GC.KeepAlive(storage); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Missing ThrowIfDisposed() before native calls.
Process, Read, Write, and the ColorGradient/DepthNormal Parameters getters access Handle and invoke native P/Invoke calls without checking disposal state, unlike Name (line 44-45) and every method in QuantizedPyramid (lines 167, 176, 188) in this same file. Calling these on a disposed modality will pass a stale/invalid handle to native code instead of raising a clean ObjectDisposedException.
🛡️ Proposed fix
public QuantizedPyramid Process(Mat src, Mat? mask = null)
{
+ ThrowIfDisposed();
ArgumentNullException.ThrowIfNull(src);
src.ThrowIfDisposed();
mask?.ThrowIfDisposed();
...
public void Read(FileNode node)
{
+ ThrowIfDisposed();
ArgumentNullException.ThrowIfNull(node);
...
public void Write(FileStorage storage)
{
+ ThrowIfDisposed();
ArgumentNullException.ThrowIfNull(storage);
...
private (float Weak, nuint Count, float Strong) Parameters
{
get
{
+ ThrowIfDisposed();
NativeMethods.HandleException(NativeMethods.rgbd_linemod_ColorGradient_get((apply the analogous change to DepthNormal.Parameters.)
As per path instructions: "use the documented Algorithm ownership, disposal, P/Invoke, null-checking, disposal-checking, KeepAlive, OutputArray Fix, and vector-return patterns."
Also applies to: 95-103, 129-137
🤖 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/LinemodModality.cs` around lines 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.
Source: Path instructions
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
New Features
Tests