Skip to content

Conversation

@NicoHinderling
Copy link
Contributor

Related to https://linear.app/getsentry/issue/EME-550/ios-insights-duplicate-files

Write PDF/SVG raw data to disk during asset catalog parsing to enable duplicate detection of SVG and pdf files

@NicoHinderling NicoHinderling requested review from a team as code owners December 10, 2025 21:21
@linear
Copy link

linear bot commented Dec 10, 2025

@NicoHinderling NicoHinderling added the skip-changelog Apply this label to PRs that do not contain any user-facing changes label Dec 10, 2025
.appendingPathExtension(fileExtension)
try! data.write(to: fileURL)
}
// Save images that can be converted to CGImage
Copy link
Member

Choose a reason for hiding this comment

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

Assuming that PDF/SVG fail the later CGImageDestinationCreateWithURL() calls? Just wondering if it's worth trying to fit this into images[imageId] at all, just a bit confusing that dictionary no longer contains all images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, for SVGs CGImageDestinationCreateWithURL would return nil. For PDFs, the call would have succeeded, but the vector data would have been removed...

that said I realized this change can increase the size of ParsedAssets by a lot, so I've come up with a different lighterweight approach that achieves the same fix

@NicoHinderling NicoHinderling force-pushed the ensure-pdfs-and-svgs-saved branch from 4c5b870 to 7acd373 Compare December 11, 2025 18:17
@NicoHinderling NicoHinderling merged commit 34196c0 into master Dec 11, 2025
29 checks passed
@NicoHinderling NicoHinderling deleted the ensure-pdfs-and-svgs-saved branch December 11, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Apply this label to PRs that do not contain any user-facing changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants