Conversation
stmitt
requested review from
maerki,
matzf,
maurhofer-ubique and
zimmermannubique
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fd41f6efb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| std::lock_guard<std::recursive_mutex> lock(self->mutex); | ||
| self->loadingResult = DataLoaderResult(std::nullopt, std::nullopt, LoaderStatus::ERROR_OTHER, "parse error"); | ||
| LogError << "Unable to parse geobuf: " <<= ex.what(); | ||
| return; |
There was a problem hiding this comment.
In the Geobuf branch, parse errors set loadingResult and then return from the async callback, which skips the resolveAllWaitingPromises() call at the bottom of load(). Any caller waiting via waitIfNotLoaded() will block indefinitely when a source returns malformed Geobuf, so the layer can hang instead of reporting a completed failure state.
Useful? React with 👍 / 👎.
| } | ||
| properties[context.keys[keyIndex]] = context.values[valueIndex]; | ||
| } | ||
| context.values.clear(); |
There was a problem hiding this comment.
readProperties always clears context.values, but the decoder supports both properties (tag 14) and custom_properties (tag 15) within the same Geobuf message. If both are present, decoding the first one empties the value table and the second one can no longer resolve its value indexes, causing those fields to be silently dropped.
Useful? React with 👍 / 👎.
|
🚨 Performance Regression Detected! 🚨 Some benchmarks have worsened in this PR. |
1 participant
Adds support for geobuf as a style.json source.