Fix buffer overflow in Grouping3D, fix CoordinateFilter mask bug, reduce memory usage

[Cybis320]

• Fix buffer overflow in getAllPoints: propagateLine could write past the end of max_line_points when the bidirectional point search found more points than find3DLines' single-pass counter. Added bounds check. This was reported as heap corruption crashes (free(): invalid size, corrupted size vs. prev_size, munmap_chunk(): invalid pointer) on at least one RPi 5 station running 1080p CSI2 cameras.
• Fix int32 overflow in line3DDistance_simple and point3DDistance: intermediate cross-product terms overflow 32-bit int at higher resolutions. Changed to long long.
• Fix CoordinateFilter mask bug: (mask == 0) was comparing a MaskStructure object to 0, always evaluating to False. The mask has been silently ignored since this class was introduced. Now correctly accesses mask.img.
• Remove scipy.ndimage dependency from Image.py: CoordinateFilter was allocating ~50MB of temporary arrays per detection call via np.indices and distance_transform_cdt to do what simple scalar comparisons accomplish.
• Use explicit 32-bit dtypes in Image.py and ExtractStars.py where float64/int64 defaults were unnecessary.

[Copilot]

Pull request overview

This PR addresses stability and performance issues in RMS’s detection pipeline by fixing a reported heap corruption in 3D grouping, correcting a long-standing CoordinateFilter mask application bug, and reducing per-call memory usage by removing a heavy SciPy ndimage dependency.

Changes:

• Prevent out-of-bounds writes in getAllPoints and widen intermediate integer math in 3D distance calculations to avoid overflow at higher resolutions.
• Replace CoordinateFilter’s ndimage-based distance transform with lightweight border/mask checks and correctly apply MaskStructure.img.
• Reduce memory/precision overhead by using explicit 32-bit dtypes in a few hot paths.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
RMS/Routines/Image.py	Reworks CoordinateFilter (removes SciPy ndimage usage), switches some operations to float32, and reduces mask allocation dtype.
RMS/Routines/Grouping3Dcy.pyx	Fixes potential buffer overflow in point propagation and prevents int32 overflow in intermediate distance computations.
RMS/ExtractStars.py	Uses int32 indices for PSF fitting index grids to avoid unnecessary larger default dtypes.

Comments suppressed due to low confidence (1)

RMS/Routines/Image.py:104

• filterCoordinates converts coords to a NumPy array (coords_int) but then boolean-indexes the original coords (filtered_coords = coords[valid_flags]). If callers pass a Python list (or any non-ndarray sequence), this will raise because boolean indexing is not supported. Consider converting once up front (e.g., coords_arr = np.asarray(coords)) and using that for both calculations and filtered_coords.

        coords_int = np.array(coords).astype(np.int32)

        x_vals = coords_int[:, 0]
        y_vals = coords_int[:, 1]

        # Check border distance
        valid_flags = (
            (x_vals >= self.border) & (x_vals < self.w - self.border) &
            (y_vals >= self.border) & (y_vals < self.h - self.border)
        )

        # Check mask (only for coordinates that passed the border check)
        if self.mask_img is not None:
            for i in range(len(valid_flags)):
                if valid_flags[i]:
                    y, x = y_vals[i], x_vals[i]
                    if 0 <= y < self.mask_img.shape[0] and 0 <= x < self.mask_img.shape[1]:
                        if self.mask_img[y, x] == 0:
                            valid_flags[i] = False

        filtered_coords = coords[valid_flags]

        return filtered_coords, valid_flags

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

filterCoordinates will crash on empty input when coords is an empty list/1D array: coords_int = np.array(coords) becomes shape (0,), and coords_int[:, 0] raises an IndexError. Adding an early return for len(coords) == 0 (return empty coords + empty flags) would make this function robust.

Suggested change

	coords_int = np.array(coords).astype(np.int32)
	# Convert to a NumPy array and handle empty input safely
	coords_array = np.array(coords)
	if coords_array.size == 0:
	# No coordinates to filter: return empty coordinates and empty flags
	return coords_array, np.zeros(0, dtype=bool)
	coords_int = coords_array.astype(np.int32)

[Copilot]

The mask check loops in Python over every coordinate (for i in range(len(valid_flags))), which can become a hotspot when filtering many centroids. This can be vectorized by applying valid_flags to x_vals/y_vals and doing a single advanced-indexing lookup into mask_img (plus a vectorized in-bounds check if needed).

Suggested change

	for i in range(len(valid_flags)):
	if valid_flags[i]:
	y, x = y_vals[i], x_vals[i]
	if 0 <= y < self.mask_img.shape[0] and 0 <= x < self.mask_img.shape[1]:
	if self.mask_img[y, x] == 0:
	valid_flags[i] = False
	# Select coordinates that are currently valid
	xs_valid = x_vals[valid_flags]
	ys_valid = y_vals[valid_flags]
	# Vectorized in-bounds check for the mask image
	in_bounds = (
	(ys_valid >= 0) & (ys_valid < self.mask_img.shape[0]) &
	(xs_valid >= 0) & (xs_valid < self.mask_img.shape[1])
	)
	# Assume all currently valid coordinates remain valid w.r.t. mask
	mask_ok = np.ones_like(xs_valid, dtype=bool)
	# For those within the mask bounds, invalidate where mask pixel is zero
	if np.any(in_bounds):
	ys_in = ys_valid[in_bounds]
	xs_in = xs_valid[in_bounds]
	mask_vals = self.mask_img[ys_in, xs_in]
	mask_ok[in_bounds] = mask_vals != 0
	# Update the original valid_flags in a vectorized manner
	valid_flags[valid_flags] &= mask_ok

[Copilot]

thickLine initializes photom_mask twice; the second initialization overwrites the first and makes the first allocation unnecessary. Removing the redundant initialization avoids an extra full-frame allocation and makes the control flow clearer.

[Copilot]

The new i >= arr_size guard prevents out-of-bounds writes, but it also silently truncates the set of points returned by getAllPoints when max_array_size is undersized (notably when called from remove3DPoints with max_array_size=max_line.counter). That can leave some on-line points in point_list, changing subsequent line finding/removal behavior. Consider ensuring max_array_size is always large enough (e.g., fall back to point_list_size or grow/reallocate when full) so correctness doesn’t depend on the counter estimate.

Suggested change

	# Stop if the output array is full
	if i >= arr_size:
	break
	# Ensure there is enough space in the output array
	if i >= arr_size:
	# Grow the array (e.g. double its current size) to avoid truncating valid points
	if arr_size == 0:
	arr_size = 1
	else:
	arr_size = arr_size * 2
	new_max_line_points = np.empty((arr_size, max_line_points.shape[1]), dtype=UINT16_TYPE)
	new_max_line_points[:i, :] = max_line_points[:i, :]
	max_line_points = new_max_line_points