-
Notifications
You must be signed in to change notification settings - Fork 60
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
MoonCats: use pre-generated images on IPFS #13
Conversation
}) | ||
}) | ||
ctx.imageSmoothingEnabled = false | ||
ctx.drawImage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're grabbing a PNG file from a remote URL, and then painting it onto a Canvas element? Why not simply use the remote URL itself as the URL to make an IMG tag of, rather than needing to re-paint every one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do! The issue is that the images don’t have any padding, which makes them look bad when displayed directly. And since I was already doing this when the image was generated in canvas, I decided to keep the extra padding so it looks good :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right; that could also be accomplished with CSS, rather than spending the processing time to bake it into the image itself, but it's probably not a huge CPU hit to do this sort of processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, it’s a small canvas that’s doesn’t get displayed on screen or inserted in the DOM tree, it shouldn’t have any noticeable cost. Otherwise CSS would certainly be an option yes in other contexts, but this library only returns a data structure, so the image has to be a URL.
Now I probably need to add my thinking as comments in the code itself, thanks for making me realize it 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized you were the person that generated the assets and hosted them on IPFS, so thank you again 🤗 .
I was wondering if adding some padding on the .png directly could be a possibility? Or if you think there could be an issue doing that? That would make the process much more straightforward for useNft() haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're welcome! The reason why I did not add padding to the PNGs that I created is that because they're intended for re-use, I didn't want to presume how much padding was "enough" for all use-cases. Having them with no padding means all websites that want to include them can pick exactly how much padding they need for their use-case. Having no padding also keeps the file size down (adding blank pixels adds a hair more to the file size per picture. It's not a lot, but in sites that may want to show whole galleries of lots of MoonCats, it could add up to larger network transfers to get them all). So, no, I'm not planning on adding in a hard-coded border/padding around the images at this time, to keep them as low file-size and as versatile as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completely understandable. Thanks for having taken the time to give me a detailled answer!
See https://www.reddit.com/r/MoonCatRescue/comments/m5d7mx/svg_imagery_of_all_rescued_mooncats/