davidcsterratt / retistruct

Computational reconstruction and transformation of flattened retinae
http://davidcsterratt.github.io/retistruct/
7 stars 7 forks source link

Outlines should be checked for crossovers #6

Open davidcsterratt opened 8 years ago

davidcsterratt commented 8 years ago

If an outline contains crossovers, these should be checked for and an error or warning returned.

briancohn commented 7 years ago

outline_is_complex(outline) == is_complex(outline)

looking for intersection of any two line:

image

I wrote a line intersection function for a class project that achieves this: https://github.com/briancohn/robo-movement-feasibility/blob/iss2/main.py#L40

Feel free to adapt to R! you'd just have to do this: Given:

list_of_outline_coordinate_pairs = list((123,154),(154,275), ... )

Apply line intersection check:

lapply(list_of_outline_coordinate_pairs, function(x) {
does_line_cross_with_any_other_point_excluding_self_and_neighbors(x, list_of_coordinate_pairs)
})
briancohn commented 7 years ago

that said, this implementation would be ~O(n^2), where n is number of outline points but it probably would still run very quickly, and you could parallelize easily with parallel::mclapply

davidcsterratt commented 7 years ago

Thanks Brian. I'd forgotten a bit about what the point of this issue was and this has prompted me to remember.

I think the algorithm you use is found in line.line.intersection() in geometry.R, which is used in remove.intersections(). This function is actually used in idt.connect.segments() for the idt format (but not the csv or ijroi formats). In this case the problem of intersections between crossing lines is fixed by adding new points, rather than been reported as an error or a warning.

I think the issue doesn't tend to arise in the ijroi format, as the ROI function in ImageJ doesn't (I think) allow crossovers anyway. In the CSV format things are less constrained.

I think we should probably implement the check, which would throw an error, for all formats, probably in Outline() (though I would retain the auto-fix in idt.connect.segments() for backwards compatibility). However, I think this is a job for the next iteration of Retistruct - even a change like this can throw up more questions.