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

Snapshotter (runtime styling) delegates #235

Merged
merged 5 commits into from
Mar 26, 2020

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Mar 23, 2020

Added a protocol and property for MGLMapSnapshotter’s delegate, which it uses for runtime styling. Generalized MGLStyle to be usable in the absence of MGLMapView. Moved MGLOpenGLStyleLayer code around to compile.

In macosapp, the File ‣ Export Image command now matches the main map’s layer visibility settings and translated labels.

Added unit tests and snapshot tests of MGLMapSnapshotter that work on both iOS and macOS, unlike the iOS-only integration tests of MGLMapSnapshotter. This test suite comes with an easy way to snapshot-test runtime styling.

Fixes #200, fixes #236.

/cc @mapbox/maps-ios

@1ec5 1ec5 added the enhancement New feature or request label Mar 23, 2020
@1ec5 1ec5 added this to the release-vanillashake milestone Mar 23, 2020
@1ec5 1ec5 requested review from julianrex and a team March 23, 2020 09:19
@1ec5 1ec5 self-assigned this Mar 23, 2020
Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

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

This looks good - my main questions are listed inline, but boil down to any potential confusion/overlap with existing handlers.

Also, do you have any tests for the new style handling in snapshotter?

platform/darwin/src/MGLMapSnapshotter.h Show resolved Hide resolved
platform/darwin/src/MGLMapSnapshotter.h Show resolved Hide resolved
@1ec5
Copy link
Contributor Author

1ec5 commented Mar 23, 2020

Also, do you have any tests for the new style handling in snapshotter?

Just the manual macosapp thing so far, but I can add an integration test.

@julianrex julianrex requested a review from jmkiley March 23, 2020 19:16
@@ -107,13 +68,20 @@ - (instancetype)initWithIdentifier:(NSString *)identifier {
return (mbgl::style::CustomLayer *)super.rawLayer;
}

- (MGLMapView *)mapView {
if ([self.style.stylable isKindOfClass:[MGLMapView class]]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As implemented in this PR, MGLOpenGLStyleLayer doesn’t support snapshotting, but it wouldn’t be too difficult to make it work if necessary.

@1ec5
Copy link
Contributor Author

1ec5 commented Mar 24, 2020

I’ve added unit tests and snapshot tests of MGLMapSnapshotter that work on both iOS and macOS, unlike the iOS-only integration tests of MGLMapSnapshotter, which will help catch issues like #236. This test suite comes with an easy way to snapshot-test runtime styling for other purposes too.

Added a protocol and property for MGLMapSnapshotter’s delegate, which it uses for runtime styling. Generalized MGLStyle to be usable in the absence of MGLMapView. Moved MGLOpenGLStyleLayer code around to compile.
@1ec5 1ec5 force-pushed the 1ec5-snapshotter-runtime-styling-200 branch from 7aa43fb to 889a481 Compare March 24, 2020 17:50
@1ec5 1ec5 force-pushed the 1ec5-snapshotter-runtime-styling-200 branch from 889a481 to b5797d3 Compare March 24, 2020 18:00
Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

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

Thank you - tests look great!

@1ec5 1ec5 merged commit 1d46646 into master Mar 26, 2020
@1ec5 1ec5 deleted the 1ec5-snapshotter-runtime-styling-200 branch March 26, 2020 22:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshotter overlay handler not called Platform bindings for runtime styling APIs on MapSnapshotter
2 participants