Skip to content

Commit

Permalink
Remove functions.try_fastpath_argb due to segfault
Browse files Browse the repository at this point in the history
In some rare occasions in CI, try_fastpath_argb would trigger a segfault

Since this function pre-dates functions_qimage.try_make_qimage, we can
retire this function with minimal impact to end users.

Furthermore, uncomment out the tests that were commented out in #3007
  • Loading branch information
j9ac9k committed Apr 28, 2024
1 parent 00bae71 commit d01055e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 64 deletions.
66 changes: 8 additions & 58 deletions pyqtgraph/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
'solve3DTransform', 'solveBilinearTransform',
'clip_scalar', 'clip_array', 'rescaleData', 'applyLookupTable',
'makeRGBA', 'makeARGB',
# 'try_fastpath_argb', 'ndarray_to_qimage',
# 'ndarray_to_qimage',
'makeQImage',
# 'ndarray_from_qimage',
'imageToArray', 'colorToAlpha',
Expand Down Expand Up @@ -1257,7 +1257,7 @@ def rescaleData(data, scale, offset, dtype=None, clip=None):
# Cupy does not support nditer
# https://github.com/cupy/cupy/issues/5021

data_out = data.astype(work_dtype, copy=True)
data_out = data.astype(work_dtype)
data_out -= offset
data_out *= scale

Expand Down Expand Up @@ -1360,7 +1360,7 @@ def makeARGB(data, lut=None, levels=None, scale=None, useRGBA=False, maskNans=Tr

if lut is not None and not isinstance(lut, xp.ndarray):
lut = xp.array(lut)

if levels is None:
# automatically decide levels based on data dtype
if data.dtype.kind == 'u':
Expand All @@ -1374,7 +1374,7 @@ def makeARGB(data, lut=None, levels=None, scale=None, useRGBA=False, maskNans=Tr
raise Exception('levels argument is required for float input types')
if not isinstance(levels, xp.ndarray):
levels = xp.array(levels)
levels = levels.astype(xp.float64)
levels = levels.astype(xp.float64, copy=True)
if levels.ndim == 1:
if levels.shape[0] != 2:
raise Exception('levels argument must have length 2')
Expand Down Expand Up @@ -1407,7 +1407,7 @@ def makeARGB(data, lut=None, levels=None, scale=None, useRGBA=False, maskNans=Tr
nanMask = xp.isnan(data)
if data.ndim > 2:
nanMask = xp.any(nanMask, axis=-1)

# Apply levels if given
if levels is not None:
if isinstance(levels, xp.ndarray) and levels.ndim == 2:
Expand Down Expand Up @@ -1460,12 +1460,7 @@ def makeARGB(data, lut=None, levels=None, scale=None, useRGBA=False, maskNans=Tr
else:
dst_order = [1, 2, 3, 0] # A,R,G,B (ARGB32 big endian)

# copy data into image array
fastpath = try_fastpath_argb(xp, data, imgData, useRGBA)

if fastpath:
pass
elif data.ndim == 2:
if data.ndim == 2:
# This is tempting:
# imgData[..., :3] = data[..., xp.newaxis]
# ..but it turns out this is faster:
Expand All @@ -1477,16 +1472,15 @@ def makeARGB(data, lut=None, levels=None, scale=None, useRGBA=False, maskNans=Tr
else:
for i in range(0, data.shape[2]):
imgData[..., dst_order[i]] = data[..., i]

profile('reorder channels')

# add opaque alpha channel if needed
if data.ndim == 3 and data.shape[2] == 4:
alpha = True
else:
alpha = False
if not fastpath: # fastpath has already filled it in
imgData[..., dst_order[3]] = 255
imgData[..., dst_order[3]] = 255

# apply nan mask through alpha channel
if nanMask is not None:
Expand All @@ -1501,50 +1495,6 @@ def makeARGB(data, lut=None, levels=None, scale=None, useRGBA=False, maskNans=Tr
return imgData, alpha


def try_fastpath_argb(xp, ain, aout, useRGBA):
# we only optimize for certain cases
# return False if we did not handle it
can_handle = xp is np and ain.dtype == xp.ubyte and ain.flags['C_CONTIGUOUS']
if not can_handle:
return False

nrows, ncols = ain.shape[:2]
nchans = 1 if ain.ndim == 2 else ain.shape[2]

Format = QtGui.QImage.Format

if nchans == 1:
in_fmt = Format.Format_Grayscale8
elif nchans == 3:
in_fmt = Format.Format_RGB888
else:
in_fmt = Format.Format_RGBA8888

if useRGBA:
out_fmt = Format.Format_RGBA8888
else:
out_fmt = Format.Format_ARGB32

if in_fmt == out_fmt:
aout[:] = ain
return True

npixels_chunk = 512*1024
batch = int(npixels_chunk / ncols / nchans)
batch = max(1, batch)
row_beg = 0
while row_beg < nrows:
row_end = min(row_beg + batch, nrows)
ain_view = ain[row_beg:row_end, ...]
aout_view = aout[row_beg:row_end, ...]
qimg = QtGui.QImage(ain_view, ncols, ain_view.shape[0], ain.strides[0], in_fmt)
qimg = qimg.convertToFormat(out_fmt)
aout_view[:] = imageToArray(qimg, copy=False, transpose=False)
row_beg = row_end

return True


def ndarray_to_qimage(arr, fmt):
"""
Low level function to encapsulate QImage creation differences between bindings.
Expand Down
10 changes: 4 additions & 6 deletions tests/test_makeARGB.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,21 +165,19 @@ def test_makeARGB_with_nans():
im2, alpha = _makeARGB(im1, levels=(0, 1))
assert alpha
assert im2[3, 5, 3] == 0 # nan pixel is transparent

# assert im2[0, 0, 3] == 255 # doesn't affect other pixels
assert im2[0, 0, 3] == 255 # doesn't affect other pixels

# With masking nans disabled, the nan pixel shouldn't be transparent
# im2, alpha = _makeARGB(im1, levels=(0, 1), maskNans=False)
# assert im2[3, 5, 3] == 0 # nan pixel is transparent

im2, alpha = _makeARGB(im1, levels=(0, 1), maskNans=False)
assert im2[3, 5, 3] == 255 # nan pixel is transparent

# 3d RGB input image, any color channel of a pixel is nan
im1 = np.ones((10, 12, 3))
im1[3, 5, 1] = np.nan
im2, alpha = _makeARGB(im1, levels=(0, 1))
assert alpha
assert im2[3, 5, 3] == 0 # nan pixel is transparent
# assert im2[0, 0, 3] == 255 # doesn't affect other pixels
assert im2[0, 0, 3] == 255 # doesn't affect other pixels

# 3d RGBA input image, any color channel of a pixel is nan
im1 = np.ones((10, 12, 4))
Expand Down

0 comments on commit d01055e

Please sign in to comment.