feat(#123): added-iconlayer-support #129
Conversation
| | "getRadius" | ||
| | "getFillColor" | ||
| | "getLineColor" |
There was a problem hiding this comment.
These don't exist on IconLayerProps
| /** | ||
| * Radius accessor. | ||
| * @default 1 | ||
| */ | ||
| getRadius?: FloatAccessor; | ||
| /** | ||
| * Fill color accessor. | ||
| * @default [0, 0, 0, 255] | ||
| */ | ||
| getFillColor?: ColorAccessor; | ||
| /** | ||
| * Stroke color accessor. | ||
| * @default [0, 0, 0, 255] | ||
| */ | ||
| getLineColor?: ColorAccessor; | ||
| /** | ||
| * Stroke width accessor. | ||
| * @default 1 | ||
| */ | ||
| getLineWidth?: FloatAccessor; |
There was a problem hiding this comment.
These don't make sense for IconLayer
| * If not provided, will be inferred by finding a column with extension type | ||
| * `"geoarrow.point"` or `"geoarrow.multipoint"`. | ||
| */ | ||
| getPosition?: ga.vector.PointVector | ga.vector.MultiPointVector; |
There was a problem hiding this comment.
For now, you should remove | ga.vector.MultiPointVector since this layer as written only supports points
| | "iconAtlas" | ||
| | "iconMapping" |
There was a problem hiding this comment.
We only want to omit the properties that we redefine. It doesn't look like we redefine these, so you don't want to remove these from the type.
There was a problem hiding this comment.
That said we do want to redefine any accessors. So we would usually want to add getIcon, getSize, getColor, getAngle, getPixelOffset. We need to check that deck works with a vectorized getIcon function though. No other layers have a string return type from a get* accessor
|
I haven't had time to organize it yet, but in theory development here might be moving to a semi-official deck.gl fork in https://github.com/visgl/deck.gl-community |
|
@kylebarron thanks! But as part of moving to community layers repository! Is this something planned to happend per each geoarrow layer separately? Or is the plan to transfer the entire set of the geoarrow layers there all at once? |
It was already forked in visgl/deck.gl-community#67 and the existing layers are all at https://github.com/visgl/deck.gl-community/tree/master/modules/arrow-layers. But I haven't had time to actually test using them in my code (in Lonboard), and no one else is working on this right now. If you were interested, you could try out using the |
|
@kylebarron ok so I will check that version! |
|
Yeah that sounds great |
iconAtlastoiconAtlasConfig. For some reasoniconAtlasparam is overwritten in the parent class. I assume it is due to the property type ofimageand since it also acceptsstringand this causes the overwritten. I am not very familiar with deckgl's layer param validations if you can guide me to fix it I can finish this PR.I assume a multipoint support can also be added here. maybe in the next PR.
Any additional comments are welcomed. Thanks!