Simplify sprite image loading #3514
Conversation
| const canvas = window.document.createElement('canvas'); | ||
| const context = canvas.getContext('2d'); | ||
| canvas.width = img.width; | ||
| canvas.height = img.height; | ||
| context.drawImage(img, 0, 0); | ||
| return context.getImageData(0, 0, img.width, img.height).data; | ||
| img.data = context.getImageData(0, 0, img.width, img.height).data; |
There was a problem hiding this comment.
img is an instance of HTMLImageElement which does not have a data property. It might be better to return a vanilla object with width, height, data, and complete properties.
There was a problem hiding this comment.
Not does it have getData() method it was augmented with earlier. There are some places in the code that expect getImage to return the image object rather than underlying data, such as ImageSource. On the other hand, moving canvas-based getData code to ImageSprite will make stubbing in Node more difficult because you have to stub its methods rather than ajax.getImage. So I suggest leaving this as is for now.
There was a problem hiding this comment.
On a second thought, we can make this much cleaner by introducing a separate browser.getImageData utility function and stubbing it out.
| }; | ||
| if (!sameOrigin(url)) { | ||
| img.crossOrigin = "Anonymous"; |
There was a problem hiding this comment.
Are you sure Anonymous should be capitalized? https://developer.mozilla.org/en-US/docs/Web/HTML/CORS_settings_attributes
There was a problem hiding this comment.
On the other hand, this example https://developer.mozilla.org/ru/docs/Web/HTML/CORS_enabled_image uses capitalized version. Video code below (unchanged) used capitalized version too.
|
@lucaswoj added another commit that makes handling/stubbing of image data much cleaner. |
mourner
force-pushed
the
cleanup-sprite
branch
from
9bac505
to
2b50f16
Compare
👀 @lucaswoj