-
Notifications
You must be signed in to change notification settings - Fork 122
Conversation
ac49d8e
to
4b218cd
Compare
4b218cd
to
955cbe2
Compare
955cbe2
to
f3d0dfd
Compare
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.
Looks good. Is there a unit test that we could add?
I’ve added unit tests. The NSImage and UIImage extensions now round-trip cap insets (the inverse of the content rect). Note that NSImage and UIImage only support a single stretchable region, not the multiple stretchable regions introduced in mapbox/mapbox-gl-native#16030 for sprite sheets. |
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.
Just the one question.
mbgl::optional<mbgl::style::ImageContent> imageContent; | ||
if (!MGLEdgeInsetsIsZero(self.capInsets)) { | ||
imageContent = (mbgl::style::ImageContent){ | ||
.left = static_cast<float>(self.capInsets.left * self.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.
I'm confused by the * self.scale
here, compared with the / self.scale
above. What's going on here? (The macOS version below has *
in both)
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.
-initWithMGLStyleImage:
divides by the scale and -mgl_styleImageWithIdentifier:
multiplies by the scale on both platforms.
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.
Oh, I see what you mean – you’re referring to the stretchX
and stretchY
variables. Yes, I think the division above is incorrect. The tests aren’t catching the mistake because they only assert on the image’s content rect, not its stretchable areas per se.
The MGLSymbolStyleLayer.iconTextFit property now respects the cap insets of any nine-part stretchable image passed into the -[MGLStyle setImage:forName:] method. The “Manipulate Style” command in macosapp demonstrates the stretchable image feature by adding a single Ohio state route shield image, which has cap insets defined in the asset catalog, and labeling each Ohio state route with that image, stretched to fit the route number without widening the icon’s stroke.
cca1189
to
4e0a77e
Compare
Note that NSImage and UIImage only support a single resizable region (though it can be tiled as an alternative to stretching). There are third-party solutions for native support for multiple stretchable regions, but the usual way to implement such images is to draw the image programmatically on demand. |
The
MGLSymbolStyleLayer.iconTextFit
property now respects the cap insets of any nine-part stretchable image passed into the-[MGLStyle setImage:forName:]
method.The “Manipulate Style” command in macosapp demonstrates the stretchable image feature by adding a single Ohio state route shield image (from a PDF with cap insets in the asset catalog) and labeling each Ohio state route with that image, stretched to fit the route number without widening the icon’s stroke.
Depends on #181. Retarget and rebase this PR onto master before merging.
/ref mapbox/mapbox-gl-native#16030
/cc @mapbox/maps-ios @kkaefer