Skip to content

Fix VecXX implementation #1851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shimat merged 6 commits into from
Mar 31, 2026
Merged

Fix VecXX implementation #1851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd9bee7
fix VecXX implementation
shimat Mar 31, 2026
4c125ca
add tests
shimat Mar 31, 2026
e5b38c6
fix: apply CodeRabbit auto-fixes
coderabbitai[bot] Mar 31, 2026
b685318
fix tests
shimat Mar 31, 2026
df47d6f
fix warnings
shimat Mar 31, 2026
aca15a7
fix warnings
shimat Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/OpenCvSharp/Modules/core/Struct/Vec/IVec.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public interface IVec;
/// </summary>
/// <typeparam name="TSelf"></typeparam>
/// <typeparam name="TElem"></typeparam>
public interface IVec<TSelf, out TElem> : IVec
public interface IVec<TSelf, TElem> : IVec
where TSelf : IVec
where TElem : unmanaged
{
Expand Down Expand Up @@ -49,5 +49,5 @@ public interface IVec<TSelf, out TElem> : IVec
/// </summary>
/// <param name="i"></param>
/// <returns></returns>
TElem this[int i] { get; }
TElem this[int i] { get; set; }
}
20 changes: 12 additions & 8 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec2b.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices;
using OpenCvSharp.Internal.Util;

#pragma warning disable CA1051

namespace OpenCvSharp;

