Added image icons in content browser

[FacelessTiger]

Currently textures/images in the Content Browser panel display the default file icon, as a QOL feature I've changed it to use the image itself.

To avoid having to recreate the texture every frame I add it to a cache (unordered map of file path string to texture). An issue with this approach is that if the file is deleted it wouldn't be removed from the cache so I added the code below to remove the textures from cache if the file no longer exists.

for (auto it = m_ImageIcons.cbegin(), next_it = it; it != m_ImageIcons.cend(); it = next_it)
{
	++next_it;

	if (!std::filesystem::exists((*it).first))
		m_ImageIcons.erase(it);
}

Unfortunately the for loop has to be written in this way since a normal auto [key, val] for loop breaks when you try to remove a key while iterating it.

[VagueLobster]

Just so you know... that for loop with iterator will Never call m_ImageIcons.erase(it);
because you have typed: if (!std::filesystem::exists((*it).first)) instead of: if (std::filesystem::exists((*it).first)).
so basically you have unused code... and possibly a memory leak, cannot quite remember if Refs can get memory leaks though.
Basically the iterator loop is pointless, because it does nothing.

[FacelessTiger]

Just so you know... that for loop with iterator will Never call m_ImageIcons.erase(it); because you have typed: if (!std::filesystem::exists((*it).first)) instead of: if (std::filesystem::exists((*it).first)). so basically you have unused code... and possibly a memory leak, cannot quite remember if Refs can get memory leaks though. Basically the iterator loop is pointless, because it does nothing.

I might be misunderstanding what you're saying but, the point of the line is to make sure the path does NOT exist on disk so it can be removed from the list. I believe what your suggestion would do is just remove ALL items from the list every frame then reload, slowing down the program by a ton (shown via a print below)

whereas normally it only removes when something on disk is deleted

I can also verify it works by deleting the image on disk then renaming another file to be the deleted image.

When the code is commented out, this discrepancy happens

When it's not it works as expected

[VagueLobster]

Well, now i don't get what you're saying with the last 4 screenshots, sorry! (EDIT: I just read your message again, and now i understand what you mean by the images 🙂)
But maybe you're right, i dunno what your purpose of the original code was :) I just assumed that you wanted to delete every frame, because it runs very slow either way (i've tried it in my engine and had to make my own version that's inspired by your code)

[VagueLobster]

Btw, just a little FYI: you're not deleting the thumbnail icons for png's when going into another folder 😉

[FacelessTiger]

Well, now i don't get what you're saying with the last 4 screenshots, sorry! (EDIT: I just read your message again, and now i understand what you mean by the images 🙂) But maybe you're right, i dunno what your purpose of the original code was :) I just assumed that you wanted to delete every frame, because it runs very slow either way (i've tried it in my engine and had to make my own version that's inspired by your code)

Yeah, I probably could've explained it a bit better sorry about that haha. I'm not sure why it's slow for you, when I tried to reload the image from disk constantly its slow but all it should be running is checking if a couple of paths exist. Then again, I've only does this with about 2-3 images, I haven't tested performance with a high amount of images.

Btw, just a little FYI: you're not deleting the thumbnail icons for png's when going into another folder 😉

If you mean that it doesn't clear the cache every folder change (rather than it doesn't DETECT deletes/renames in non-open folders, because that should work with how its written) then that's true. My thought process though, is that it's only going to load around 20-30 images at once (most probably being in a single folder) so I'm not sure if it's worth to clear the cache every folder change.

But! It wouldn't be hard to write it to support that if Cherno sees it and requests so. I would be happy to commit the change.

[VagueLobster]

yeah let's see if/when cherno reacts to this, and if he chooses to write his own 😉

[FacelessTiger]

With the addition of the filewatcher, #595 is a better implementation of this system.