cmu-cs-academy / desktop-cmu-graphics

BSD 3-Clause "New" or "Revised" License
15 stars 9 forks source link

Passing a PIL image to drawImage in Desktop CMU Graphics causes a confusing error #67

Open austin-schick opened 3 months ago

austin-schick commented 3 months ago

This is the error:

An error occurred. Here is the stack trace:
  "/Users/austin-work/cs-academy/desktop-cmu-graphics/foo.py", line 7:
        drawImage(i, 200, 200)
Exception: TypeError: CMUImage.url should be string-or-CMUImage (but <PIL.PngImagePlugin.PngImageFile image mode=P size=262x124 at 0x107808AF0> is of type PngImageFile)

The reference to .url in this error is confusing, as is the reference to PngImageFile. Improving this error would be good. Ideally we would even suggest a particular fix here, like "Did you forget to wrap a PIL image in a CMUImage"?

schmave commented 2 months ago

Notes from further conversation with Austin.

The reason we don't let you pass a PIL Image to redrawAll is that it will be slow. Creating a CMUImage wrapper around a PIL image will not help if you create the CMUImage in redrawAll. When you pass a string path to drawImage, we create a CMUImage wrapper and cache it that so that it is used the next time you draw. To fully fix this issue, we should add similar caching for PIL images passed directly to drawImage (including some check to see if the PIL image has been modified since the last draw) and then disallow creating CMUImages in redrawAll. Or something like that.