Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature idea: how about a get_np_pixels() method? #329

Closed
hx2A opened this issue Jul 27, 2023 · 7 comments
Closed

feature idea: how about a get_np_pixels() method? #329

hx2A opened this issue Jul 27, 2023 · 7 comments

Comments

@hx2A
Copy link
Collaborator

hx2A commented Jul 27, 2023

We have get_pixels() that can get us a single pixel or a PImage (should it be a Py5Image?) area of pixels. Could we get a np_array quickly without having to do the load_np_pixels() thing before using .np_pixels[] etc.? Would it make sense?

I suppose there could be a performance penalty for calling load_np_pixels every time...

Originally posted by @villares in #328

@villares
Copy link
Contributor

villares commented Jul 28, 2023

Would it be overkill to have a default keyword argument like copy=True that could be called as copy=False to get the np_array view?

I suppose the advanced users would load and access the np_pixels array for that, so maybe this would be useless complexity.

hx2A added a commit to hx2A/py5generator_fork that referenced this issue Jul 30, 2023
@hx2A
Copy link
Collaborator Author

hx2A commented Jul 30, 2023

Here's where this is now:

    def get_np_pixels(self, x: int, y: int, w: int, h: int, *, bands: str = 'ARGB', dst: npt.NDArray[np.uint8] = None) -> npt.NDArray[np.uint8]:
        """$class_Sketch_get_np_pixels"""
        x_slice = slice(x, x + w)
        y_slice = slice(y, y + h)

        self.load_np_pixels()
        if bands == 'L':
            pixels = self._np_pixels[x_slice, y_slice]
            pixels = ((pixels[:, :, 0:1] / 255.0) * pixels[:, :, 1:] @ [0.299, 0.587, 0.114]).astype(np.uint8)
        elif bands == 'ARGB':
            pixels = self._np_pixels[x_slice, y_slice]
        elif bands == 'RGB':
            pixels = self._np_pixels[x_slice, y_slice, 1:]
        elif bands == 'RGBA':
            pixels = np.roll(self._np_pixels[x_slice, y_slice], 1, axis=2)
        elif bands == 'BGR':
            pixels = self._np_pixels[x_slice, y_slice, 3:0:-1]
        elif bands == 'BGRA':
            pixels = np.dstack([self._np_pixels[x_slice, y_slice, 3:0:-1], self._np_pixels[x_slice, y_slice, 0]])
        else:
            raise RuntimeError(f"Unknown `bands` value '{bands}'. Supported values are 'L', 'ARGB', 'RGB', 'RGBA', 'BGR', and 'BGRA'.")

        if dst is not None:
            if dst.shape != pixels.shape:
                raise ValueError(f"Destination array has shape {dst.shape} but expected {pixels.shape}")
            else:
                dst[:] = pixels
                return dst
        else:
            return pixels if pixels.base is None else pixels.copy()

This seems to work but needs testing.

The bands parameter was AI's idea and an interesting one, even though its implementation of the idea was half wrong. It's more useful than a copy parameter. I will leave out a copy parameter because bands values of BGRA, RGBA, and L would always yield a copy no matter what, and it would be confusing to have to explain why copy is ignored for those and not the others. The pixels.base is None bit at the end is to check if the numpy array already is a copy and will call copy() if it is not. Bottom line, the returned array is never a view into another array.

Should the x, y, w, h parameters be optional? This would make it analogous to get_pixels(), and would return all of the pixels using the user specified bands. What about x, y but no w, h? Return one pixel, but as an array broken out by channels?

I suppose there could be a performance penalty for calling load_np_pixels every time...

Processing keeps track of if the pixels array is "dirty" or not so its loadPixels() method only does work if it needs to. I would expect multiple calls to this get_np_pixels() method to be reasonably efficient.

@villares
Copy link
Contributor

villares commented Jul 30, 2023

Yeah, if it is not too much trouble I think making x, y, w, h optional like get_pixels() would be nice. I'm not sure about the usefulness of the single pixel array...

And the reasoning about always providing a copy seems solid.

@hx2A
Copy link
Collaborator Author

hx2A commented Jul 31, 2023

Yeah, if it is not too much trouble I think making x, y, w, h optional like get_pixels() would be nice.

I think it's necessary. I'm also going to add a to_pil() method to convert to a PIL Image object.

@hx2A
Copy link
Collaborator Author

hx2A commented Aug 3, 2023

I'm mostly done with the coding and testing of the new get_np_pixels() and to_pil() methods. Next I need to write the documentation for it.

@hx2A
Copy link
Collaborator Author

hx2A commented Aug 6, 2023

And now just the example code. Almost there!

@hx2A
Copy link
Collaborator Author

hx2A commented Aug 7, 2023

This is complete. @villares , you can see the reference docs for the 2 new methods on the dev website. I also added the new methods to the summary page.

@hx2A hx2A closed this as completed Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants