-
-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom drawable layer v3 #3210
base: main
Are you sure you want to change the base?
Custom drawable layer v3 #3210
Conversation
…ocaru/maplibre-native into custom-drawable-layer
Bloaty Results 🐋Compared to main
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-3210-compared-to-main.txtCompared to d387090 (legacy)
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-3210-compared-to-legacy.txt |
Benchmark Results ⚡
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/benchmark-results/pr-3210-compared-to-main.txt |
I like the changes to the Custom Drawable interface and the example. How did you end up feeling about the OBJ reader? |
for more information, see https://pre-commit.ci
In the example the OBJ reader ended up being used with a sphere and it's mostly just a proof of concept. It can be extended later to use vertex color and normals from the file. |
Bloaty Results (iOS) 🐋Compared to main
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-3210-compared-to-main.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding a note to the Developer Docs in docs/mdbook
with a screenshot and a few words, just to make people know this exists?
You can drag the screenshot in any GitHub comment so we let GitHub host it for us.
Maybe in a separate PR if you would just like to get this merged first?
I get a crash with Metal when built via Bazel...?
|
I can add the motivation behind it and the intended usage (would prefer a different PR but it's ok either way).
If it's an exception when enabling the custom drawable layer ( |
Update for
CustomDrawableLayer
interface and existing example.removeDrawable
for existing drawables (currently the only way to update vertex geometry is to remove and re-add the drawable)setGeometryOptions
/addGeometry
for uploading externally generated geometry (examplegenerateGeometry
andloadGeometry
using tinyobjloader)CustomDrawableLayer
tweakers to work with consolidated UBOs