1j01 / jspaint

🎨 Classic MS Paint, REVIVED + ✨Extras
https://jspaint.app/about
MIT License
7.25k stars 568 forks source link

On every tool action image of tool is re-requested #189

Closed Zekfad closed 3 years ago

Zekfad commented 4 years ago

Right now on every tool action image of that tool is re-requested, causing additional unnecessary traffic usage, here's stacktrace: 187: https://github.com/1j01/jspaint/blob/0db633b83fc7eba48d0c2b9213a92c513740cfbb/src/helpers.js#L185-L189 192: https://github.com/1j01/jspaint/blob/0db633b83fc7eba48d0c2b9213a92c513740cfbb/src/helpers.js#L191-L193 110: https://github.com/1j01/jspaint/blob/0db633b83fc7eba48d0c2b9213a92c513740cfbb/src/tools.js#L108-L112

Zekfad commented 4 years ago

Here's screenshot:

Spoiler ![image](https://user-images.githubusercontent.com/8970959/87828551-d6e8ad80-c885-11ea-9dac-c5369e1a1492.png)
Zekfad commented 4 years ago

Simplest solution will be to cache Image object, e.g.

const icons_cache = new Array();// cache
// ...
function get_help_folder_icon(file_name) {
    let cache_entry;
    if (cache_entry = icons_cache.filter(entry => entry.file_name === file_name)[0]) {
        return cache_entry.icon_img;
    }
    const icon_img = new Image(); 
    icon_img.src = `help/${file_name}`; 
    cache_entry = {
        icon_img,
        file_name,
    };
    icons_cache.push(cache_entry);
    return icon_img; 
}
// ...
1j01 commented 3 years ago

Oops, this doesn't actually work as a solution. They need to be separate Image objects so they can be appended to the DOM separately in the History window! I think this should really be a spritesheet. That said, I could only reproduce this with "Disable cache" checked in the Chrome Devtools, which I see you also have enabled. So I'm guessing this is not a big issue and is only related to that setting, and it probably doesn't happen when the devtools are closed.

1j01 commented 3 years ago

If it's just because of a devtools setting, this isn't really a bug.