Add support for cumulative viewshed #10674
Conversation
This is a spelling issue (spellchecking of doc is a recent addition in master): " source/programs/gdal_viewshed.rst:156: : Spell check: ulative: (CUM)ulative mode will create an eight bit raster the same size as the input raster" |
ah sorry, there's one indeed actually. You likely need to tune the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
alg/viewshed/cumulative_viewshed.cpp
Outdated
| } | ||
|
|
||
| GDALDriver *memDriver = GetGDALDriverManager()->GetDriverByName("MEM"); | ||
| DatasetPtr dstDs(memDriver->Create( |
There was a problem hiding this comment.
dstDs should be checked against nullptr (memory allocation failure)
There was a problem hiding this comment.
So cumulative viewshed requires a temporary in-memory raster of the size of the source raster ? That should perhaps be indicated in the documentation as a limitation.
Put source raster dataset creation in the right place.
|
I am not a license specialist, but if you have created new .cpp files which add new features maybe you could mention yourself as author. I am curious, but by the added documentation I do not really understand what the result image will express. I found an example of hill tops https://intarch.ac.uk/journal/issue23/4/3.4.html, but what is the use case for a regular grid? |
|
@abellgithub #10676 might be of interest (this should not block this PR from progressing, but might be an opportunity to rework it once the RFC has been merged) |
|
I don't see currently any tests for the new mode? |
|
@abellgithub ready to be merged? |
|
If you're OK with things, yes. |
This adds support for cumulative viewsheds.
I've changed the C++ interface from earlier a little bit, but it's trivial (some support structures were moved out of the Viewshed class and placed in a namespace and all classes were put in that same namespace). I don't think it will change again.
There is a doxygen error but I think that's because I moved the code into a subdirectory. Maybe doxygen is confused because the documentation exists.