msohn

Thanks for sharing Anders. I regularly run into tickets by you on the DX support site and I'm often impressed how well written and researched these are. I bet the support guys at DX very much remember your name by now 😉 FWIW I "starred" a few tickets which I'd like to see fixed as well - one can only hope. Yes that is exactly my experience as well. Sometimes one feels like your hands are being tied, but then you just need to remember how much more tedious it was without the LayoutControl. Yes that's also exactly what I did - specifically TheBezier skin with different palettes and an unskinned mode where main forms are lfOffice11 (yeah I know, looks a bit old) and all dialogs use NativeStyle - this was the original look and was kept for people not wanting to adapt the looks.

We already used ExpressBars in most of the places. And just like TListView and TTreeView (regardless if original or cx version) don't support SVGs and skinning properly, so is the skinning and SVG support for TMainMenu et al very limited. With regard to TPopupMenu, if you need to use a control which cannot handle TcxPopupMenu, you can just handle popping up the menu manually via an OnContextPopup even handler (not sure if this is the right name). But I freely admit, this is only giving good results if you go all-in on the DX controls and it's a lot of work. But for us this was just a logical conclusion and it made us clean up very old parts of the GUI. The end result using TdxLayoutControl etc. is much more flexible than it ever was before.

I spent a major part of the last 4 years making our application here HighDPI compliant. And I ended up using DevExpress TcxImageList exclusively with SVGs, partially custom made. SVGs not only solve the DPI problem, they also feature a skin-adaptable palette, so that if your app has e.g. a light and a dark skin, the SVGs actually adapt their color (which they have to, to be identifiable in dark mode). With this approach a centralised ImageList is absolutely no problem - in fact you can request a specific image from the list in any desirable size/scale e.g. for use in custom drawings etc. Would you be willing to share more info on what these are, e.g. links to DX support issues? Littering code with workarounds sounds awful. I remember that back in maybe 2020 there were still a few issues, especially with the adaptable SVG palette, but with the 2022 version and RX 11.2 I'm currently using I'm actually very satisfied. I had to stop using TcxTreeView and TcxListView altogether, they are just shells around the native controls and as such do not support proper skinning and scaling as well as the SVG palettes. DX also made it quite clear that they don't intend to put more work into these controls, but rather in the alternatives TcxTreeList and TdxListViewControl. And since TcxTreeList is also capable to replace TVirtualTreeView I went this route.

There's an excellent blog post by Vincent Parret: https://www.finalbuilder.com/resources/blogs/code-signing-with-usb-tokens

You only need to make sure you compile a universal binary. Packing and deployment is unaffected. Ensure the ARM64 platform is enabled and the compiler option set as described here: https://docwiki.embarcadero.com/RADStudio/Athens/en/Delphi_Considerations_for_Multi-Device_Applications#Universal_Binaries

Yes there is. Have a look at TSelectionEditor and its RequiresUnits method: https://docwiki.embarcadero.com/Libraries/Alexandria/en/DesignEditors.TSelectionEditor Many 3rd party products do this (e.g. DevExpress) especially when components have event handlers containing types that are defined in another unit. Because adding such an event handler will otherwise break compiling and the user going on a hunt for the unit.

You're missing TObject.InitInstance which ensures all fields are initialised before the constructor is called. Edit: link for convenience https://docwiki.embarcadero.com/Libraries/Athens/en/System.TObject.InitInstance

