-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
@frederoni I realized that the following code made the image on macOS be resized 4x
I'm changing that. @1ec5 @frederoni thinking about #10089 makes me realize that if we don't keep the scale and we make 456×480 be 1:1 pixel ratio that would be misleading developers since that would be a scale back for retina screens and images won't look good. Consider the scenario where someone takes a screenshot on an iPhone 7 (375×667) then they present the screenshot in the next controller (full screen) someone would expect a 750×1334 size image that will look good, instead we will return the 375×667 version and the OS will have to scale up resulting on a quality loss. |
@frederoni thank you for clarifying this. That's what I meant the resulting image should be 750 x 1334. @1ec5 is this what you mean? This is what is get in my current branch could you please test if still happens in your branch?:
|
@frederoni I can confirm that the 4x scaling on macOS happens with this branch too. When I comment this code:
I get an image sized 2052 × 1434 (Download and compare please) |
Yep, this change only fixes the iOS version. |
@frederoni I fixed the issue https://github.com/mapbox/mapbox-gl-native/pull/10020/files#diff-56bc5c1e41273957efd873aa83e6c522R159 I had to do something similar to what you did for iOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
65936d7 fixes the macOS implementation to produce a correctly sized image.
|
||
// Process image watermark in a work queue | ||
dispatch_queue_t workQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); | ||
dispatch_async(workQueue, ^{ | ||
#if TARGET_OS_IPHONE | ||
UIImage *logoImage = [UIImage imageNamed:@"mapbox" inBundle:[NSBundle mgl_frameworkBundle] compatibleWithTraitCollection:nil]; | ||
|
||
UIGraphicsBeginImageContext(mglImage.size); | ||
UIGraphicsBeginImageContextWithOptions(mglImage.size, NO, self.options.scale); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The map isn’t necessarily opaque. As in #7859, if the background style layer has a non-opaque fill color, an alpha channel may be needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created this ticket to follow up the tail work.
65936d7
to
a6f6f9b
Compare
Fixes #10089
Fixes the scale of the snapshot on iOS by letting CGImage resize the output.
Snapshotting on macOS already works as intended.@1ec5 👀
@fabian-guerra feel free to cherry-pick this into #10020