Skip to content

Commit

Permalink
Fix crash when user updates icons with images of different sizes
Browse files Browse the repository at this point in the history
  • Loading branch information
alasram committed Nov 5, 2024
1 parent 7ba023c commit 9bb7d8a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 8 deletions.
40 changes: 39 additions & 1 deletion src/mbgl/renderer/image_atlas.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <mbgl/renderer/image_atlas.hpp>
#include <mbgl/renderer/image_manager.hpp>
#include <mbgl/util/logging.hpp>

#include <mapbox/shelf-pack.hpp>

Expand Down Expand Up @@ -63,7 +64,44 @@ void populateImagePatches(ImagePositions& imagePositions,
const auto updatedImage = imageManager.getSharedImage(name);
if (updatedImage == nullptr) continue;

patches.emplace_back(*updatedImage, position.paddedRect);
Immutable<style::Image::Impl> imagePatch = *updatedImage;

// The max patch size that fits the current Atlas is the paddedRect size and the padding
assert(position.paddedRect.w > ImagePosition::padding * 2);
assert(position.paddedRect.h > ImagePosition::padding * 2);
uint32_t maxPatchWidth = position.paddedRect.w - ImagePosition::padding * 2;
uint32_t maxPatchHeight = position.paddedRect.h - ImagePosition::padding * 2;
if (maxPatchWidth < imagePatch->image.size.width || maxPatchHeight < imagePatch->image.size.height) {
// imagePositions are created in makeImageAtlas
// User can update the image. e.g. an Android call to
// MapLibreMap.getStyle.addImage(imageId, imageBitmap), which will call ImageManager.updateImage
// If the updated image is larger than the previous image then position.paddedRect area in the atlas
// won't fit the new image. ImageManager is unaware of the the atlas packing.
// This code simply prints an error message and resizes the image to fit the atlas to avoid crashes
// A better solution (potentially expensive) is to repack the atlas: this requires keeping the
// previous atlas image and detect when a repack is required.
// Another possibility is to simply throw an exception and requires the user to provide different
// IDs for images with different sizes
const auto& imageImpl = *updatedImage->get();
Log::Error(Event::General,
imageImpl.id + " does not fit the atlas. " + imageImpl.id +
" will be resized from:" + std::to_string(imageImpl.image.size.width) + "x" +
std::to_string(imageImpl.image.size.height) + " to:" + std::to_string(maxPatchWidth) +
"x" + std::to_string(maxPatchHeight));
auto resizedImage = imagePatch->image.clone();
auto newWidth = std::min(maxPatchWidth, imagePatch->image.size.width);
auto newHeight = std::min(maxPatchHeight, imagePatch->image.size.height);
resizedImage.resize({newWidth, newHeight});
auto mutableImagePatch = makeMutable<style::Image::Impl>(imageImpl.id,
std::move(resizedImage),
imageImpl.pixelRatio,
imageImpl.sdf,
style::ImageStretches(),
style::ImageStretches());
imagePatch = std::move(mutableImagePatch);
}

patches.emplace_back(imagePatch, position.paddedRect);
position.version = version;
}
}
Expand Down
16 changes: 9 additions & 7 deletions src/mbgl/tile/geometry_tile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,17 @@ void GeometryTileRenderData::upload(gfx::UploadPass& uploadPass) {

if (atlasTextures->icon && !imagePatches.empty()) {
for (const auto& imagePatch : imagePatches) { // patch updated images.
uint32_t xDest = imagePatch.paddedRect.x + ImagePosition::padding;
uint32_t yDest = imagePatch.paddedRect.y + ImagePosition::padding;
if (atlasTextures->icon->getSize().width < xDest + imagePatch.image->image.size.width ||
atlasTextures->icon->getSize().height < yDest + imagePatch.image->image.size.height) {
Log::Error(Event::General, imagePatch.image->id + " does not fit the icon atlas");
assert(false);
}
#if MLN_DRAWABLE_RENDERER
atlasTextures->icon->uploadSubRegion(imagePatch.image->image,
imagePatch.paddedRect.x + ImagePosition::padding,
imagePatch.paddedRect.y + ImagePosition::padding);
atlasTextures->icon->uploadSubRegion(imagePatch.image->image, xDest, yDest);
#else
uploadPass.updateTextureSub(*atlasTextures->icon,
imagePatch.image->image,
imagePatch.paddedRect.x + ImagePosition::padding,
imagePatch.paddedRect.y + ImagePosition::padding);
uploadPass.updateTextureSub(*atlasTextures->icon, imagePatch.image->image, xDest, yDest);
#endif
}
imagePatches.clear();
Expand Down

0 comments on commit 9bb7d8a

Please sign in to comment.