/// <summary>
/// 2-Tuple of byte (System.Byte)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
[SuppressMessage("Design", "CA1051: Do not declare visible instance fields")]
// ReSharper disable once InconsistentNaming
public struct Vec2b : IVec<Vec2b, byte>, IEquatable<Vec2b>
{
Expand Down Expand Up @@ -130,6 +129,11 @@ public byte this[int i]
// ReSharper restore InconsistentNaming
#pragma warning restore 1591

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 2 elements of this vector.</summary>
public Span<byte> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 2);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec2b other) =>
Item0 == other.Item0 &&
Expand Down Expand Up @@ -160,10 +164,10 @@ public readonly override bool Equals(object? obj)
public readonly override int GetHashCode()
{
#if NETSTANDARD2_0
unchecked
{
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
unchecked
{
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
#else
return HashCode.Combine(Item0, Item1);
#endif
Expand Down
19 changes: 12 additions & 7 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec2d.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace OpenCvSharp;
/// <summary>
/// 2-Tuple of double (System.Double)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
public struct Vec2d : IVec<Vec2d, double>, IEquatable<Vec2d>
{
Expand Down Expand Up @@ -82,6 +81,7 @@ public Vec2d(double item0, double item1)
public static Vec2d operator -(Vec2d a, Vec2d b) => a.Subtract(b);
public static Vec2d operator *(Vec2d a, double alpha) => a.Multiply(alpha);
public static Vec2d operator /(Vec2d a, double alpha) => a.Divide(alpha);
public static Vec2d operator -(Vec2d a) => new(-a.Item0, -a.Item1);
#pragma warning restore 1591

/// <summary>
Expand Down Expand Up @@ -111,6 +111,11 @@ public double this[int i]

#endregion

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 2 elements of this vector.</summary>
public Span<double> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 2);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec2d other) => Item0.Equals(other.Item0) && Item1.Equals(other.Item1);

Expand All @@ -130,23 +135,23 @@ public readonly override bool Equals(object? obj)
/// <param name="a"></param>
/// <param name="b"></param>
/// <returns></returns>
public static bool operator ==(Vec2d a, Vec2d b) => a.Equals(b);
public static bool operator ==(Vec2d a, Vec2d b) => a.Item0 == b.Item0 && a.Item1 == b.Item1;

/// <summary>
/// </summary>
/// <param name="a"></param>
/// <param name="b"></param>
/// <returns></returns>
public static bool operator !=(Vec2d a, Vec2d b) => !(a == b);
public static bool operator !=(Vec2d a, Vec2d b) => a.Item0 != b.Item0 || a.Item1 != b.Item1;

/// <inheritdoc />
public readonly override int GetHashCode()
{
#if NETSTANDARD2_0
unchecked
{
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
unchecked
{
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
#else
return HashCode.Combine(Item0, Item1);
#endif
Expand Down
19 changes: 12 additions & 7 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec2f.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices;

#pragma warning disable CA1051

namespace OpenCvSharp;

/// <summary>
/// 2-Tuple of float (System.Single)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
[SuppressMessage("Design", "CA1051: Do not declare visible instance fields")]
// ReSharper disable once InconsistentNaming
public struct Vec2f : IVec<Vec2f, float>, IEquatable<Vec2f>
{
Expand Down Expand Up @@ -83,6 +82,7 @@ public Vec2f(float item0, float item1)
public static Vec2f operator -(Vec2f a, Vec2f b) => a.Subtract(b);
public static Vec2f operator *(Vec2f a, double alpha) => a.Multiply(alpha);
public static Vec2f operator /(Vec2f a, double alpha) => a.Divide(alpha);
public static Vec2f operator -(Vec2f a) => new(-a.Item0, -a.Item1);
#pragma warning restore 1591

/// <summary>
Expand Down Expand Up @@ -119,6 +119,11 @@ public float this[int i]
// ReSharper restore InconsistentNaming
#pragma warning restore 1591

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 2 elements of this vector.</summary>
public Span<float> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 2);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec2f other) => Item0.Equals(other.Item0) && Item1.Equals(other.Item1);

Expand All @@ -134,14 +139,14 @@ public readonly override bool Equals(object? obj)
/// <param name="a"></param>
/// <param name="b"></param>
/// <returns></returns>
public static bool operator ==(Vec2f a, Vec2f b) => a.Equals(b);
public static bool operator ==(Vec2f a, Vec2f b) => a.Item0 == b.Item0 && a.Item1 == b.Item1;

/// <summary>
/// </summary>
/// <param name="a"></param>
/// <param name="b"></param>
/// <returns></returns>
public static bool operator !=(Vec2f a, Vec2f b) => !(a == b);
public static bool operator !=(Vec2f a, Vec2f b) => a.Item0 != b.Item0 || a.Item1 != b.Item1;
Comment on lines +142 to +149
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In .NET/C#, does float.NaN.Equals(float.NaN)returntruewhilefloat.NaN == float.NaNreturnsfalse?

💡 Result:

Yes, in .NET/C#, float.NaN.Equals(float.NaN) returns true while float.NaN == float.NaN returns false. This behavior follows the IEEE 754 floating-point standard for the == operator, which treats NaN as not equal to anything, including itself. However, the Equals method on float (Single) is overridden to return true for two NaNs to satisfy the Object.Equals contract requirements (reflexivity: x.Equals(x) must be true, and consistency for use in collections like dictionaries). Official Microsoft documentation for Single.NaN explicitly demonstrates this: Console.WriteLine("NaN == NaN: {0}", Single.NaN == Single.NaN); // False Console.WriteLine("NaN.Equals(NaN): {0}", Single.NaN.Equals(Single.NaN)); // True The same applies to double.NaN. Use float.IsNaN to detect NaN values, as direct comparisons always fail.

Citations:

Align ==/!= operators with Equals method to handle NaN consistently.

The operators at lines 142–149 use raw float comparisons (==/!=), which follow IEEE 754 semantics where NaN == NaN is false. However, Equals(Vec2f) at line 128 uses Single.Equals, which returns true for two NaN values. This creates an inconsistency: (NaN, 1).Equals((NaN, 1)) returns true, but (NaN, 1) == (NaN, 1) returns false.

Either delegate the operators to Equals:

-    public static bool operator ==(Vec2f a, Vec2f b) => a.Item0 == b.Item0 && a.Item1 == b.Item1;
-    public static bool operator !=(Vec2f a, Vec2f b) => a.Item0 != b.Item0 || a.Item1 != b.Item1;
+    public static bool operator ==(Vec2f a, Vec2f b) => a.Equals(b);
+    public static bool operator !=(Vec2f a, Vec2f b) => !a.Equals(b);

Or update Equals to use raw comparisons. Choose one approach to ensure consistency.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/OpenCvSharp/Modules/core/Struct/Vec/Vec2f.cs` around lines 142 - 149, The
== and != operators in Vec2f must be made consistent with the existing
Equals(Vec2f) behavior (which treats NaN pairs as equal); change operator == to
return a.Equals(b) and operator != to return !a.Equals(b) so both operator
overloads delegate to the Vec2f.Equals(Vec2f) implementation.


/// <inheritdoc />
public readonly override int GetHashCode()
Expand All @@ -152,7 +157,7 @@ public readonly override int GetHashCode()
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
#else
return HashCode.Combine(Item0, Item1);
return HashCode.Combine(Item0, Item1);
#endif
}

Expand Down
21 changes: 13 additions & 8 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices;
using OpenCvSharp.Internal.Util;

#pragma warning disable CA1051

namespace OpenCvSharp;

/// <summary>
/// 2-Tuple of int (System.Int32)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
[SuppressMessage("Design", "CA1051: Do not declare visible instance fields")]
// ReSharper disable once InconsistentNaming
public struct Vec2i : IVec<Vec2i, int>, IEquatable<Vec2i>
{
Expand Down Expand Up @@ -84,6 +83,7 @@ public Vec2i(int item0, int item1)
public static Vec2i operator -(Vec2i a, Vec2i b) => a.Subtract(b);
public static Vec2i operator *(Vec2i a, double alpha) => a.Multiply(alpha);
public static Vec2i operator /(Vec2i a, double alpha) => a.Divide(alpha);
public static Vec2i operator -(Vec2i a) => new(SaturateCast.ToInt32(-(long)a.Item0), SaturateCast.ToInt32(-(long)a.Item1));
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In C#, are intadditions and subtractions evaluated in uncheckedintby default, so0 - int.MinValue wraps before the value is passed to another method unless one operand is widened first?

💡 Result:

Yes, in C#, additions and subtractions on int operands are evaluated in the default unchecked context, causing wraparound on overflow. Specifically, 0 - int.MinValue computes to int.MaxValue due to two's complement wraparound (-(-2147483648) = 2147483647, which fits in int), with no overflow occurring. This wrapped int result is then passed to another method unchanged unless widening (e.g., to long) occurs first. There is no automatic widening of both operands to a larger type like long for binary int operations; binary numeric promotions only apply when mixing certain smaller types or with floating-point, but two ints stay as int. The default is unchecked unless the /checked+ compiler option is set or explicit checked blocks are used. Constant expressions are checked at compile-time, but runtime non-constant int arithmetic defaults to unchecked wraparound.

Citations:

🏁 Script executed:

# First, check if the file exists and get its basic info
ls -la src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs
wc -l src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs

Repository: shimat/opencvsharp

Length of output: 212

🏁 Script executed:

# Read the file to see line 86 and surrounding context
cat -n src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs | sed -n '80,95p'

Repository: shimat/opencvsharp

Length of output: 803

🏁 Script executed:

# Find the Add and Subtract methods in the file
cat -n src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs | grep -A 2 "public readonly Vec2i Add\|public readonly Vec2i Subtract"

Repository: shimat/opencvsharp

Length of output: 419

🏁 Script executed:

# Get a broader view to understand the structure
cat -n src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs | head -170

Repository: shimat/opencvsharp

Length of output: 6027

Match the new unary saturation behavior in the binary +/- paths.

Line 86 widens to long before negation, so -new Vec2i(int.MinValue, 0) saturates correctly. The Add and Subtract methods at lines 50–52 and 59–61 perform arithmetic in int context, causing overflow to wrap before SaturateCast runs, producing inconsistent boundary results.

Widen both operands to long in Add and Subtract to match the unary negation behavior:

Minimal fix 
 public readonly Vec2i Add(Vec2i other) => new(
-    SaturateCast.ToInt32(Item0 + other.Item0),
-    SaturateCast.ToInt32(Item1 + other.Item1));
+    SaturateCast.ToInt32((long)Item0 + other.Item0),
+    SaturateCast.ToInt32((long)Item1 + other.Item1));

 public readonly Vec2i Subtract(Vec2i other) => new(
-    SaturateCast.ToInt32(Item0 - other.Item0),
-    SaturateCast.ToInt32(Item1 - other.Item1));
+    SaturateCast.ToInt32((long)Item0 - other.Item0),
+    SaturateCast.ToInt32((long)Item1 - other.Item1));
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/OpenCvSharp/Modules/core/Struct/Vec/Vec2i.cs` at line 86, The Add and
Subtract methods in Vec2i perform arithmetic in int context which allows
overflow before passing results to SaturateCast; to match the unary operator-
behavior you must widen both operands to long inside Add and Subtract (e.g.,
cast Item0/Item1 to long, perform the + or - on longs) and then call
SaturateCast.ToInt32 on the long result so boundary cases (like int.MinValue
negation) saturate correctly; update Vec2i.Add, Vec2i.Subtract (and ensure
consistency with operator - and SaturateCast usage) to use long arithmetic.

#pragma warning restore 1591

/// <summary>
Expand Down Expand Up @@ -120,6 +120,11 @@ public int this[int i]
// ReSharper restore InconsistentNaming
#pragma warning restore 1591

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 2 elements of this vector.</summary>
public Span<int> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 2);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec2i other) =>
Item0 == other.Item0 &&
Expand Down Expand Up @@ -150,10 +155,10 @@ public readonly override bool Equals(object? obj)
public readonly override int GetHashCode()
{
#if NETSTANDARD2_0
unchecked
{
return (Item0 * 397) ^ Item1;
}
unchecked
{
return (Item0 * 397) ^ Item1;
}
#else
return HashCode.Combine(Item0, Item1);
#endif
Expand Down
9 changes: 7 additions & 2 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec2s.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ namespace OpenCvSharp;
/// <summary>
/// 2-Tuple of short (System.Int16)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
// ReSharper disable once InconsistentNaming
public struct Vec2s : IVec<Vec2s, short>, IEquatable<Vec2s>
Expand Down Expand Up @@ -84,6 +83,7 @@ public Vec2s(short item0, short item1)
public static Vec2s operator -(Vec2s a, Vec2s b) => a.Subtract(b);
public static Vec2s operator *(Vec2s a, double alpha) => a.Multiply(alpha);
public static Vec2s operator /(Vec2s a, double alpha) => a.Divide(alpha);
public static Vec2s operator -(Vec2s a) => new(SaturateCast.ToInt16(-a.Item0), SaturateCast.ToInt16(-a.Item1));
#pragma warning restore 1591

/// <summary>
Expand Down Expand Up @@ -122,6 +122,11 @@ public short this[int i]
// ReSharper restore InconsistentNaming
#pragma warning restore 1591

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 2 elements of this vector.</summary>
public Span<short> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 2);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec2s other) => Item0 == other.Item0 && Item1 == other.Item1;

Expand Down Expand Up @@ -155,7 +160,7 @@ public readonly override int GetHashCode()
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
#else
return HashCode.Combine(Item0, Item1);
return HashCode.Combine(Item0, Item1);
#endif
}

