search

imglib / imglib2-roi

Regions of interest (ROIs) and labelings for ImgLib2
Other
9 stars 8 forks source link

Improve RealPointCollection Design #36

Open awalter17 opened 6 years ago

awalter17 commented 6 years ago

Hi!

This issue outlines several possible improvements to the RealPointCollection design, originally noted by @ctrueden.

  1. Preserve point order. DefaultWritableRealPointCollection does not preserve point order. Meaning, someone could create a RealPointCollection with a Collection of points and then retrieve an Iterable via points() with the points in a different order. Ideally, point order should be preserved.

  2. Avoid small objects. DefaultWritableRealPointCollection is backed by a HashMap which hashes on Trove double lists, meaning there are many small objects. The purpose of this was to ensure that the hash is on point location, not object reference. It would be nice to hash on something else, possibly something like XOR of the point coordinates? XOR can't be used because point pairs like (1, 3) and (3, 1) would have the same hash.

  3. Naive implementation of RealPointCollection. All the current implementations of RealPointCollection are backed by a secondary data structure, which makes test(...) very efficient. However, it could be useful to have a "lighter-weight" RealPointCollection that isn't backed by an additional data structure for times when test(...) performance isn't critical (i.e. SciView).

  4. Generalize DefaultWritableRealPointCollection constructor. DefaultWritableRealPointCollection#DefaultWritableRealPointCollection(java.util.Collection) takes a Collection<L> as input which is limiting. Instead this constructor should be generalized to take Collection<? extends L>.

  5. Improve performance of addPoint(...) bounds update. When calling addPoint(...) on DefaultWritableRealPointCollection, the bounds are updated by looping all points in the collection. However, this is very inefficient. The bounds can be updated by simply checking if the new point is smaller/larger than the current min/max.

Please let me know your thoughts on these suggested improvements. Also note, there have been some suggested improvements to RealPointCollection in #34 as well.

ctrueden commented 6 years ago

Thanks @awalter17. Note that (5) is solved by PR #35.