-
Notifications
You must be signed in to change notification settings - Fork 139
feat: added support for GeometryCollections #1384
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
5ac724a
4dfb37c
b84190c
0a57f83
21c2091
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,12 @@ import { | |
| v8 as spec, | ||
| } from '@maplibre/maplibre-gl-style-spec'; | ||
| import mb2css from 'mapbox-to-css-font'; | ||
| import {Feature} from 'ol'; | ||
| import Map from 'ol/Map.js'; | ||
| import {distance} from 'ol/coordinate.js'; | ||
| import {getCenter} from 'ol/extent.js'; | ||
| import {toPromise} from 'ol/functions.js'; | ||
| import {GeometryCollection} from 'ol/geom.js'; | ||
| import RenderFeature from 'ol/render/Feature.js'; | ||
| import Circle from 'ol/style/Circle.js'; | ||
| import Fill from 'ol/style/Fill.js'; | ||
|
|
@@ -554,7 +556,28 @@ export function stylefunction( | |
| cameraObj.zoom = zoom; | ||
| cameraObj.distanceFromCenter = 0; | ||
| const featureGeometry = feature.getGeometry(); | ||
|
|
||
| if (featureGeometry instanceof GeometryCollection) { | ||
| const geoms = featureGeometry.getGeometries(); | ||
|
|
||
| for (const geom of geoms) { | ||
| const subFeature = new Feature({ | ||
| ...feature.getProperties(), | ||
shaietz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| geometry: geom, | ||
| }); | ||
|
|
||
| const subStyles = styleFunction(subFeature, resolution, onlyLayer); | ||
|
|
||
| if (subStyles && Array.isArray(subStyles)) { | ||
shaietz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| styles.push(...subStyles); | ||
| } | ||
| } | ||
|
|
||
| return styles.length > 0 ? styles : undefined; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This won't work. Styles are reused across
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm not sure i understand what you mean, anyways i have removed the push of the substyle to the styles array because it was not necessary, so is this still relevant?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, what I wrote is still relevant. Also regarding a test case with nested geometry collections.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hi, sorry for the long break, i ended up not needing this feature which is the reason i opened the issue in the first place, however i have some free time now and want to finish this up.
i don't understand why this won't work and what needs to be done in order for it to work, since i call the styleFunction() recursively for each sub geometry shouldn't it handle handle adding the individual styles to the styles array correctly? also, i added a nested geometry collection to the geojson-inline.json and it seems to be working correctly, should i also add a test to stylefunction.test.js? thanks for your patience
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason why it appears to work is because OpenLayers takes care of correct rendering of all points of the GeometryCollection, and the last style will be used. With geometry collections with mixed types, that won't work so well. Honestly, I cannot think of a way to deal with this properly. Maybe taking a look at how MapLibre deals with GeometryCollection geometries from GeoJSON sources could make sense. |
||
| } | ||
|
|
||
| const type = types[featureGeometry.getType()]; | ||
|
|
||
| const map = olLayer.get('map'); | ||
| if (map && map instanceof Map && type === 1) { | ||
| const size = map.getSize(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.