Viewshed: support observers to left and right of input raster #10264
Conversation
| return false; | ||
| } | ||
| if (!oOutExtent.contains(nX, nY)) | ||
| { | ||
| CPLError(CE_Warning, CPLE_AppDefined, |
There was a problem hiding this comment.
do we need to issue a warning if this is nominally supported ?
There was a problem hiding this comment.
I think so since it's a change in behavior from what exists. I don't think it hurts anything. It could be removed later.
| { | ||
| CPLError(CE_Warning, CPLE_AppDefined, | ||
| "NOTE: The observer location falls outside of the DEM area"); | ||
| //ABELL - Make sure observer Z is specified. |
There was a problem hiding this comment.
I'm not sure. Normally the Z is zero by default and perhaps this is still fine.
| /// Process the part of the first line to the left of the observer. | ||
| /// | ||
| /// @param nX X coordinate of the observer. | ||
| /// @param iStart X coordinate of the first cell to the left of the observer to be procssed. |
There was a problem hiding this comment.
| /// @param iStart X coordinate of the first cell to the left of the observer to be procssed. | |
| /// @param iStart X coordinate of the first cell to the left of the observer to be processed. |
autotest/pymod/test_cli_utilities.py
Outdated
| @@ -29,59 +29,14 @@ | |||
| ############################################################################### | |||
|
|
|||
| import os | |||
| import sys | |||
There was a problem hiding this comment.
it would be good if those changes to test_cli_utilities.py were put in a dedicated PR
There was a problem hiding this comment.
I didn't realize those were in there until after I pressed the button. Are you OK with them as they are?
There was a problem hiding this comment.
Actually, I'm afraid that's going to break Windows testing, because with MSVC builds, binaries go to $build_dir/apps/$config_name where config_name=Release, Debug, RelWithDebInfo, etc.
cmake/helpers/GdalSetRuntimeEnv.cmake tests care of correctly appending that directory in the PATH.
There was a problem hiding this comment.
Is there some reason not to drive all this from cmake and avoid all of this? cmake knows where it stuck the binaries.
There was a problem hiding this comment.
it is convenient to be able to run a single test file locally with pytest, without going through a whole test suite
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
|
For CI to trigger, you also need to rebase on top of latest master to fix a conflict (likely related to 082d0e7) |
|
@rouault : I think issues have been addressed here. |
you need to resolve #10264 (comment) |
This adds support for observers to the left and right of the raster. Observers above and below the raster are not supported and providing such a location for an observer generates an error. It assumes that the space outside of the raster is empty (at a height below all cells in the raster).
Tests are included.