Skip to content

Commit

Permalink
fix: apple bounding box (CGPathGetPathBoundingBox) (#2177)
Browse files Browse the repository at this point in the history
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect -->

# 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

<!-- Check completed item, when applicable, via: [X] -->

- [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 <[email protected]>
  • Loading branch information
omeid and Omeid Matten authored Nov 7, 2024
1 parent 29d4c7c commit 38186e8
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
4 changes: 2 additions & 2 deletions apple/Elements/RNSVGForeignObject.mm
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ - (void)renderGroupTo:(CGContextRef)context rect:(CGRect)rect
[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);
Expand Down
4 changes: 2 additions & 2 deletions apple/Elements/RNSVGGroup.mm
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ - (void)renderGroupTo:(CGContextRef)context rect:(CGRect)rect
[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);
Expand Down
4 changes: 2 additions & 2 deletions apple/RNSVGRenderable.mm
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,8 @@ - (void)renderLayerTo:(CGContextRef)context rect:(CGRect)rect
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;
Expand Down
6 changes: 3 additions & 3 deletions apple/RNSVGRenderableModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ @implementation RNSVGRenderableModule
BOOL clipped = [[options objectForKey:@"clipped"] boolValue];
[svg getPath:nil];

CGRect bounds = CGRectZero;
CGRect bounds = CGRectNull;
if (fill) {
bounds = CGRectUnion(bounds, svg.fillBounds);
}
Expand All @@ -150,12 +150,12 @@ @implementation RNSVGRenderableModule
}
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)};
Expand Down

0 comments on commit 38186e8

Please sign in to comment.