I'm glad I took the time to explain how I read the VTune flame graph and that you tried and verified most of what I expected (well guessed tbh). And that you now saw first-hand why the original code was written the way it was. Welcome to the world of maintaining legacy projects😉 I'm sure you figured out how to get a copy of VTune, if not refer to the thread by Anders which I linked to in the flame graph post. It contained all I needed to get going. I read your post like you reverted your changes for now, but you might want to consider a few things which could improve the solution you tried: 1. Make sure all the loaded info is kept, i.e. when opening the dialog a second time, everything is already there. That way the user has to endure the delay only once. If that proves that memory usage is of no real concern (which I'd expect), you could go ahead and 2. Consider loading the info asynchronously, i.e. in a thread/threadpool as soon as the app starts - then, if the users opens the dialog before the loading has finished, show a progress and wait for it to finish. This should behave great when it works, but making that codebase thread safe is an enormous task, especially since there don't seem to be unit- and functional tests which help you make changes with confidence

Absolutely correct. To see where the time is spent, look at the VTune screenshot I posted here a few day ago: You have to read it from the bottom up. The x axis is time spent and it's only showing the time spent in ...BaseMenuScreen.DoLevelSelect, i.e. clicking the button to show the dialog. There's basically 3 main "pillars". The right-most shows all the way from TCustomForm.Show, via your TFLevelSelect.FormShow event handler calling SetInfo. The LoadNodeLabels part is what I was referring to in my last post where only visible, uninitialised nodes are updated. That is something that I would recommend joining with .InitializeTreeView - but do it for all nodes - and show a progress while you do it. The caching happens in LemLevelNeoPack, for example in TNeoLevelEntry.GetTitle. All these property getters call LoadLevelFileData, which checks LoadState telling it what was already loaded and then only load the missing parts or do nothing. Finally, to be clear: someone (you?) already put quite some effort into optimising the LevelSelect dialog, i.e. to only spend as little time as possible with filling the GUI initially (think InitializeTreeView) and then load all other parts upon user-interaction (SetInfo calls). It just turned out to be too slow (maybe too many levels/groups/packs now) and a mess (calling SetInfo at too many places). Now the task is to undo all of that, do the slower loading of everything while showing a progress and giving your applications window message pump more time to "breathe" and fix the freezing. The result hopefully is a better, more responsive compromise: opening the dialog the first time will take a lot longer than now - the progress will move slowly - but subsequently, a lot quicker.

It already does this. Again, I don't have the code at hand right now, but I remember something like a FLoadLevel which signalled what kind of data had already been loaded so you could call the method again, and if it already loaded the info, skipped the loading, i.e. a cache. So my idea would be to fully update the tree including all the loading in InitializeTreeView, i.e. you would merge SetInfo (edit: only the part that updates the tree) and InitializeTreeView and show a progress while you do that. That would also allow to to get rid of the OnExpanded node handler you added earlier in the thread here. Because of the above cache, it wouldn't actually reload it. Again, I might be wrong, if the user-data does not have the aforementioned cache. Also because e.g. the tree nodes are only updated once - for all future calls to SetInfo a node update is skipped if it's text is non-empty. That way all the initially blank nodes created in InitializeTreeView are updated only once, including image index etc. If any of the user data influences the node text or its image, then it wouldn't be properly updated. Again - might be wrong, can't check. Fair enough - this was merely directed at considering 3rd party tree views. Also because I'm still optimistic it isn't needed.

From my limited experience with the code, I would say this is definitely the approach to take. Bear in mind though that some information might actually get changed in the dialog (it does, right?) - I had the impression that there were already optimisations in place that would prevent this from working, but I did not get to the bottom of this, so I might be wrong. I would do this first - and only if the result is still not acceptable, would I consider other options like 3rd party trees. But I'm quite positive that won't be necessary with preloaded info. Also: since your end goal is a totally different UI without a tree, why bother? Actually - that UI works so different that maybe you wouldn't even need to cache info at all since there is a lot less info which is displayed once at a time.

I was looking for an excuse to use VTune again 😉 (thanks to Anders map2pdb!) and here's a flame graph of what's going on while the form is shown: (this is without any sounds being played in the background since I don't have BASS dll lying around - and I'm glad I don't) So the Sleep in ApplicationIdle wastes about one third of the total time. And while FormCreate does spend quite some time in InitializeTreeView, half of it is now because the OnExpanded handler is called during it (that's what @Lars Fosdal was hinting at). But in the end most of the time is spent reading and parsing the level info that is spread over hundreds of text files.

Not with the built-in tools, but here's an excellent summary by Remy of what you need to do: https://stackoverflow.com/a/1130506/386473 I just used exactly that approach and was able to find and fix the memory leaks in like half an hour. I'll try to make a pull request on GitHub as the patch is a bit larger due to spacing when I introduced new try/finally blocks.

Yes I see that now - didn't properly test my changes. Have a look at the attached patch - before exiting, it makes the talisman box now visible. Fixes the issue that you described. SuperLemmixPlayer-21-25-34.patch

Are you using a GUI git client? Mine offers to apply patches (I highly recommend https://fork.dev). But after all it's a text file, it's not that difficult to read and understand it, especially since the changes are small and I described what I changed. And then there's also the command line, see https://git-scm.com/docs/git-apply