[[ Bug 21396 ]] Fix crash on startup in iOS 12 beta - #6622
Conversation
|
|
||
| glDeleteTextures(1, &t_texture); | ||
|
|
||
| free(p_bits); |
There was a problem hiding this comment.
This seems a bit dodgy - surely the bits are owned by the MCGRaster which is passed in at a level above... Shouldn't the original caller be responsible for freeing? (assuming they aren't borrowed).
There was a problem hiding this comment.
Hmm... I had just moved the free from UnlockPixels to here seeing as FlushBits in MCUIKitStackSurface takes ownership of them so I wanted FlushBits to take them in both cases.
There was a problem hiding this comment.
Can you think of an alternative that doesn't involve copying the buffer in FlushBits for MCUIKitStackSurface given the CGDataProvider is expecting them to be available.
| if (p_update) | ||
| FlushBits(p_area, p_raster . pixels, p_raster . stride); | ||
|
|
||
| free(p_raster . pixels); |
There was a problem hiding this comment.
I see that I'm leaking at the moment here in the case of ! p_update.. should now be:
if (p_update)
{
FlushBits(p_area, p_raster . pixels, p_raster . stride);
}
else
{
free(p_raster . pixels);
}
There was a problem hiding this comment.
@runrevmark perhaps FlushBits should set a boolean indicating if it has taken ownership of the bits? Then we can do:
bool t_taken = false;
if (p_update)
FlushBits(p_area, p_raster . pixels, p_raster . stride, t_taken);
if (!t_taken)
free(p_raster . pixels);
There was a problem hiding this comment.
@runrevmark I'm running with the above as it means I can only set x_taken if the raster is successfully converted to a CGImage. Will push up in a bit.
407b548 to
b1907c5
Compare
This patch fixes an issue where the view layer appears to be retaining the `CGDataProvider` from an image when it is drawn via `CGContextDrawImage`. As we were freeing the raster directly after the draw the `CALayer` could not access it from the `CGDataProvider` when rendering causing a crash. This patch therefore changes the behavior of `MCGRasterCreateCGDataProvider` where previously when calling it with the copy parameter `false` it would be left to the caller to free the buffer the data provider now takes ownership of it and it is freed via the data provider free callback. As a consequence of the change custom cursor creation on mac now copies the buffer for simplicity.
b1907c5 to
964d263
Compare
|
Okay so I think I have now stared at this too much! From what you said it fixes the issue and the code looks correct. So... |
|
@livecode-vulcan review ok 964d263 |
|
💙 review by @runrevmark ok 964d263 |
[[ Bug 21396 ]] Fix crash on startup in iOS 12 beta This patch fixes an issue where the view layer appears to be retaining the `CGDataProvider` from an image when it is drawn via `CGContextDrawImage`. As we were freeing the raster directly after the draw the `CALayer` could not access it from the `CGDataProvider` when rendering causing a crash. This patch therefore changes the behavior of `MCGRasterCreateCGDataProvider` where previously when calling it with the copy parameter `false` it would be left to the caller to free the buffer the data provider now takes ownership of it and it is freed via the data provider free callback. As a consequence of the change custom cursor creation on mac now copies the buffer for simplicity.
|
😎 test success 964d263
|
This patch fixes an issue where the view layer appears to be retaining the
CGDataProviderfrom an image when it is drawn viaCGContextDrawImage.As we were freeing the raster directly after the draw the
CALayercould notaccess it from the
CGDataProviderwhen rendering causing a crash.This patch therefore changes the behavior of
MCGRasterCreateCGDataProviderwhere previously when calling it with the copy parameter
falseit would beleft to the caller to free the buffer the data provider now takes ownership
of it and it is freed via the data provider free callback. As a consequence
of the change custom cursor creation on mac now copies the buffer for
simplicity.