Skip to content
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

Jump to bottom

[macOS][Xcode 15] Avoid using dirtyRect in drawRect: #2136

Merged
merged 1 commit into from
Sep 20, 2023
Merged

[macOS][Xcode 15] Avoid using dirtyRect in drawRect: #2136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion apple/Elements/RNSVGSvgView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ - (void)drawRect:(CGRect)rect
_boundingBox = rect;
Copy link

Choose a reason for hiding this comment

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

Is this the issue? Shouldn't this be set to [self bounds]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That didn't seem to have any effect in my testing.

The remaining issue I found is that SVG images will still render with a large width/height and not properly clip. Tracing the callstack , I got the issue to to this method in RNSVGGroups.mm:

- (void)setupGlyphContext:(CGContextRef)context
{
  CGRect clipBounds = CGContextGetClipBoundingBox(context); <----
  ....
}

CGContextGetClipBoundingBox returns an abnormally large box. My next step is to compare what it returns when compiled against Xcode 14/macOS 13

Copy link

Choose a reason for hiding this comment

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

Did you ever figure out what was going on for your case? We were seeing a similar issue where SVGImages weren't rendering properly, except for us CGContextGetClipBoundingBox at that point was abnormally small, with width and height of 1.

I'm not too familiar with Apple APIs, but the farthest I got was noticing that the abnormal context came from the call to UIGraphicsGetCurrentContext. Before I could investigate further, we suddenly started getting reasonable bounding boxes and I was no longer able to reproduce 😕.

Copy link
Contributor Author

@Saadnajmi Saadnajmi Oct 27, 2023 •
edited

Choose a reason for hiding this comment

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

I did not get any further. Might be worth filing a Github issue to track. I'm curious what your use case is?

Copy link

Choose a reason for hiding this comment

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

We're loading an image and then drawing annotations that a user specified using SVGs, basically.

I'll try to file another issue once we start seeing it again, but it unfortunately (fortunately?) seems to work on my machine™.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, the method you linked is a UIKit (aka, iOS only) method we shim on macOS: https://github.com/microsoft/react-native-macos/blob/2fe53e8050a9c0b0906c0e092764ffdebcc039d4/packages/react-native/React/Base/macOS/RCTUIKit.m#L29-L32

I've been meaning to revisit some of these shims, but ultimately I think it would call the same underlying NSGraphicsContext code. I couldn't find any apple docs about CGContextGetClipBoundingBox changing, but It very much feels like this method also had a breaking change with Xcode 15 / the macOS 14 SDK.

CGContextRef context = UIGraphicsGetCurrentContext();

[self drawToContext:context withRect:rect];
[self drawToContext:context withRect:[self bounds]];
}

- (RNSVGPlatformView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event
Expand Down