From 38186e82833d7a7d6bb3efdf272b8919028e7df0 Mon Sep 17 00:00:00 2001 From: Omeid Date: Thu, 7 Nov 2024 20:39:01 +1100 Subject: [PATCH] fix: apple bounding box (CGPathGetPathBoundingBox) (#2177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary As per the spec, bbox should not include any transformations, including scaling. It should also not include any control points. This fixes getBBox to give correct results matching Google Chrome, Firefox as per the spec. * What issues does the pull request solve? Please tag them so that they will get automatically closed once the PR is merged * What is the feature? (if applicable) Bug fix. * How did you implement the solution? Like this, see. * What areas of the library does it impact? getBBox API. ## Test Plan I build some stuff that works with SVG, the results on web were inconsistent, and digging down I realise it was a react-native-svg. Some fun fact, there was a surprisingly similar bug related to very confusing named (CGPathGetBoundingBox vs CGPathGetPathBoundingBox) APIs in Firefox some years ago as well. https://bugzilla.mozilla.org/show_bug.cgi?id=1369904 ### What's required for testing (prerequisites)? ### What are the steps to reproduce (after prerequisites)? ## Compatibility | OS | Implemented | | ------- | :---------: | | iOS | ✅ | | Android | 🔘# | | Web | 🔘 *| * Depends on the host platform. But works fine in major browsers. # will create a follow up PR. Currently getBBox on Android doesn't account for scaling properly. ## Checklist - [x] I have tested this on a device and a simulator - [x] I added documentation in `README.md` - [x] I updated the typed files (typescript) - [x] I added a test for the API in the `__tests__` folder Co-authored-by: Omeid Matten --- apple/Elements/RNSVGForeignObject.mm | 4 ++-- apple/Elements/RNSVGGroup.mm | 4 ++-- apple/RNSVGRenderable.mm | 4 ++-- apple/RNSVGRenderableModule.mm | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/apple/Elements/RNSVGForeignObject.mm b/apple/Elements/RNSVGForeignObject.mm index 3b7655d4..16472611 100644 --- a/apple/Elements/RNSVGForeignObject.mm +++ b/apple/Elements/RNSVGForeignObject.mm @@ -150,8 +150,8 @@ using namespace facebook::react; [self setHitArea:path]; if (!CGRectEqualToRect(bounds, CGRectNull)) { self.clientRect = bounds; - self.fillBounds = CGPathGetBoundingBox(path); - self.strokeBounds = CGPathGetBoundingBox(self.strokePath); + self.fillBounds = CGPathGetPathBoundingBox(path); + self.strokeBounds = CGPathGetPathBoundingBox(self.strokePath); self.pathBounds = CGRectUnion(self.fillBounds, self.strokeBounds); CGAffineTransform current = CGContextGetCTM(context); diff --git a/apple/Elements/RNSVGGroup.mm b/apple/Elements/RNSVGGroup.mm index ee56928b..44acf630 100644 --- a/apple/Elements/RNSVGGroup.mm +++ b/apple/Elements/RNSVGGroup.mm @@ -129,8 +129,8 @@ using namespace facebook::react; [self setHitArea:path]; if (!CGRectEqualToRect(bounds, CGRectNull)) { self.clientRect = bounds; - self.fillBounds = CGPathGetBoundingBox(path); - self.strokeBounds = CGPathGetBoundingBox(self.strokePath); + self.fillBounds = CGPathGetPathBoundingBox(path); + self.strokeBounds = CGPathGetPathBoundingBox(self.strokePath); self.pathBounds = CGRectUnion(self.fillBounds, self.strokeBounds); CGAffineTransform current = CGContextGetCTM(context); diff --git a/apple/RNSVGRenderable.mm b/apple/RNSVGRenderable.mm index 0cc82c09..b7c8b5c7 100644 --- a/apple/RNSVGRenderable.mm +++ b/apple/RNSVGRenderable.mm @@ -510,8 +510,8 @@ UInt32 saturate(CGFloat value) self.path = CGPathRetain(path); } [self setHitArea:path]; - self.fillBounds = CGPathGetBoundingBox(path); - self.strokeBounds = CGPathGetBoundingBox(self.strokePath); + self.fillBounds = CGPathGetPathBoundingBox(path); + self.strokeBounds = CGPathGetPathBoundingBox(self.strokePath); self.pathBounds = CGRectUnion(self.fillBounds, self.strokeBounds); } const CGRect pathBounds = self.pathBounds; diff --git a/apple/RNSVGRenderableModule.mm b/apple/RNSVGRenderableModule.mm index f6af102b..8d07fafa 100644 --- a/apple/RNSVGRenderableModule.mm +++ b/apple/RNSVGRenderableModule.mm @@ -138,7 +138,7 @@ RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(getBBox : (nonnull NSNumber *)reactTag op BOOL clipped = [[options objectForKey:@"clipped"] boolValue]; [svg getPath:nil]; - CGRect bounds = CGRectZero; + CGRect bounds = CGRectNull; if (fill) { bounds = CGRectUnion(bounds, svg.fillBounds); } @@ -150,12 +150,12 @@ RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(getBBox : (nonnull NSNumber *)reactTag op } if (clipped) { CGPathRef clipPath = [svg getClipPath]; - CGRect clipBounds = CGPathGetBoundingBox(clipPath); + CGRect clipBounds = CGPathGetPathBoundingBox(clipPath); if (clipPath && !CGRectIsEmpty(clipBounds)) { bounds = CGRectIntersection(bounds, clipBounds); } } - + if (CGRectIsNull(bounds)) bounds = CGRectZero; CGPoint origin = bounds.origin; CGSize size = bounds.size; return @{@"x" : @(origin.x), @"y" : @(origin.y), @"width" : @(size.width), @"height" : @(size.height)};