2.0 rc #597
2.0 rc #597
Conversation
WalkthroughThe recent modifications span across Android and iOS platforms, enhancing the Open Mobile Maps framework with updates to library dependencies, new layer classes for both raster and vector tiles, and improvements in map configuration and layer interaction. Significant changes include updates to map layer handling, font loading mechanisms, and the introduction of default configurations for web mercator tiled map layers. These updates facilitate more robust and customizable map interactions and presentations in mobile applications. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
maurhofer-ubique
force-pushed
the
2.0-RC
branch
from
4b590fc to
33a370d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 18
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
djinni/yaml/default_tiled_2d_map_layer_configs.yamlis excluded by:!**/*.yaml
Files selected for processing (40)
- android/readme.md (6 hunks)
- android/src/main/java/io/openmobilemaps/mapscore/map/layers/TiledRasterLayer.kt (1 hunks)
- android/src/main/java/io/openmobilemaps/mapscore/map/layers/TiledVectorLayer.kt (1 hunks)
- android/src/main/java/io/openmobilemaps/mapscore/map/loader/DataLoader.kt (1 hunks)
- android/src/main/java/io/openmobilemaps/mapscore/map/loader/FontLoader.kt (5 hunks)
- android/src/main/java/io/openmobilemaps/mapscore/map/view/MapView.kt (3 hunks)
- bridging/android/java/io/openmobilemaps/mapscore/shared/map/layers/tiled/DefaultTiled2dMapLayerConfigs.kt (1 hunks)
- bridging/android/java/io/openmobilemaps/mapscore/shared/map/layers/tiled/raster/wmts/WmtsLayerDescription.kt (1 hunks)
- bridging/android/jni/map/layers/tiled/NativeDefaultTiled2dMapLayerConfigs.cpp (1 hunks)
- bridging/android/jni/map/layers/tiled/NativeDefaultTiled2dMapLayerConfigs.h (1 hunks)
- bridging/android/jni/map/layers/tiled/raster/wmts/NativeWmtsLayerDescription.cpp (2 hunks)
- bridging/ios/MCDefaultTiled2dMapLayerConfigs+Private.h (1 hunks)
- bridging/ios/MCDefaultTiled2dMapLayerConfigs+Private.mm (1 hunks)
- bridging/ios/MCDefaultTiled2dMapLayerConfigs.h (1 hunks)
- bridging/ios/MCWmtsLayerDescription+Private.mm (2 hunks)
- bridging/ios/MCWmtsLayerDescription.h (2 hunks)
- bridging/ios/MCWmtsLayerDescription.mm (2 hunks)
- djinni/map/layers/tiled/raster/wmts/wmts_capabilities.djinni (1 hunks)
- djinni/map/layers/tiled/tiled_layer.djinni (1 hunks)
- docs/architecture.md (1 hunks)
- ios/helpers/FatalUnwrapping.swift (1 hunks)
- ios/maps/Layers/AsyncLayer.swift (1 hunks)
- ios/maps/Layers/CapabilitiesResource.swift (1 hunks)
- ios/maps/Layers/Layer.swift (1 hunks)
- ios/maps/Layers/RasterLayer.swift (1 hunks)
- ios/maps/Layers/VectorLayer.swift (1 hunks)
- ios/maps/MCCoord+Convenience.swift (1 hunks)
- ios/maps/MCMapView.swift (2 hunks)
- ios/maps/SwiftUI/MapView.swift (1 hunks)
- ios/readme.md (5 hunks)
- readme.md (2 hunks)
- shared/public/DefaultTiled2dMapLayerConfigs.h (1 hunks)
- shared/public/Tiled2dMapSourceImpl.h (1 hunks)
- shared/public/WmtsLayerDescription.h (2 hunks)
- shared/src/map/camera/MapCamera2d.cpp (5 hunks)
- shared/src/map/layers/tiled/DefaultTiled2dMapLayerConfigs.cpp (1 hunks)
- shared/src/map/layers/tiled/WebMercatorTiled2dMapLayerConfig.cpp (1 hunks)
- shared/src/map/layers/tiled/WebMercatorTiled2dMapLayerConfig.h (1 hunks)
- shared/src/map/layers/tiled/wmts/WmtsCapabilitiesResource.cpp (1 hunks)
- shared/src/map/layers/tiled/wmts/WmtsTiled2dMapLayerConfig.cpp (1 hunks)
Files skipped from review due to trivial changes (4)
- bridging/android/java/io/openmobilemaps/mapscore/shared/map/layers/tiled/DefaultTiled2dMapLayerConfigs.kt
- bridging/ios/MCDefaultTiled2dMapLayerConfigs+Private.h
- bridging/ios/MCDefaultTiled2dMapLayerConfigs.h
- readme.md
Additional comments: 23
ios/maps/Layers/Layer.swift (1)
- 11-13: The protocol definition for
Layeris clear and adheres to Swift's protocol-oriented design principles, providing a flexible contract for layer implementations.bridging/android/java/io/openmobilemaps/mapscore/shared/map/layers/tiled/raster/wmts/WmtsLayerDescription.kt (1)
- 11-11: The update to make the
boundsproperty nullable inWmtsLayerDescriptionis a sensible change that increases flexibility in map layer configurations, aligning with Kotlin's null safety features.ios/helpers/FatalUnwrapping.swift (1)
- 10-24: The custom operator
!!for unwrapping optionals with a failure message is implemented correctly, using Swift best practices. Caution is advised when usingfatalErrorin non-throwing contexts to ensure it aligns with the application's error handling strategy and does not lead to avoidable crashes.android/src/main/java/io/openmobilemaps/mapscore/map/loader/DataLoader.kt (1)
- 36-36: Setting the
referrerproperty to a default empty string is a sensible default that can help avoid null pointer exceptions when theRefererInterceptoris used. However, ensure that downstream consumers of theDataLoaderclass are aware of this change, as it might affect HTTP requests if the server expects a specific referrer header for authorization or logging purposes.android/src/main/java/io/openmobilemaps/mapscore/map/view/MapView.kt (3)
- 188-194: The methods for adding
TiledRasterLayerandTiledVectorLayerto theMapVieware correctly implemented. They properly wrap the layer-specific interfaces before adding them, ensuring compatibility with the existingaddLayermethod that accepts aLayerInterface. This approach maintains consistency and modularity in handling different layer types.- 200-206: The methods for inserting
TiledRasterLayerandTiledVectorLayerat a specific index are correctly implemented. They ensure that the layer interfaces are correctly wrapped and inserted at the desired position, which is crucial for maintaining the correct layer order in the map view.- 220-226: The methods for removing
TiledRasterLayerandTiledVectorLayerfrom theMapVieware correctly implemented. They ensure that the specific layer interfaces are correctly identified and removed, which is essential for dynamic layer management within the map view.ios/maps/SwiftUI/MapView.swift (2)
- 104-135: The method
updateCamerain the SwiftUIMapViewis well-implemented, taking into account the camera's current state and whether updates should be animated. It correctly handles different update modes (center,zoom,visibleRect) based on user interaction or map-driven changes. This approach ensures a smooth and responsive map view experience in SwiftUI applications.- 138-175: The method
updateLayersin the SwiftUIMapViewcorrectly manages the addition and removal of layers based on the current and previous state. This dynamic layer management is crucial for maintaining an up-to-date map view that reflects the desired content and layer ordering. The use ofneedsInsertandneedsRemovalarrays to track changes is an efficient way to minimize unnecessary updates.ios/maps/MCMapView.swift (2)
- 35-35: Providing a default value for the
mapConfigparameter in theMCMapViewinitializer is a user-friendly enhancement that simplifies the creation of map views with default configurations. This change makes it easier for developers to get started with the map view without requiring detailed knowledge of map configurations.- 294-316: The addition of new methods for adding, inserting, and removing layers using the
Layerprotocol inMCMapViewenhances the flexibility and ease of use of the map view in Swift. These methods provide a more Swift-friendly API for managing layers, aligning with Swift's type system and protocols. This improvement is particularly beneficial for developers working with Swift and SwiftUI.ios/readme.md (5)
- 34-34: The dependency version for Open Mobile Maps is correctly updated to
2.0.0to reflect the new major release. This ensures users will integrate the latest version with all the new features and improvements.- 52-60: The example code for initializing and configuring
MCMapViewwith aTiledRasterLayerand setting the camera position is clear and well-documented. It provides a straightforward guide for developers on how to start using the map view in their iOS applications.- 155-251: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [136-159]
The line layer example is comprehensive, showcasing how to create and add a line to the mapView. The explanation of the
MCLineStyleandMCColorStateListparameters enhances understanding. Ensure consistency in the documentation regarding the use of color and style parameters across different layer types.Verification successful
The search results indicate that both
MCLineStyleandMCColorStateListare consistently mentioned across various files in the repository, including the iOS readme, bridging code for iOS, and YAML configuration files for Djinni (which is often used for cross-platform C++ projects). These mentions span from definitions and imports in header files (*.hand*.mmfiles) to their usage in the iOS documentation (ios/readme.md).Given this context, it appears that the documentation for
MCLineStyleandMCColorStateListis indeed consistent across different layer types, as these components are referenced in both the iOS-specific documentation and the underlying bridging and configuration code. This consistency suggests that the review comment's concern about ensuring consistency in the documentation regarding the use of color and style parameters across different layer types has been addressed by the existing codebase and documentation.Therefore, based on the information gathered from the repository, the review comment regarding the need to ensure consistency in the documentation of color and style parameters across different layer types can be considered verified, as the search results show a consistent usage and documentation of
MCLineStyleandMCColorStateListacross the codebase.* 160-238: The custom layer configuration example is detailed, providing insights into creating a custom raster tile service layer. This section effectively demonstrates how to override various configuration methods. It's crucial to ensure that all methods and properties used here are well-documented elsewhere in the SDK documentation.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent documentation of color and style parameters rg "MCLineStyle|MCColorStateList" ios/docs/Length of output: 95
Script:
#!/bin/bash # Broaden the search for mentions of MCLineStyle or MCColorStateList across the entire repository rg "MCLineStyle|MCColorStateList"Length of output: 3816
Verification successful
The search results indicate that the methods and properties used in the custom layer configuration example (
MCTiled2dMapLayerConfig,getZoomInfo,getCoordinateSystemIdentifier,getBounds,getTileUrl,getLayerName,getZoomLevelInfos) are indeed mentioned within theios/readme.mdfile, which contains the example itself. Additionally, there's a reference toMCTiled2dMapLayerConfiginios/maps/Layers/RasterLayer.swift, suggesting that the SDK does provide some level of documentation or examples for these methods and properties within the iOS context. Similar methods are also overridden in the Android context (android/readme.md), indicating a cross-platform consistency in the approach to customizing map layers.Given this context, it can be concluded that the review comment about ensuring that all methods and properties used in the custom layer configuration example are well-documented elsewhere in the SDK documentation is addressed by the presence of these references within the project's documentation and codebase. However, the depth and quality of the documentation cannot be fully assessed based on the search results alone, as it would require a manual review of the content of these references.
Based on the information available, the review comment is verified in the sense that references to the mentioned methods and properties exist within the project's documentation and codebase, suggesting some level of documentation is provided.
* 240-246: The section on changing the map projection provides a concise example of initializing the map view with a different coordinate system. This is a valuable customization option for developers. Ensure that `MCCoordinateSystemFactory` and its methods are documented for developers to explore other available coordinate systems.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure documentation for methods and properties used in custom layer config rg "MCTiled2dMapLayerConfig|getZoomInfo|getCoordinateSystemIdentifier|getBounds|getTileUrl|getLayerName|getZoomLevelInfos" ios/docs/Length of output: 184
Script:
#!/bin/bash # Broaden the search for documentation of methods and properties used in custom layer config across all iOS-related files rg "MCTiled2dMapLayerConfig|getZoomInfo|getCoordinateSystemIdentifier|getBounds|getTileUrl|getLayerName|getZoomLevelInfos" --glob "*.md" --glob "*.swift"Length of output: 5396
android/readme.md (3)
- 68-68: The dependency version for
io.openmobilemaps:mapscore:2.0.0has been updated. Ensure that this version is correctly published on MavenCentral and is accessible for developers.- 87-90: Updated library dependencies for
androidx.activity,androidx.lifecycle,org.jetbrains.kotlinx, andcom.squareup.okhttp3. It's crucial to verify that these updated versions are compatible with each other and with the minimum SDK version supported by the library (Android 8.0+).- 150-221: The custom
Tiled2dLayerConfigexample is comprehensive, covering various aspects of layer configuration. It's important to ensure that the example code is tested and works as expected. Additionally, consider adding comments within the code snippet to explain the purpose of each override and configuration option for better understanding.shared/src/map/camera/MapCamera2d.cpp (4)
- 204-204: Using
std::clampto ensurezoomstays within the bounds ofzoomMaxandzoomMinis a clean and efficient approach. This change simplifies the code and improves readability.- 503-503: The use of
std::clampin theonMovemethod for adjustingzoombased on user interaction is a good practice. It ensures that the zoom level remains within the allowed range in a concise manner.- 601-601: Again, the use of
std::clampfor adjusting the target zoom level in theonDoubleClickmethod is a concise and effective way to maintain the zoom level within the specified bounds.- 629-629: The consistent use of
std::clampto maintain the zoom level within bounds in theonTwoFingerClickmethod is noted and appreciated. This approach enhances code readability and maintainability.
| const auto availableTiles = conversionHelper->convertRect(layerSystemId, *bounds); | ||
|
|
||
| const double tLength = zoomLevelInfo.tileWidthLayerSystemUnits / 256; | ||
| const double tWidthAdj = leftToRight ? tLength : -tLength; | ||
| const double tHeightAdj = topToBottom ? tLength : -tLength; | ||
| const double originX = leftToRight ? zoomLevelInfo.bounds.topLeft.x : -zoomLevelInfo.bounds.bottomRight.x; | ||
| const double originY = topToBottom ? zoomLevelInfo.bounds.bottomRight.y : -zoomLevelInfo.bounds.topLeft.y; | ||
| const double minAvailableX = leftToRight ? std::min(availableTiles.topLeft.x, availableTiles.bottomRight.x) : -std::max(availableTiles.topLeft.x, availableTiles.bottomRight.x); | ||
| const double minAvailableY = topToBottom ? std::min(availableTiles.topLeft.y, availableTiles.bottomRight.y) : -std::max(availableTiles.topLeft.y, availableTiles.bottomRight.y); | ||
| const double maxAvailableX = leftToRight ? std::max(availableTiles.topLeft.x, availableTiles.bottomRight.x) : -std::min(availableTiles.topLeft.x, availableTiles.bottomRight.x); | ||
| const double maxAvailableY = topToBottom ? std::max(availableTiles.topLeft.y, availableTiles.bottomRight.y) : -std::min(availableTiles.topLeft.y, availableTiles.bottomRight.y); | ||
|
|
||
| int min_left_pixel = floor((converted.topLeft.x - zoomLevelInfo.bounds.topLeft.x) / tLength); | ||
|
|
||
| int min_left_pixel = floor((minAvailableX - originX) / tLength); | ||
| int min_left = std::max(0, min_left_pixel / 256); | ||
| int max_top_pixel = floor((zoomLevelInfo.bounds.topLeft.y - converted.topLeft.y) / tLength); | ||
| int max_top = std::min(zoomLevelInfo.numTilesY, max_top_pixel / 256); | ||
| int max_left_pixel = floor((converted.bottomRight.x - zoomLevelInfo.bounds.topLeft.x) / tLength); | ||
|
|
||
| int max_left_pixel = floor((maxAvailableX - originX) / tLength); | ||
| int max_left = std::min(zoomLevelInfo.numTilesX, max_left_pixel / 256); | ||
| int min_top_pixel = floor((zoomLevelInfo.bounds.topLeft.y - converted.bottomRight.y) / tLength); | ||
| int min_top = min_top_pixel / 256; | ||
| min_top = std::max(0, min_top); | ||
|
|
||
| int min_top_pixel = floor((minAvailableY - originY) / tLength); | ||
| int min_top = std::max(0, min_top_pixel / 256); | ||
|
|
||
| int max_top_pixel = floor((maxAvailableY - originY) / tLength); | ||
| int max_top = std::min(zoomLevelInfo.numTilesY, max_top_pixel / 256); |
There was a problem hiding this comment.
The modifications in the tile bounds calculation logic introduce several new variables and calculations to adjust the available tiles based on the layer's bounds and the conversion helper's output. These changes seem to address the need for more precise tile visibility and loading based on dynamic map bounds and zoom levels. However, there are a few points to consider:
-
Correctness and Logic: The logic for calculating
min_left,max_left,min_top, andmax_topappears sound, taking into account the layer's origin and adjusting for the tile's length. This should improve the accuracy of tile selection for rendering. -
Performance: The introduction of additional calculations within the loop for determining visible tiles could impact performance, especially for maps with a large number of tiles. It's important to ensure that these calculations are optimized and that their performance impact is evaluated, particularly for mobile devices with limited resources.
-
Maintainability: The code is relatively complex, with several nested calculations and conditionals. Adding comments to explain the purpose of critical calculations, especially those involving adjustments based on the layer's bounds and origin, would enhance maintainability and readability.
-
Best Practices: The use of
std::minandstd::maxto ensure the calculated tile indices remain within valid ranges is a good practice. However, the magic number256(used in pixel-to-tile conversion) could be defined as a constant with a descriptive name to improve code clarity.
Overall, the changes are aligned with the objectives of enhancing tile mapping and visibility. It would be beneficial to add unit tests covering various scenarios to ensure the correctness of these adjustments, especially in edge cases involving extreme zoom levels or unusual map bounds.
Consider optimizing the new calculations for performance, adding explanatory comments for complex logic, and defining magic numbers as named constants to improve code clarity and maintainability.
| if (zoomMin == -1) { | ||
| // Viewport has not beed set yet | ||
| this->zoom = zoom; | ||
| return; | ||
| } |
There was a problem hiding this comment.
The check for zoomMin == -1 as a condition to determine if the viewport has not been set yet seems to rely on a magic number. It's generally a good practice to define such values as named constants to improve readability and maintainability.
Consider defining a constant for the uninitialized state of zoomMin instead of using -1 directly. This could improve code clarity and make the intention behind the check more explicit.
| ``` | ||
|
|
||
| <h2>How to use</h2> | ||
|
|
||
| ### Initializing the library | ||
|
|
||
| To use the library, it needs to be initialized as early as possible, e.g. in the oOnCreate(), of the hosting Application by calling: | ||
|
|
There was a problem hiding this comment.
The library initialization code snippet is provided without context. It would be beneficial to include a complete example, showing where this code snippet should be placed, such as within the Application class's onCreate method.
| mapView.setupMap(MapConfig(CoordinateSystemFactory.getEpsg3857System())) | ||
| mapView.registerLifecycle(lifecycle) | ||
| ``` | ||
|
|
||
| The map needs to be initialized with a `MapConfig`, which specifies the coordinate system used and the respective bounds and zoom levels that the camera operates in. Additionally, the `MapView` needs to be registered to a lifecycle that also provides a coroutine context, in which the maps tasks can be scheduled (e.g. for tile loading). | ||
| The map needs to be initialized with a `MapConfig`, which specifies the coordinate system that the camera operates in. A popular default is the web-mercator system (EPSG 3857). Additionally, the `MapView` needs to be registered to a lifecycle. |
There was a problem hiding this comment.
The setup instructions for MapView are clear, but it might be helpful to include a note about the necessity of having the appropriate permissions (e.g., Internet, Access Network State) declared in the AndroidManifest.xml file, especially since network requests are likely to be made for map data.
| This MapView can be filled with layers. The simplest case is to add a raster layer (e.g. one containing the data from [OpenStreetMap](https://wiki.openstreetmap.org/)), for which the Tiled2dMapRasterLayer provides a convenience initializer to create raster layer with web mercator tiles. | ||
|
|
||
| ```kotlin | ||
| mapView.setupMap(MapConfig(CoordinateSystemFactory.getEpsg3857System())) | ||
| val tiledRasterLayer = TiledRasterLayer() | ||
| ``` | ||
|
|
||
| To display the tiles, a Tiled2dMapRasterLayer must be created with both a Tiled2dMapLayerConfig and the implementation of a TextureLoader. | ||
| Per default, the above constructor uses the default implementation of the `DataLoader` with some default parameters, which uses OkHttp to load a bitmap from a given URL. It can be instantiated and supplied to the `TiledRasterLayer` with additional parameter configurations. Of course, even a fully custom implementation of the `LoaderInterface` can be used as well. For example: | ||
|
|
||
| ```kotlin | ||
| val tiledLayer = Tiled2dMapRasterLayerInterface.create(layerConfig, textureLoader) | ||
| val dataLoader = DataLoader(this, cacheDir, 50L * 1024L * 1024L, "example-referrer") | ||
| ``` | ||
|
|
||
| Open Maps Mobile provides a default implementation for a `DataLoader`, which uses OkHttp to load a bitmap from a given URL. Of course, a custom implementation of the `LoaderInterface` can be used as well. | ||
| For a more custom layer creation, you can directly use the methods provided within the `Tiled2dRasterLayerInterface`. Finally, the layer reference can be added to the MapView. | ||
|
|
||
| ```kotlin | ||
| val textureLoader = DataLoader(this, cacheDir, 50L * 1024L * 1024L) | ||
| mapView.addLayer(tiledRasterLayer) |
There was a problem hiding this comment.
The example for displaying a tiled raster map is straightforward. However, it would be beneficial to mention the source of the tiles (e.g., OpenStreetMap) explicitly in the documentation to clarify the default behavior and any legal or attribution requirements.
| @@ -10,7 +10,7 @@ - (nonnull instancetype)initWithIdentifier:(nonnull NSString *)identifier | |||
| title:(nullable NSString *)title | |||
| abstractText:(nullable NSString *)abstractText | |||
| dimensions:(nonnull NSArray<MCWmtsLayerDimension *> *)dimensions | |||
| bounds:(nonnull MCRectCoord *)bounds | |||
| bounds:(nullable MCRectCoord *)bounds | |||
There was a problem hiding this comment.
The implementation changes in initWithIdentifier and wmtsLayerDescriptionWithIdentifier methods to support nullable bounds are consistent with the header file modifications. This allows for more flexible handling of layer descriptions, especially in scenarios where bounds might not be defined. Ensure that the application logic that interacts with instances of MCWmtsLayerDescription is updated to handle nullable bounds appropriately.
Also applies to: 35-35
| MCCoordinateSystemIdentifiers.epsg3857() | ||
| } | ||
|
|
||
| // Defines the bounds of the layer | ||
| func getBounds() -> MCRectCoord { | ||
| let identifer = MCCoordinateSystemIdentifiers.epsg3857() | ||
| let topLeft = MCCoord(systemIdentifier: identifer, | ||
| x: -20037508.34, | ||
| y: 20037508.34, z: 0.0) | ||
| let bottomRight = MCCoord(systemIdentifier: identifer, | ||
| x: 20037508.34, | ||
| y: -20037508.34, z: 0.0) | ||
| return MCRectCoord( | ||
| topLeft: topLeft, | ||
| bottomRight: bottomRight) | ||
| } | ||
|
|
||
| // Defines the url-pattern to load tiles. Enter a valid OSM tile server here | ||
| func getTileUrl(_ x: Int32, y: Int32, zoom: Int32) -> String { | ||
| return "https://example.com/tiles/\(zoom)/\(x)/\(y).png" | ||
| } | ||
|
|
||
| // The Layername | ||
| func getLayerName() -> String { | ||
| "OSM Layer" | ||
| } | ||
|
|
||
| // List of valid zoom-levels and their target zoom-value, the tile size in | ||
| // the layers coordinate system, the number of tiles on that level and the | ||
| // zoom identifier used for the tile-url (see getTileUrl above) | ||
| func getZoomLevelInfos() -> [MCTiled2dMapZoomLevelInfo] { | ||
| [ | ||
| .init(zoom: 559082264.029, tileWidthLayerSystemUnits: 40_075_016, numTilesX: 1, numTilesY: 1, numTilesT: 1, zoomLevelIdentifier: 0, bounds: getBounds()), | ||
| .init(zoom: 279541132.015, tileWidthLayerSystemUnits: 20_037_508, numTilesX: 2, numTilesY: 2, numTilesT: 1, zoomLevelIdentifier: 1, bounds: getBounds()), | ||
| .init(zoom: 139770566.007, tileWidthLayerSystemUnits: 10_018_754, numTilesX: 4, numTilesY: 4, numTilesT: 1, zoomLevelIdentifier: 2, bounds: getBounds()), | ||
| .init(zoom: 69885283.0036, tileWidthLayerSystemUnits: 5_009_377.1, numTilesX: 8, numTilesY: 8, numTilesT: 1, zoomLevelIdentifier: 3, bounds: getBounds()), | ||
| .init(zoom: 34942641.5018, tileWidthLayerSystemUnits: 2_504_688.5, numTilesX: 16, numTilesY: 16, numTilesT: 1, zoomLevelIdentifier: 4, bounds: getBounds()), | ||
| .init(zoom: 17471320.7509, tileWidthLayerSystemUnits: 1_252_344.3, numTilesX: 32, numTilesY: 32, numTilesT: 1, zoomLevelIdentifier: 5, bounds: getBounds()), | ||
| .init(zoom: 8735660.37545, tileWidthLayerSystemUnits: 626_172.1, numTilesX: 64, numTilesY: 64, numTilesT: 1, zoomLevelIdentifier: 6, bounds: getBounds()), | ||
| .init(zoom: 4367830.18773, tileWidthLayerSystemUnits: 313_086.1, numTilesX: 128, numTilesY: 128, numTilesT: 1, zoomLevelIdentifier: 7, bounds: getBounds()), | ||
| .init(zoom: 2183915.09386, tileWidthLayerSystemUnits: 156_543, numTilesX: 256, numTilesY: 256, numTilesT: 1, zoomLevelIdentifier: 8, bounds: getBounds()), | ||
| .init(zoom: 1091957.54693, tileWidthLayerSystemUnits: 78271.5, numTilesX: 512, numTilesY: 512, numTilesT: 1, zoomLevelIdentifier: 9, bounds: getBounds()), | ||
| .init(zoom: 545978.773466, tileWidthLayerSystemUnits: 39135.8, numTilesX: 1024, numTilesY: 1024, numTilesT: 1, zoomLevelIdentifier: 10, bounds: getBounds()), | ||
| .init(zoom: 272989.386733, tileWidthLayerSystemUnits: 19567.9, numTilesX: 2048, numTilesY: 2048, numTilesT: 1, zoomLevelIdentifier: 11, bounds: getBounds()), | ||
| .init(zoom: 136494.693366, tileWidthLayerSystemUnits: 9783.94, numTilesX: 4096, numTilesY: 4096, numTilesT: 1, zoomLevelIdentifier: 12, bounds: getBounds()), | ||
| .init(zoom: 68247.3466832, tileWidthLayerSystemUnits: 4891.97, numTilesX: 8192, numTilesY: 8192, numTilesT: 1, zoomLevelIdentifier: 13, bounds: getBounds()), | ||
| .init(zoom: 34123.6733416, tileWidthLayerSystemUnits: 2445.98, numTilesX: 16384, numTilesY: 16384, numTilesT: 1, zoomLevelIdentifier: 14, bounds: getBounds()), | ||
| .init(zoom: 17061.8366708, tileWidthLayerSystemUnits: 1222.99, numTilesX: 32768, numTilesY: 32768, numTilesT: 1, zoomLevelIdentifier: 15, bounds: getBounds()), | ||
| .init(zoom: 8530.91833540, tileWidthLayerSystemUnits: 611.496, numTilesX: 65536, numTilesY: 65536, numTilesT: 1, zoomLevelIdentifier: 16, bounds: getBounds()), | ||
| .init(zoom: 4265.45916770, tileWidthLayerSystemUnits: 305.748, numTilesX: 131_072, numTilesY: 131_072, numTilesT: 1, zoomLevelIdentifier: 17, bounds: getBounds()), | ||
| .init(zoom: 2132.72958385, tileWidthLayerSystemUnits: 152.874, numTilesX: 262_144, numTilesY: 262_144, numTilesT: 1, zoomLevelIdentifier: 18, bounds: getBounds()), | ||
| .init(zoom: 1066.36479193, tileWidthLayerSystemUnits: 76.437, numTilesX: 524_288, numTilesY: 524_288, numTilesT: 1, zoomLevelIdentifier: 19, bounds: getBounds()), | ||
| .init(zoom: 533.18239597, tileWidthLayerSystemUnits: 38.2185, numTilesX: 1_048_576, numTilesY: 1_048_576, numTilesT: 1, zoomLevelIdentifier: 20, bounds: getBounds()), | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| #### MapViewController | ||
|
|
||
| Once we have the layer config, we can add a map view to our view controller. | ||
|
|
||
| The library provides a factory for the [EPSG3857](https://epsg.io/4326) Coordinate system, which we can use to initialize the map view. For this example, we also use the default implementation of the [TextureLoader](https://github.com/openmobilemaps/maps-core/blob/develop/ios/maps/MCTextureLoader.swift), but this can also be implemented by the app itself. | ||
| The framework provides a view that can be filled with layers. The simplest case is to add a raster layer. TiledRasterLayer provides a convenience initializer to create raster layer with web mercator tiles. | ||
|
|
||
|
|
||
| ```swift | ||
| import MapCore | ||
| import MapCoreSharedModule | ||
|
|
||
| class MapViewController: UIViewController { | ||
| var mapConfig = MCMapConfig(mapCoordinateSystem: MCCoordinateSystemFactory.getEpsg3857System()) | ||
|
|
||
| lazy var mapView = MCMapView(mapConfig: mapConfig) | ||
|
|
||
| lazy var loader = MCTextureLoader() | ||
|
|
||
| lazy var rasterLayer = MCTiled2dMapRasterLayerInterface.create(TiledLayerConfig(), | ||
| loader: loader) | ||
|
|
||
| lazy var mapView = MCMapView() | ||
| override func loadView() { view = mapView } | ||
|
|
||
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
|
|
||
| mapView.add(layer: rasterLayer?.asLayerInterface()) | ||
| mapView.add(layer: TiledRasterLayer("osm", webMercatorUrlFormat: "https://tiles.sample.org/{z}/{x}/{y}.png")) | ||
|
|
||
| mapView.camera.move(toCenterPositionZoom: MCCoord(lat: 46.962592372639634, lon: 8.378232525377973), zoom: 1000000, animated: true) | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Parsing a WMTS Capability | ||
|
|
||
| #### Parsing a WMTS Capability | ||
|
|
||
| Open Mobile Maps supports the [WMTS standard](https://en.wikipedia.org/wiki/Web_Map_Tile_Service) and can parse their Capability XML file to generate raster layer configurations. | ||
|
|
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [66-81]
The section on parsing a WMTS Capability XML file to generate raster layer configurations is informative. However, it mentions that this feature is still being improved. It would be beneficial to provide a link to the documentation or issue tracker where developers can follow updates on this feature.
Would you like me to add a link to the documentation or create an issue for tracking improvements to the WMTS capability parsing feature?
| ### Vector Tiles | ||
|
|
||
| Open Mobile Maps supports most of the [Vector tiles standard](https://docs.mapbox.com/data/tilesets/guides/vector-tiles-standards/). To add a layer simply reference the style URL. | ||
|
|
||
| ```swift | ||
| mapView.add(layer: try! VectorLayer("base-map", styleURL: "https://www.sample.org/base-map/style.json") | ||
| ``` | ||
|
|
||
| Additional features and differences will be documented soon. |
There was a problem hiding this comment.
The introduction of support for Vector Tiles is a significant enhancement. It's mentioned that additional features and differences will be documented soon. Ensuring timely documentation updates is crucial for developers to fully leverage this feature.
Do you need assistance in documenting the additional features and differences for Vector Tiles support?
| ``` | ||
| This feature is still being improved to support a wider range of WMTS capabilities. | ||
|
|
||
| ### Polygon layer | ||
| For this example, we also use the default implementation of the [TextureLoader](https://github.com/openmobilemaps/maps-core/blob/develop/ios/maps/MCTextureLoader.swift), but this can also be implemented by the app itself. | ||
|
|
||
|
|
||
| ### Vector Tiles | ||
|
|
||
| Open Mobile Maps supports most of the [Vector tiles standard](https://docs.mapbox.com/data/tilesets/guides/vector-tiles-standards/). To add a layer simply reference the style URL. | ||
|
|
||
| ```swift | ||
| mapView.add(layer: try! VectorLayer("base-map", styleURL: "https://www.sample.org/base-map/style.json") | ||
| ``` | ||
|
|
||
| Additional features and differences will be documented soon. | ||
|
|
||
|
|
||
|
|
||
| ### Overlays | ||
|
|
||
| #### Polygon layer | ||
|
|
||
| Open Mobile Maps provides a simple interface to create a polygon layer. The layer handles the rendering of the given polygons and calls the callback handler in case of user interaction. | ||
|
|
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [96-117]
The example for creating and adding a polygon layer is clear and demonstrates how to use the MCPolygonLayerInterface effectively. Including user interaction handling in the example would provide a more comprehensive guide.
| mapView.add(layer: polygonLayer?.asLayerInterface()) | ||
| ``` | ||
|
|
||
| ### Icon layer | ||
| #### Icon layer | ||
|
|
||
| A simple icon layer is implemented as well. This supports displaying textures at the given coordinates. A scale parameter has to be provided which specifies how the icon should be affected by camera movements. In case of user interaction, the given callback handler will be called. | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate extensive references to both TextureHolder and MCIconFactory across various parts of the codebase, including iOS-specific implementations (TextureHolder.swift, MCIconFactory.h, and related files), shared code (shared/public/TextureHolderInterface.h, shared/public/IconFactory.h), and even Android-specific files. This suggests that these classes are integral parts of the application's architecture, dealing with texture handling and icon creation functionalities, respectively.
However, the search results do not directly confirm the presence of documentation for these classes within the repository. The references found are primarily from source code files, headers, and implementation details, rather than explicit documentation files or sections within a documentation directory. This could mean that while the classes are widely used and implemented across the platform, their documentation might not be readily available in the searched locations or might be included within the code files themselves rather than separate documentation resources.
Given the context of the review comment, which emphasizes the importance of having TextureHolder and MCIconFactory classes explained somewhere in the documentation for developers unfamiliar with these APIs, the absence of explicit documentation files in the search results suggests that the documentation might not be as accessible or detailed as it could be for these specific classes.
Considering the evidence from the search results and the context provided by the review comment, it seems appropriate to conclude that while the classes are indeed integral to the application and references to them are abundant in the codebase, the specific request for accessible documentation in a dedicated location has not been confirmed through this search.
Therefore, the review comment stands as a valid suggestion for improving the documentation, especially for developers new to the project or those not familiar with the intricacies of TextureHolder and MCIconFactory.
Analysis chain
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [117-136]
The icon layer example is well-documented, showing how to display textures at specific coordinates. It's important to ensure that the TextureHolder and MCIconFactory classes are explained somewhere in the documentation for developers unfamiliar with these APIs.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if TextureHolder and MCIconFactory classes are documented
rg "TextureHolder|MCIconFactory" ios/docs/
Length of output: 94
Script:
#!/bin/bash
# Broaden the search for TextureHolder and MCIconFactory documentation across the entire repository
rg "TextureHolder|MCIconFactory"
Length of output: 62901
Summary by CodeRabbit
TiledRasterLayerandTiledVectorLayerclasses for enhanced map layer handling.MapConfigand improved display of tiled maps.DataLoaderto have a default empty string for thereferrerproperty.WmtsLayerDescriptionto allow nullableboundsproperty.MapCamera2dto usestd::clamp.android/readme.mdandios/readme.md.FontLoader.Tiled2dMapSourceImpl.readme.mdby updating supported iOS version and refining the "Features" section.