Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Add image accessor to MGLStyle #7096

Merged
merged 4 commits into from
Dec 6, 2016
Merged

Add image accessor to MGLStyle #7096

merged 4 commits into from
Dec 6, 2016

Conversation

rmnblm
Copy link

@rmnblm rmnblm commented Nov 16, 2016 •
edited
Loading

@1ec5: I'm struggling a bit right now. Could you give a quick review for the current state of implementation to see if I'm heading into the right direction? What's currently not working is to initialize an MGLImage from the byte array of a sprite image.

This PR resolves issue #7012.

@mention-bot
Copy link

@rmnblm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @boundsj, @frederoni and @jfirebaugh to be potential reviewers.

@rmnblm rmnblm changed the title [core, ios, macos] Add possibility to get MGLImage from style by name WIP: [core, ios, macos] Add possibility to get MGLImage from style by name Nov 16, 2016
@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling labels Nov 21, 2016
@rmnblm
Copy link
Author

rmnblm commented Nov 24, 2016

@1ec5 Just a small reminder ⏱ (I'm pretty sure that you're totally busy and that's okay, just let me know if you need more details or something I can do ☺️)

@1ec5 1ec5 self-assigned this Nov 25, 2016
@1ec5
Copy link
Contributor

1ec5 commented Nov 25, 2016 •
edited
Loading

Hi @rmnblm, this is on my plate to take a look at. Indeed, it’s a little busy with the holiday season and preparations for the v3.4.0 release. Thanks for your patience!

Incidentally, I think it would be nice to get this fix into v3.4.x given that it rounds out the runtime styling API. For that to happen, please retarget this PR at the release-ios-v3.4.0 branch, where we’re developing v3.4.0 and subsequent patch releases.

@rmnblm rmnblm changed the base branch from master to release-ios-v3.4.0 November 26, 2016 11:52
@rmnblm rmnblm changed the base branch from release-ios-v3.4.0 to master November 26, 2016 11:52
@rmnblm rmnblm changed the base branch from master to release-ios-v3.4.0 November 26, 2016 11:53
1ec5
1ec5 suggested changes Nov 30, 2016
View reviewed changes
Copy link
Contributor

@1ec5 1ec5 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmnblm, thanks for retargeting this PR. It needs to be rebased on the release-ios-v3.4.0 branch as well. I can take care of that, but first you’ll need to check the “allow maintainers to edit this branch” box.

platform/darwin/src/MGLStyle.mm
return nil;
}

auto imageData = [NSData dataWithBytes:spriteImage->image.data.get() length:spriteImage->image.bytes()];
Copy link
Contributor

@1ec5 1ec5 Nov 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PremultipliedImage has a field called size that likely has what you want.

@1ec5 1ec5 added this to the ios-3.4.1 milestone Nov 30, 2016
@rmnblm
Copy link
Author

rmnblm commented Dec 1, 2016 •
edited
Loading

@1ec5 I enabled "Allow edits from maintainers." 👍

1ec5
1ec5 approved these changes Dec 5, 2016
View reviewed changes
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All set.

@1ec5 1ec5 changed the title WIP: [core, ios, macos] Add possibility to get MGLImage from style by name Add image accessor to MGLStyle Dec 5, 2016
@1ec5
[ios, macos] Fixed -imageForName:
329a621 
Convert from sprite images to platform images using the existing encodePNG() function, which is also used for printing. Allow -imageForName: to return nil without an assertion failure. Added a basic test.
@1ec5 1ec5 modified the milestones: ios-v3.4.0, ios-3.4.1 Dec 5, 2016
@1ec5 1ec5 merged commit 1045934 into mapbox:release-ios-v3.4.0 Dec 6, 2016
@1ec5
Copy link
Contributor

1ec5 commented Dec 6, 2016

Thanks for your contribution, @rmnblm!

This was referenced Dec 6, 2016
Closed
Closed
1ec5 added a commit that referenced this pull request Dec 6, 2016
@1ec5
[test] Fixed Map-GetImage
9456bec 
Followup to #7096.
@1ec5
Copy link
Contributor

1ec5 commented Dec 6, 2016

9456bec fixes an mbgl test failure caused by this PR.

For future reference, if you make changes to mbgl or its tests, please run make xproj and run the mbgl-test scheme. (You can also turn on the --gtest-filter= argument that’s currently disabled in the scheme editor to run a specific test.) Unfortunately, we don’t run CI builds for third-party PRs. I’ll try to remember to push a dry-run branch in the main repository, to make sure all the CI builds pass, whenever we get one.

@rmnblm rmnblm deleted the features/style_image_accessor branch December 6, 2016 08:00
@1ec5 1ec5 mentioned this pull request Dec 6, 2016
Merged
@rmnblm
Copy link
Author

rmnblm commented Dec 7, 2016

Thanks for merging and especially fixing the subsequent issue @1ec5.
Like the title of the PR said it was WIP but it surely wasn't clever from me to check in a currently failing test so it was clearly my fault, sorry.

@1ec5
Copy link
Contributor

1ec5 commented Dec 7, 2016

No worries. Looking forward to your future PRs! 😃

@1ec5 1ec5 mentioned this pull request Feb 10, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@1ec5 1ec5 1ec5 approved these changes

@boundsj boundsj boundsj approved these changes

Assignees

@1ec5 1ec5

Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling
Projects
None yet
Milestone
ios-v3.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@rmnblm @mention-bot @1ec5 @boundsj