Skip to content

Commit 8297ae3

Browse files
committed
fix(math): resolve strict weak ordering violations in comparison operators
1 parent 62f7cf7 commit 8297ae3

6 files changed

Lines changed: 80 additions & 26 deletions

File tree

pathfinder/common/color.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,14 @@ struct ColorU {
9797

9898
ColorU lerp(const ColorU& other, float t) const;
9999

100+
bool operator==(const ColorU& rhs) const {
101+
return to_u32() == rhs.to_u32();
102+
}
103+
104+
bool operator!=(const ColorU& rhs) const {
105+
return to_u32() != rhs.to_u32();
106+
}
107+
100108
bool operator<(const ColorU& rhs) const {
101109
return to_u32() < rhs.to_u32();
102110
}

pathfinder/common/math/mat2.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct Mat2 {
7979

8080
// For being used as ordered key.
8181
bool operator<(const Mat2 &b) const {
82-
return v[0] < b.v[0] && v[1] < b.v[1] && v[2] < b.v[2] && v[3] < b.v[3];
82+
return std::tie(v[0], v[1], v[2], v[3]) < std::tie(b.v[0], b.v[1], b.v[2], b.v[3]);
8383
}
8484

8585
Vec2F operator*(const Vec2F &other) const {

pathfinder/common/math/transform2.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,15 @@ struct Transform2 {
101101

102102
// For being used as ordered key.
103103
bool operator<(const Transform2 &rhs) const {
104-
return matrix < rhs.matrix && vector < rhs.vector;
104+
return std::tie(matrix, vector) < std::tie(rhs.matrix, rhs.vector);
105+
}
106+
107+
bool operator==(const Transform2 &rhs) const {
108+
return matrix == rhs.matrix && vector == rhs.vector;
109+
}
110+
111+
bool operator!=(const Transform2 &rhs) const {
112+
return !(*this == rhs);
105113
}
106114
};
107115

pathfinder/common/math/vec2.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <algorithm>
55
#include <cmath>
66
#include <sstream>
7+
#include <tuple>
78

89
#include "../logger.h"
910

@@ -133,13 +134,13 @@ struct Vec2 {
133134
return x == b.x && y == b.y;
134135
}
135136

136-
// For being used as ordered key.
137-
bool operator<(const Vec2 &b) const {
138-
return x < b.x && y < b.y;
137+
bool operator!=(const Vec2 &b) const {
138+
return !(*this == b);
139139
}
140140

141-
bool operator!=(const Vec2 &b) const {
142-
return x != b.x || y != b.y;
141+
// For being used as ordered key.
142+
bool operator<(const Vec2 &b) const {
143+
return std::tie(x, y) < std::tie(b.x, b.y);
143144
}
144145

145146
void operator+=(const Vec2 &b) {

pathfinder/core/data/line_segment.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,17 @@ struct LineSegmentF {
102102

103103
// For being used as ordered key.
104104
bool operator<(const LineSegmentF &rhs) const {
105-
bool res = value.get<0>() < rhs.value.get<0>();
106-
res = res && value.get<1>() < rhs.value.get<1>();
107-
res = res && value.get<2>() < rhs.value.get<2>();
108-
res = res && value.get<3>() < rhs.value.get<3>();
109-
return res;
105+
return std::forward_as_tuple(value.get<0>(), value.get<1>(), value.get<2>(), value.get<3>()) <
106+
std::forward_as_tuple(rhs.value.get<0>(), rhs.value.get<1>(), rhs.value.get<2>(), rhs.value.get<3>());
107+
}
108+
109+
bool operator==(const LineSegmentF &rhs) const {
110+
return std::forward_as_tuple(value.get<0>(), value.get<1>(), value.get<2>(), value.get<3>()) ==
111+
std::forward_as_tuple(rhs.value.get<0>(), rhs.value.get<1>(), rhs.value.get<2>(), rhs.value.get<3>());
112+
}
113+
114+
bool operator!=(const LineSegmentF &rhs) const {
115+
return !(*this == rhs);
110116
}
111117
};
112118

pathfinder/core/paint/gradient.h

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,21 @@ struct ColorStop {
2020

2121
/// The color of the gradient stop.
2222
ColorU color;
23+
24+
bool operator<(const ColorStop &rhs) const {
25+
if (offset != rhs.offset) {
26+
return offset < rhs.offset;
27+
}
28+
return color < rhs.color;
29+
}
30+
31+
bool operator==(const ColorStop &rhs) const {
32+
return offset == rhs.offset && color == rhs.color;
33+
}
34+
35+
bool operator!=(const ColorStop &rhs) const {
36+
return !(*this == rhs);
37+
}
2338
};
2439

2540
struct GradientRadial {
@@ -130,22 +145,38 @@ class Gradient {
130145

131146
// For being used as ordered key.
132147
bool operator<(const Gradient &rhs) const {
133-
if (wrap == rhs.wrap) {
134-
if (geometry.type == rhs.geometry.type) {
135-
if (geometry.type == GradientGeometry::Type::Linear) {
136-
return geometry.linear < rhs.geometry.linear;
137-
} else {
138-
bool res = geometry.radial.line < rhs.geometry.radial.line;
139-
res = res && geometry.radial.radii < rhs.geometry.radial.radii;
140-
res = res && geometry.radial.transform < rhs.geometry.radial.transform;
141-
return res;
142-
}
143-
} else {
144-
return geometry.type < rhs.geometry.type;
145-
}
146-
} else {
148+
// 1. Compare the outer wrap mode first.
149+
if (wrap != rhs.wrap) {
147150
return wrap < rhs.wrap;
148151
}
152+
153+
// 2. Compare the gradient geometry type.
154+
if (geometry.type != rhs.geometry.type) {
155+
return geometry.type < rhs.geometry.type;
156+
}
157+
158+
// 3. Branch based on the type and perform strict lexicographical comparison.
159+
if (geometry.type == GradientGeometry::Type::Linear) {
160+
// Standard total-ordering chain for linear gradient.
161+
if (geometry.linear < rhs.geometry.linear) return true;
162+
if (rhs.geometry.linear < geometry.linear) return false;
163+
} else { // Radial
164+
// CRITICAL FIX: Replaced the incorrect '&&' logic with lexicographical comparison
165+
// using std::forward_as_tuple to maintain Strict Weak Ordering for rvalues/lvalues.
166+
auto tuple_lhs =
167+
std::forward_as_tuple(geometry.radial.line, geometry.radial.radii, geometry.radial.transform);
168+
auto tuple_rhs = std::forward_as_tuple(rhs.geometry.radial.line,
169+
rhs.geometry.radial.radii,
170+
rhs.geometry.radial.transform);
171+
if (tuple_lhs != tuple_rhs) {
172+
return tuple_lhs < tuple_rhs;
173+
}
174+
}
175+
176+
// 4. CRITICAL FIX: Compare the 'stops' vector (color stops).
177+
// If all geometry attributes are identical, gradients with different colors must not be treated as equivalent.
178+
// std::vector's operator< automatically performs a sequential lexicographical comparison on its elements.
179+
return stops < rhs.stops;
149180
}
150181

151182
private:

0 commit comments

Comments
 (0)