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

Accept a PIL Image as input to draw_image() #590

Merged
merged 3 commits into from
Feb 19, 2021

Conversation

jwiggins
Copy link
Member

This adds Image from the Python Imaging Library as an acceptable argument for draw_image on all relevant kiva backends. Further, it uses Image internally to handle conversion of color modes to something which is compatible with the color mode of the graphics context.

Fixes #574

@jwiggins jwiggins force-pushed the feature/pil-image-interchange branch from d5c00cc to eab208f Compare February 16, 2021 08:15
@jwiggins
Copy link
Member Author

jwiggins commented Feb 16, 2021

Damn. Looks like PIL.ImageQt doesn't work the same on Pyside2... Saved by the new pillow 6.2.2 eggs on EDM!

@corranwebster
Copy link
Contributor

corranwebster commented Feb 16, 2021

One thing to be careful about - I think Enable uses the agg backend for rendering backbuffers no matter what the Kiva library being used is and then (I suspect) uses draw_image to do the blitting. Removing the ability of the draw_image to accept agg images may break this.

This is the relevant code, I think:

enable/enable/component.py

Lines 766 to 805 in 8bc13b1

if not self.draw_valid:
# get a reference to the GraphicsContext class from the object
GraphicsContext = gc.__class__
if hasattr(GraphicsContext, "create_from_gc"):
# For some backends, such as the mac, a much more efficient
# backbuffer can be created from the window gc.
bb = GraphicsContext.create_from_gc(
gc, (int(width), int(height))
)
else:
bb = GraphicsContext((int(width), int(height)))
# if not fill_padding, then we have to fill the backbuffer
# with the window color. This is the only way I've found that
# it works- perhaps if we had better blend support we could set
# the alpha to 0, but for now doing so causes the backbuffer's
# background to be white
if not self.fill_padding:
with bb:
bb.set_antialias(False)
bb.set_fill_color(self.window.bgcolor_)
bb.draw_rect((x, y, width, height), FILL)
# Fixme: should there be a +1 here?
bb.translate_ctm(-x + 0.5, -y + 0.5)
# There are a couple of strategies we could use here, but we
# have to do something about view_bounds. This is because
# if we only partially render the object into the backbuffer,
# we will have problems if we then render with different view
# bounds.
for layer in self.draw_order:
if layer != "overlay":
self._dispatch_draw(layer, bb, view_bounds, mode)
self._backbuffer = bb
self.draw_valid = True
# Blit the backbuffer and then draw the overlay on top
gc.draw_image(self._backbuffer, (x, y, width, height))

Edit: we might be OK - it looks like it tries to create a backbuffer of the same GC type.

@jwiggins
Copy link
Member Author

Removing the ability of the draw_image to accept agg images may break this.

The hasattr(img, "bmp_array"): checks are basically equivalent to checking isinstance(img, agg.GraphicsContextArray). Additionally, most backends can accept an instance of themselves as the image argument.

kiva/cairo.py Show resolved Hide resolved
kiva/cairo.py Show resolved Hide resolved
@@ -54,8 +54,7 @@ def _image_default(self):
if components == 4:
img[:, :, 3] = np.linspace(0, 255, num=512*512).reshape(512, 512)

pilimg = Image.fromarray(img).convert(mode)
return np.array(pilimg)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in #589

kiva/pdf.py Show resolved Hide resolved
Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes make sense to me and IIUC, the code is implicitly tested. The code definitely looks cleaner - and mostly the same between the various kiva backends.

Not sure if adding unittests make the code easier to understand.

Comment on lines -562 to -577
def copy_padded(array):
""" Pad image width to a multiple of 4 pixels, and minimum dims of
12x12. QImage is very particular about its data.
"""
y, x, d = array.shape
pad = (lambda v: (4 - (v % 4)) % 4)
nx = max(x + pad(x), 12)
ny = max(y, 12)
if x == nx and y == ny:
return array
ret = np.zeros((ny, nx, d), dtype=np.uint8)
ret[:y, :x] = array[:]
return ret
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, we don't need to copy and pad the array data - because the use of PIL.Image.Image means that this is implicitly handled. Correct?

Copy link
Member Author

@jwiggins jwiggins Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do, but PIL handles it for us now via PIL.ImageQt.ImageQt

kiva/tests/test_qpainter_drawing.py Outdated Show resolved Hide resolved
@jwiggins
Copy link
Member Author

@corranwebster Do you have any more feedback or questions?

@corranwebster
Copy link
Contributor

@corranwebster Do you have any more feedback or questions?

I think I'm good with this.

@jwiggins jwiggins merged commit d0b20c9 into master Feb 19, 2021
@jwiggins
Copy link
Member Author

Thanks for the feedback

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

Successfully merging this pull request may close these issues.

Fix QPainter backend image drawing
3 participants