Avoid queue duplicates in morphological reconstruction backward scan

[hawahee]

It should not be necessary to add duplicate indices to the queue during the backward scan of the morphological reconstruction.

With the proposed change and the current static length of the queue (dims[0] * dims[1]) the queue should no longer be able to fill up during the backward scan. Therefore, the reconstruct_hybrid loop could also be changed to only repeat for the propagate step. Feel free to let me know if you want this additional change.

[wkearn]

@hawahee: Thanks for this change!

Breaking from the loop in the backward scan as you have proposed is correct. For my own reference, here is the relevant portion of the algorithm from Vincent (1993):

If there exists $q \in N^-_G(p)$ such that J(q) < J(p) and J(q) < I(q)
fifo_add(p)

i.e. we only need to add the pixel p to the queue once if there is a single backwards neighbor that satisfies the condition. Right now, we can add the pixel p up to 4 times.

It took some thinking, but I am also convinced that the queue cannot ever fill up during backward_scan. While the queue array has a length dims[0] * dims[1], it can only hold dims[0] * dims[1] - 1 elements. Nevertheless, the upper right corner pixel of the DEM cannot ever be added to the queue because it has no backwards neighbors. Even if we tried to add every valid pixel to the queue (which I'm not sure is possible), we would only ever add dims[0] * dims[1] - 1 pixels.

If you'd like, feel free to make the change that only checks if we need to repeat after propagate on this PR. You can also open a new PR if you would prefer.

[hawahee]

I have now added the additional changes.

Why is the repeat loop limited to a maximum of 2 scans? Although not common, I believe that many scans (e.g. 10+) can be needed for certain chaotic examples, because the queue will fill up during the propagate step. Is it not possible that skipping the pixel in the propagate step because the queue is full will result in a wrong result when limiting the number of scans?

[wkearn]

Why is the repeat loop limited to a maximum of 2 scans? Although not common, I believe that many scans (e.g. 10+) can be needed for certain chaotic examples, because the queue will fill up during the propagate step. Is it not possible that skipping the pixel in the propagate step because the queue is full will result in a wrong result when limiting the number of scans?

The answer to your second question is "possibly." My intuition is because the marker elevation of a pixel never decreases, if you fail to enqueue a pixel and one of its neighbors is lower than it, that update will still be able to be applied in the next iteration. But this could easily be wrong.

It would be helpful to have some more test cases where weird stuff happens in the reconstruction algorithm. Having applied your changes to backward_scan, none of our current tests ever fill up the queues, so we only ever run one scan.

Vincent's paper says that the breadth-first scan needs to visit pixels more than once when two regional maxima of the marker image with different elevations are next to each other. That might be a good place to start.

I'll merge your changes here so we have them generally and open an issue to continue this discussion.