search

GeoTIFF / georaster-layer-for-leaflet

Display GeoTIFFs and soon other types of raster on your Leaflet Map
https://geotiff.github.io/georaster-layer-for-leaflet-example/
Apache License 2.0
286 stars 57 forks source link

Extra props passed to custom draw function #27

Closed scazz010 closed 4 years ago

scazz010 commented 4 years ago

Hi again!

I'm using the library to render a DEM so my raster values represent altitude at each point.

I'm adding hill shading to a layer. So, I hook into the customDrawFunction - except I don't get access to the entire raster here and I need the surrounding pixels to determine slop angle and aspect

https://github.com/GeoTIFF/georaster-layer-for-leaflet/blob/34cb38957822642be285eff1fd272ea8d02a8020/georaster-layer-for-leaflet.js#L253

Describe the solution you'd like Would be great if you could also pass the w + h variables, along with the raster being rendered to the customDrawFunction

References: https://observablehq.com/@sahilchinoy/hillshader

As before, I've solved for me - but I guess this could be useful to others!

I'd be happy to open a PR if you think the extra props is sensible.....
I could see people misusing it and calculating min/max values for the entire raster every time a pixel is drawn, but you can't protect everyone!

Thanks again :) S

DanielJDufour commented 4 years ago

Hi. A PR would be most welcome! Perhaps we should say rasterX and rasterY for the new variables? I'm not sure we need to return a reference to the georaster object, because presumably the developer would already have access to this. But can't hurt to include it! Thank you!

scazz010 commented 4 years ago

I think the developer has the entire georaster object, but if it's a cogeo optimized tiff, the getRasters function creates a set of options for sampling which are then passed to getValues

We then get this much lighter raster which is the raster I'm proposing to pass down into the customDrawFunction, along with the rasterX and rasterY coordinates.

just checking I'm not talking nonsense and that adding those values is required for my use-case/still makes sense? It's a little bit of a separate story for cogeo tiffs I guess?

https://github.com/GeoTIFF/georaster-layer-for-leaflet/blob/34cb38957822642be285eff1fd272ea8d02a8020/georaster-layer-for-leaflet.js#L64

DanielJDufour commented 4 years ago

Excellent Point! Yes, let's include it :-) What would be a good name for the variable? The source code uses tileRasters, but I'm afraid that variable name isn't very clear to people who might like to use this library. Open to your thoughts :-)

scazz010 commented 4 years ago

https://github.com/GeoTIFF/georaster-layer-for-leaflet/blob/34cb38957822642be285eff1fd272ea8d02a8020/georaster-layer-for-leaflet.js#L248

You've commented sampledRaster - which I quite like! I was going to suggest renderedRaster or something of that ilk. I'm going to to with sampledRaster, but feel free to change it. Been busy, but I'll get a PR in today (hopefully ;) )

DanielJDufour commented 4 years ago

Hi, @scazz010 . Let me know if you want me to submit a PR for this. (Looks like I might have some extra free-time in January).

DanielJDufour commented 4 years ago

Hi, @scazz010 . I updated the customDrawFunction to include the requested params. Please see https://github.com/GeoTIFF/georaster-layer-for-leaflet/blob/master/ADVANCED.md#custom-draw-function. Thank you again for the great suggestion!