Release 3.0.1 #693
Release 3.0.1 #693
Conversation
…style jsons, increase geojson extent to uint32_t, ensure proper tile reload check after geoJson-source did load
fixes future exception
WalkthroughThis pull request updates the project to version 3.0.1. It introduces a new feature that allows GeoJSON sources to specify Changes
Sequence Diagram(s)sequenceDiagram
participant JSON as Style JSON Parser
participant Parser as Tiled2dMapVectorLayerParserHelper
participant Config as Tiled2dMapVectorGeoJSONLayerConfig
participant Loader as GeoJSON Data Loader
JSON->>Parser: Parse style JSON (including minZoom & extent)
Parser-->>Config: Pass extracted options (minZoom, extent, etc.)
Config->>Loader: Configure GeoJSON source with options
Loader-->>Config: Load data asynchronously with error handling
sequenceDiagram
participant Caller as Async Caller
participant Manager as VectorLayer Source Manager
participant Thread as Worker Thread
Caller->>Manager: Request tile information
Manager->>Thread: Invoke method with thread_local variables
Thread-->>Manager: Process tile data with error handling (try-catch)
Manager-->>Caller: Return processed data or log error
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
android/src/main/java/io/openmobilemaps/mapscore/map/util/AssetLocalDataProvider.kt (1)
106-106: Great addition of error logging!Adding this log statement improves debugging by providing visibility when GeoJSON loading fails due to a missing data loader. This follows the error logging pattern used elsewhere in the class.
For even better alignment, consider using
LoaderStatus.ERROR_OTHERinstead ofERROR_404on line 111, as this is more of a configuration issue (missing loader) than a "not found" situation.CHANGELOG.md (1)
3-8: Fix grammatical error in changelog.There's a grammatical error in the last bullet point.
-Catch c++ future exception if the promise is deallocated while the its not fulfilled +Catch c++ future exception if the promise is deallocated while it's not fulfilled🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: A determiner cannot be combined with a possessive pronoun. Did you mean simply “the” or “its”?
Context: ...ion if the promise is deallocated while the its not fulfilled ## Version 3.0.0 - Intro...(A_MY)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG.md(1 hunks)android/gradle.properties(1 hunks)android/readme.md(1 hunks)android/src/main/java/io/openmobilemaps/mapscore/map/util/AssetLocalDataProvider.kt(1 hunks)ios/graphics/Model/Polygon/PolygonGroup2d.swift(1 hunks)ios/graphics/Model/Polygon/PolygonPatternGroup2d.swift(1 hunks)ios/readme.md(1 hunks)shared/public/Actor.h(6 hunks)shared/public/LoaderHelper.h(1 hunks)shared/src/MapsCoreSharedModule.cpp(1 hunks)shared/src/map/layers/effect/SphereEffectLayer.cpp(1 hunks)shared/src/map/layers/tiled/vector/Tiled2dMapVectorGeoJSONLayerConfig.h(1 hunks)shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayer.cpp(3 hunks)shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayerParserHelper.cpp(1 hunks)shared/src/map/layers/tiled/vector/geojson/Tiled2dVectorGeoJsonSource.h(1 hunks)shared/src/map/layers/tiled/vector/geojson/geojsonvt/geojsonvt.hpp(2 hunks)shared/src/map/layers/tiled/vector/geojson/geojsonvt/tile.hpp(2 hunks)shared/src/map/layers/tiled/vector/sourcemanagers/Tiled2dMapVectorSourceDataManager.cpp(1 hunks)shared/src/map/layers/tiled/vector/sourcemanagers/Tiled2dMapVectorSourceRasterTileDataManager.cpp(2 hunks)shared/src/map/layers/tiled/vector/symbol/Tiled2dMapVectorSymbolLabelObject.cpp(1 hunks)shared/src/map/layers/tiled/vector/tiles/polygon/Tiled2dMapVectorPolygonTile.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- android/gradle.properties
- android/readme.md
- shared/src/MapsCoreSharedModule.cpp
- ios/readme.md
🧰 Additional context used
🪛 Cppcheck (2.10-2)
shared/src/map/layers/tiled/vector/sourcemanagers/Tiled2dMapVectorSourceRasterTileDataManager.cpp
[error] 199-199: syntax error
(syntaxError)
shared/src/map/layers/tiled/vector/sourcemanagers/Tiled2dMapVectorSourceDataManager.cpp
[error] 46-46: syntax error
(syntaxError)
shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayer.cpp
[error] 1162-1162: syntax error
(syntaxError)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~7-~7: A determiner cannot be combined with a possessive pronoun. Did you mean simply “the” or “its”?
Context: ...ion if the promise is deallocated while the its not fulfilled ## Version 3.0.0 - Intro...
(A_MY)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (20)
shared/src/map/layers/tiled/vector/geojson/Tiled2dVectorGeoJsonSource.h (1)
69-69: LGTM: Properly initializes lastVisibleTilesHashThe initialization of
lastVisibleTilesHashto 0 is appropriate as mentioned in the comment on line 68, since there's no concept of curT for GeoJSON.shared/src/map/layers/tiled/vector/sourcemanagers/Tiled2dMapVectorSourceDataManager.cpp (1)
43-47: Improved error handling for future resultsGood addition of try-catch block that prevents unhandled exceptions when getting the future result from the
readyManager. This enhances the robustness of the constructor.Note: The static analysis warning about a syntax error on line 46 appears to be a false positive. The
LogError << "message" <<= e.what()syntax seems to be part of the project's custom logging framework.🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 46-46: syntax error
(syntaxError)
shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayerParserHelper.cpp (1)
204-206: Added support for 'extent' property in GeoJSON sourcesThis addition completes the feature that allows GeoJSON sources to specify
extentoptions in style JSON files. The implementation follows the same pattern as the existing code forminzoomandmaxzoomproperties, and correctly usesuint32_tto support larger values.shared/public/Actor.h (1)
90-90: Enhanced error logging for better diagnostics.The error logging for
WeakActornull pointer conditions now includes additional diagnostic information fromfn.diagnostics.toString(). This provides more context when debugging issues where the actor's object or mailbox is no longer available.Also applies to: 101-101, 112-112, 123-123, 141-141, 159-159
shared/src/map/layers/tiled/vector/Tiled2dMapVectorGeoJSONLayerConfig.h (1)
28-28: Added support for minZoom in GeoJSON sources.This implements the new feature mentioned in the changelog that allows GeoJSON sources to specify minimum zoom levels. The configuration now properly reads the
minZoomfrom the GeoJSON data and uses it when generating zoom level information instead of using a hardcoded value of 0.Also applies to: 30-31, 33-33
shared/src/map/layers/tiled/vector/tiles/polygon/Tiled2dMapVectorPolygonTile.cpp (1)
102-102: Memory optimization for striped polygon renderingThis change optimizes memory allocation by conditionally reserving the appropriate amount of space for shader styles based on whether the polygon is striped. For striped polygons, two additional float values per feature are needed to store the stripe width.
shared/src/map/layers/tiled/vector/sourcemanagers/Tiled2dMapVectorSourceRasterTileDataManager.cpp (2)
195-201: Enhanced error handling for asynchronous operationsThe added try-catch block properly handles potential exceptions when retrieving tile data from the raster source. This prevents crashes and allows for graceful degradation when the future operation fails.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 199-199: syntax error
(syntaxError)
306-312: Enhanced error handling for asynchronous operationsSimilar to the previous change, this try-catch block provides robust error handling for the asynchronous operation in
reloadLayerContent. Good practice to ensure consistent error handling across similar code patterns.shared/src/map/layers/tiled/vector/symbol/Tiled2dMapVectorSymbolLabelObject.cpp (1)
351-352: Improved thread safety and performance for baseline vectorThe change from static to thread_local storage improves thread safety by ensuring each thread has its own vector instance. Additionally, using resize() with an initial value is more efficient than repeatedly pushing back values in a loop.
ios/graphics/Model/Polygon/PolygonGroup2d.swift (1)
139-146: Performance optimization for vertex processingThe implementation has been improved to leverage SIMD more effectively by directly processing vertex data as SIMD4 instead of individual components. This reduces pointer arithmetic and conditional checks, resulting in more efficient code.
ios/graphics/Model/Polygon/PolygonPatternGroup2d.swift (1)
166-173: Well-optimized SIMD code!This change efficiently processes vertex data by using SIMD4 instead of individual Float values. This approach reduces the number of operations needed and improves memory access patterns by reading 4 components at once.
shared/src/map/layers/tiled/vector/geojson/geojsonvt/tile.hpp (2)
22-22: Good type expansion to support larger extents.Changing from
uint16_ttouint32_tallows for much larger tile extents (up to 4,294,967,295 instead of 65,535), which is needed for the new feature allowing GeoJSON sources to specify custom extents in style JSON files.
41-41: Type consistency maintained in constructor.Correctly updated the constructor parameter type to match the member variable change, ensuring type consistency throughout the class definition.
shared/src/map/layers/tiled/vector/geojson/geojsonvt/geojsonvt.hpp (3)
22-22: Good type expansion for extent.Changing from
uint16_ttouint32_tallows for much larger tile extents, consistent with the changes in tile.hpp.
25-25: Good type expansion for buffer.Changing the buffer type from
uint16_ttouint32_tensures consistency with the extent type and allows for larger buffer sizes.
90-90: Improved error handling for loader results.This change properly captures the status and error code from the result into the loadingResult, which ensures that error information is correctly preserved for error handling.
shared/src/map/layers/tiled/vector/Tiled2dMapVectorLayer.cpp (3)
670-670: Thread-safety improvement.Replacing
staticwiththread_localensures each thread has its own instance of the vector, preventing potential race conditions in multi-threaded contexts. This is an important improvement for thread safety.
1158-1164: Enhanced error handling for tile information retrieval.Added proper exception handling when retrieving current tile information. If an exception occurs, it logs the error and continues with the next source, improving the robustness of the application.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 1162-1162: syntax error
(syntaxError)
1399-1405: Consistent error handling pattern.Applied the same error handling pattern as in getFeatureContext, maintaining code consistency while improving robustness.
shared/src/map/layers/effect/SphereEffectLayer.cpp (1)
77-78: Good improvement for thread safety!Converting these vectors from
statictothread_localstorage is an excellent change. This ensures each thread has its own instance of the coefficient vectors, eliminating potential data races that could occur if multiple threads callupdate()concurrently. The initialization and usage patterns remain unchanged, but the code is now properly thread-safe.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor