Skip to content

Commit e35c685

Browse files
committed
Remove NSSize non-negative guarantee
We couldn't really enforce it anyhow, and it was just annoying to work with
1 parent 025b5be commit e35c685

File tree

2 files changed

+71
-50
lines changed

2 files changed

+71
-50
lines changed

objc2/CHANGELOG_FOUNDATION.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
2020
* Added `CGSize`, `CGPoint` and `CGRect` (just aliases to equivalent
2121
`NS`-types, but helps readability).
2222

23+
### Changed
24+
* **BREAKING**: `NSSize::new` no longer requires it's arguments to be
25+
non-negative. Use `NSSize::abs` or `NSRect::standardize` if the API you're
26+
binding to requires a non-negative size.
27+
2328

2429
## objc2 0.3.0-beta.2 - 2022-08-28
2530

objc2/src/foundation/geometry.rs

Lines changed: 66 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,10 @@ impl CGPoint {
100100

101101
/// A two-dimensional size.
102102
///
103-
/// The width and height are guaranteed to be non-negative, so methods that
104-
/// expect that can safely accept [`CGSize`] as a parameter.
103+
/// As this is sometimes used to represent a distance vector, rather than a
104+
/// physical size, the width and height are _not_ guaranteed to be
105+
/// non-negative! Methods that expect that must use one of [`CGSize::abs`] or
106+
/// [`CGRect::standardize`].
105107
///
106108
/// This technically belongs to the `CoreGraphics` framework, but we define it
107109
/// here for convenience.
@@ -110,8 +112,10 @@ impl CGPoint {
110112
#[repr(C)]
111113
#[derive(Clone, Copy, Debug, PartialEq, Default)]
112114
pub struct CGSize {
113-
width: CGFloat,
114-
height: CGFloat,
115+
/// The dimensions along the x-axis.
116+
pub width: CGFloat,
117+
/// The dimensions along the y-axis.
118+
pub height: CGFloat,
115119
}
116120

117121
unsafe impl Encode for CGSize {
@@ -132,50 +136,58 @@ impl CGSize {
132136
/// ```
133137
/// use objc2::foundation::CGSize;
134138
/// let size = CGSize::new(10.0, 2.3);
135-
/// assert_eq!(size.width(), 10.0);
136-
/// assert_eq!(size.height(), 2.3);
139+
/// assert_eq!(size.width, 10.0);
140+
/// assert_eq!(size.height, 2.3);
137141
/// ```
138142
///
139-
/// ```should_panic
143+
/// Negative values are allowed (though often undesired).
144+
///
145+
/// ```
140146
/// use objc2::foundation::CGSize;
141147
/// let size = CGSize::new(-1.0, 0.0);
148+
/// assert_eq!(size.width, -1.0);
142149
/// ```
143150
#[inline]
144151
#[doc(alias = "NSMakeSize")]
145152
#[doc(alias = "CGSizeMake")]
146153
pub fn new(width: CGFloat, height: CGFloat) -> Self {
147-
// The documentation explicitly says:
154+
// The documentation for NSSize explicitly says:
148155
// > If the value of width or height is negative, however, the
149156
// > behavior of some methods may be undefined.
150157
//
151-
// Hence, we _must_ disallow negative widths and heights.
152-
153-
// TODO: Prevent NaN? Prevent infinities?
154-
match (width < 0.0, height < 0.0) {
155-
(true, true) => panic!("CGSize cannot have negative width and height"),
156-
(true, false) => panic!("CGSize cannot have negative width"),
157-
(false, true) => panic!("CGSize cannot have negative height"),
158-
(false, false) => Self { width, height },
159-
}
158+
// But since this type can come from FFI, we'll leave it up to the
159+
// user to ensure that it is used safely.
160+
Self { width, height }
160161
}
161162

162-
/// A size that is 0.0 in both dimensions.
163-
#[doc(alias = "NSZeroSize")]
164-
#[doc(alias = "CGSizeZero")]
165-
pub const ZERO: Self = Self {
166-
width: 0.0,
167-
height: 0.0,
168-
};
169-
163+
/// Convert the size to a non-negative size.
164+
///
165+
/// This can be used to convert the size to a safe value.
166+
///
167+
///
168+
/// # Examples
169+
///
170+
/// ```
171+
/// use objc2::foundation::CGSize;
172+
/// assert_eq!(CGSize::new(-1.0, 1.0).abs(), CGSize::new(1.0, 1.0));
173+
/// ```
170174
#[inline]
171-
pub const fn width(self) -> CGFloat {
172-
self.width
175+
pub fn abs(self) -> Self {
176+
Self::new(self.width.abs(), self.height.abs())
173177
}
174178

175-
#[inline]
176-
pub const fn height(self) -> CGFloat {
177-
self.height
178-
}
179+
/// A size that is 0.0 in both dimensions.
180+
///
181+
///
182+
/// # Examples
183+
///
184+
/// ```
185+
/// use objc2::foundation::CGSize;
186+
/// assert_eq!(CGSize::ZERO, CGSize { width: 0.0, height: 0.0 });
187+
/// ```
188+
#[doc(alias = "NSZeroSize")]
189+
#[doc(alias = "CGSizeZero")]
190+
pub const ZERO: Self = Self::new(0.0, 0.0);
179191
}
180192

181193
/// The location and dimensions of a rectangle.
@@ -234,6 +246,26 @@ impl CGRect {
234246
#[doc(alias = "CGRectZero")]
235247
pub const ZERO: Self = Self::new(CGPoint::ZERO, CGSize::ZERO);
236248

249+
/// Returns a rectangle with a positive width and height.
250+
///
251+
/// This is often useful
252+
///
253+
///
254+
/// # Examples
255+
///
256+
/// ```
257+
/// use objc2::foundation::{CGPoint, CGRect, CGSize};
258+
/// let origin = CGPoint::new(1.0, 1.0);
259+
/// let size = CGSize::new(-5.0, -2.0);
260+
/// let rect = CGRect::new(origin, size);
261+
/// assert_eq!(rect.standardize().size, CGSize::new(5.0, 2.0));
262+
/// ```
263+
#[inline]
264+
#[doc(alias = "CGRectStandardize")]
265+
pub fn standardize(self) -> Self {
266+
Self::new(self.origin, self.size.abs())
267+
}
268+
237269
/// The smallest coordinate of the rectangle.
238270
#[inline]
239271
#[doc(alias = "CGRectGetMinX")]
@@ -302,7 +334,6 @@ impl CGRect {
302334
// TODO: NSPointInRect / CGRectContainsPoint
303335
// TODO: NSOffsetRect / CGRectOffset
304336

305-
// TODO: CGRectStandardize
306337
// TODO: CGRectIsNull
307338
// TODO: CGRectIsInfinite
308339
// TODO: CGRectInfinite
@@ -343,30 +374,15 @@ pub type NSRect = CGRect;
343374
mod tests {
344375
use super::*;
345376

346-
#[test]
347-
#[should_panic = "CGSize cannot have negative width and height"]
348-
fn test_cgsize_new_both_negative() {
349-
CGSize::new(-1.0, -1.0);
350-
}
351-
352-
#[test]
353-
#[should_panic = "CGSize cannot have negative width"]
354-
fn test_cgsize_new_width_negative() {
355-
CGSize::new(-1.0, 1.0);
356-
}
357-
358-
#[test]
359-
#[should_panic = "CGSize cannot have negative height"]
360-
fn test_cgsize_new_height_negative() {
361-
CGSize::new(1.0, -1.0);
362-
}
363-
364377
#[test]
365378
fn test_cgsize_new() {
366379
CGSize::new(1.0, 1.0);
367380
CGSize::new(0.0, -0.0);
368381
CGSize::new(-0.0, 0.0);
369382
CGSize::new(-0.0, -0.0);
383+
CGSize::new(-1.0, -1.0);
384+
CGSize::new(-1.0, 1.0);
385+
CGSize::new(1.0, -1.0);
370386
}
371387

372388
// We know the Rust implementation handles NaN, infinite, negative zero

0 commit comments

Comments
 (0)