blueskythlikesclouds / MikuMikuLibrary

Format library and file editor for Hatsune Miku: Project DIVA games
MIT License
197 stars 45 forks source link

Resources class properties not caching objects #8

Closed samyuu closed 5 years ago

samyuu commented 5 years ago

A bitmap's GetHashCode method calculates the hash code of the bitmap object and not of the underlying bitmap image data resulting in different hash codes being calculated for the same bitmap every time this constructor is called. https://github.com/blueskythlikesclouds/MikuMikuLibrary/blob/63d95d1cb5ef07e046727ecdf40137dab1d7372a/MikuMikuModel/DataNodes/Wrappers/DataTreeNode.cs#L79-L81

This is causing ever increasing up to minute long loading times for node collections larger than a few hundred. Mostly affected by this are FarcArchives with a large amount of files such as mikitm.farc or shader.farc which would otherwise load in a matter of seconds.

I propose perhaps adding all image resources to the DataTreeNode.ImageList up front and referencing them by using the TreeView.ImageIndex property instead.

blueskythlikesclouds commented 5 years ago

I was aware of the issue but didn't know it affected it this much, wow. I'll think something for it. Your idea seems doable.

samyuu commented 5 years ago

After some closer reexamination it turns out the real problem lies within the Properties.Resources class. All of the static resources create both a new ResourceManager as well as a new resource object ever single time their getters are called which for the icons alone is twice per node instantiation. To make things even worse the way these resources are used throughout the code, the lack of { get; } auto properties, only amplifies this issue with the current resource model.

Simply adding backing fields for all static resource properties will resolve this issue and overall greatly improve performance and memory usage. The DataTreeNode constructor will then also correctly cache ImageKeys for the same object instances.

blueskythlikesclouds commented 5 years ago

Wow Microsoft... GG. It's better to make icons external at this point.

Could make a folder like AppData and put all the icons there, then make a custom class to handle loading and caching of them.

Or... like you said, initialize them beforehand statically and make the Icon properties return them.

blueskythlikesclouds commented 5 years ago

Fixed cdc6387