Fix horizon angle calculation for distant terrain #75
Conversation
|
I thought this algorithm was missing something, good to fix this. Can you also take a look at #64? |
|
Yep will take a look. I need it anyway |
|
Why would NaN block the horizon? That is assuming it is Inf (instead of -Inf before). Could we make it configurable? Also, seeing my old reply in previous PR #59 (comment), I would love to re-use these kernels to calculate a viewshed, which is 99% the same, it's just changing the output to a bool. Any thoughts on such a refactor? I rather not duplicate kernels, but not sure if adding an if/else for a different output (type) is wise here. |
| max_tan = max(max_tan, tan_angle) | ||
| tan_angle = (prev_elev - elev_pos) / (k * dist) | ||
| if tan_angle > max_tan | ||
| max_tan = tan_angle |
There was a problem hiding this comment.
Adding out[r, c] = True here would give you a viewshed.
|
Re NaN: I wondered that too. What is the height of NaN. You could assume zero is more often accurate? (the sea often missing). We can make it a keyword. We can totally share the kernel, just type it with a struct and the if/else will compile away. |
2 participants
I messed up the algorithm a bit.
The sweep used (prev_elev - elev) / dist against only the immediate predecessor cell, with max_tan accumulated across the entire ray. So:
The fix walks every visible cell back along the ray at each position and divides by the actual distance (k * dist). NaN cells reset the visible_start index so terrain past a NaN is hidden. Allocation-free; O(N²) per ray.
Also adds a small check to block multiple of 8 e.g. 24 angles slipping through that could erroneously work previously.