Expand Down
14 changes: 9 additions & 5 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec2w.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices;
using OpenCvSharp.Internal.Util;

#pragma warning disable CA1051

namespace OpenCvSharp;

/// <summary>
/// 2-Tuple of ushort (System.UInt16)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
[SuppressMessage("Design", "CA1051: Do not declare visible instance fields")]
// ReSharper disable once InconsistentNaming
public struct Vec2w : IVec<Vec2w, ushort>, IEquatable<Vec2w>
{
Expand Down Expand Up @@ -123,6 +122,11 @@ public ushort this[int i]
// ReSharper restore InconsistentNaming
#pragma warning restore 1591

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 2 elements of this vector.</summary>
public Span<ushort> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 2);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec2w other) => Item0 == other.Item0 && Item1 == other.Item1;

Expand Down Expand Up @@ -156,7 +160,7 @@ public readonly override int GetHashCode()
return (Item0.GetHashCode() * 397) ^ Item1.GetHashCode();
}
#else
return HashCode.Combine(Item0, Item1);
return HashCode.Combine(Item0, Item1);
#endif
}

Expand Down
14 changes: 9 additions & 5 deletions src/OpenCvSharp/Modules/core/Struct/Vec/Vec3b.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices;
using OpenCvSharp.Internal.Util;

#pragma warning disable CA1051

namespace OpenCvSharp;

/// <summary>
/// 3-Tuple of byte (System.Byte)
/// </summary>
[Serializable]
[StructLayout(LayoutKind.Sequential)]
[SuppressMessage("Design", "CA1051: Do not declare visible instance fields")]
// ReSharper disable once InconsistentNaming
public struct Vec3b : IVec<Vec3b, byte>, IEquatable<Vec3b>
{
Expand Down Expand Up @@ -137,6 +136,11 @@ public byte this[int i]
// ReSharper restore InconsistentNaming
#pragma warning restore 1591

#if !NETSTANDARD2_0
/// <summary>Returns a <see cref="Span{T}"/> over the 3 elements of this vector.</summary>
public Span<byte> AsSpan() => MemoryMarshal.CreateSpan(ref Item0, 3);
#endif

/// <inheritdoc />
public readonly bool Equals(Vec3b other) =>
Item0 == other.Item0 &&
Expand Down Expand Up @@ -176,7 +180,7 @@ public readonly override int GetHashCode()
return hashCode;
}
#else
return HashCode.Combine(Item0, Item1, Item2);
return HashCode.Combine(Item0, Item1, Item2);
#endif
}

Expand Down
Loading
Loading