chore(preprod): Add AssetCatalog Parsing support for Multisize sets#2879
chore(preprod): Add AssetCatalog Parsing support for Multisize sets#2879NicoHinderling merged 1 commit intomasterfrom
Conversation
| if fileExtension != "svg", let unslicedImage = unslicedImage { | ||
| let format = fileExtension.isEmpty ? "png" : fileExtension | ||
| images[imageId] = (cgImage: unslicedImage, format: format) | ||
| } |
There was a problem hiding this comment.
I know you previously mentioned that we should be defensive and not include images missing a file type but in this case we want to include some that indeed don't have one
There was a problem hiding this comment.
We can keep the old defensive logic if we restructure this a bit:
if
unslicedImage == nil && className == "_CUIThemeMultisizeImageSetRendition",
let result = findImageForMultisizeSet(rendition, key, assetKeys, structuredThemeStore) {
unslicedImage = result.image
width = result.width
height = result.height
images[imageId] = (cgImage: unslicedImage, format: "png")
} else !fileExtension.isEmpty && fileExtension != "svg", let unslicedImage = unslicedImage {
images[imageId] = (cgImage: unslicedImage, format: fileExtension)
}
a43cca2 to
0ac1433
Compare
| if fileExtension != "svg", let unslicedImage = unslicedImage { | ||
| let format = fileExtension.isEmpty ? "png" : fileExtension | ||
| images[imageId] = (cgImage: unslicedImage, format: format) | ||
| } |
There was a problem hiding this comment.
We can keep the old defensive logic if we restructure this a bit:
if
unslicedImage == nil && className == "_CUIThemeMultisizeImageSetRendition",
let result = findImageForMultisizeSet(rendition, key, assetKeys, structuredThemeStore) {
unslicedImage = result.image
width = result.width
height = result.height
images[imageId] = (cgImage: unslicedImage, format: "png")
} else !fileExtension.isEmpty && fileExtension != "svg", let unslicedImage = unslicedImage {
images[imageId] = (cgImage: unslicedImage, format: fileExtension)
}
|
|
||
| let isVector = type == 9 | ||
| let (width, height, unslicedImage) = resolveImageDimensions(rendition, isVector) | ||
| var (width, height, unslicedImage) = resolveImageDimensions(rendition, isVector) |
There was a problem hiding this comment.
Making this mutable and then later on depending on unslicedImage to be nil for the findImageForMultisizeSet() feels weird to me. Is there something more stable we can check to see if it's a multisize set, like the type? Similar to how we do let isVector = type == 9
There was a problem hiding this comment.
Like I figured this would be structured like:
if isMultisizeImageSet {
// logic
} else {
// old logic
}
0ac1433 to
2ffddb5
Compare
Previously in #2879 I added the ability for us to collect multiset icons and have them included in the ParsedAssets folder / assets.json. There were two problems: 1. we were only picking up the primary image of the set. For example, for a multiset called `AppIcon_Alt_Color_Red`, we'd only pick up the first image (`red-light-min.png`) when really it contains three, all of which we want to consider when analyzing for insights 2. We were saving the multiset image as the multiset object, rather than as an image, which led us to have to [add some undesirable logic in the launchpad service](https://github.com/getsentry/launchpad/pull/437/files#r2470576784) With these changes, we now get all the images in the multisets and we save them as images
Certain assets weren't getting picked up for the Firefox app, and the ones missing appeared to be multisize sets, like this
They won't have a a file extension either it seems (ex.
Assets.car/AppIcon_Alt_Color_Purple). So we'll go ahead and save them as a (default type) png