davidegironi / advanceddatagridview

A .NET WinForms DataGridView with advanced capabilities
392 stars 123 forks source link

Port to .NET 5 #49

Closed billpeet closed 4 years ago

billpeet commented 4 years ago

The future of .NET is .NET 5 :) It appears the DG.AdvancedDataGridView NuGet package actually works on a .NET 5 project (even if NuGet complains), it looks like it's at least partly compatible. Any chance of officially targeting .NET 5 in a future release?

davidegironi commented 4 years ago

Hello @billpeet , unluckily I've no plan for .NET 5 or .NET Core, time to develop this open project is reduced. Also, up to now NET.5 is still RC, i think I will in the future skip .NET Core and go to .NET 5, when it will be stable. Of course hope I've time to perform this task.

kirsan31 commented 3 years ago

@davidegironi if your still interested, I made it here. And it's working at first look.

What done:

What was not checked:

davidegironi commented 3 years ago

That's sound interesting, let me check it in the weekend. Meanwhile if you can check the nuget and _DevTools let me know.

davidegironi commented 3 years ago

I'm working on the net5 version. I think I will make a new branch. NET5 works, but I want to write the compiling script for dll and nuget package. I'm almost done with both, I think next week I'll have time to work on this.

davidegironi commented 3 years ago

Hello, check this out: https://www.file.io/download/lRRkuJowXmFN Target net40 and net5.0-windows. Updated the AutoBuild script and the NuGet builder, now both works (find AutoBuild updated version here: http://davidegironi.blogspot.com/2014/04/autobuild-is-build-automation-tool-for.html) I've done my porting from nonSDK to SDK project, cause in your one there are a few things changed in the sln file also. Check it out and let me know. Once you have done I'll post this no nuget, then I'll add the other changes requested here: https://github.com/davidegironi/advanceddatagridview/issues/60

kirsan31 commented 3 years ago

Hi @davidegironi. Sorry I have very hard time at work for now. I will back as soon as I can.

davidegironi commented 3 years ago

As a side note, if you want to check also the menustrip mod, you can find it here: https://github.com/davidegironi/advanceddatagridview/issues/58#issuecomment-819727960

This update contains the rewrite of the checklist filter logic, it's quite important. So It's better to check it a lot before publishing.

kirsan31 commented 3 years ago

@davidegironi

Hello, check this out: https://www.file.io/download/lRRkuJowXmFN

Hi, I looked at the code, and use .net5 version for some time and all seems ok (in terms of porting to .net5).

As a side note, if you want to check also the menustrip mod, you can find it here: #58 (comment)

File is not available any more.

P.s. While testing, I found some not serious memory problems in demo app and in lib too. But fixes are trivial. More serious problem is memory leak in ColumnHeaderCell :( In OnColumnAdded we replace existing HeaderCell with our ColumnHeaderCell. So, in ColumnHeaderCell constructor we need to do a full clone of existing HeaderCell to dispose it after cloning. But this doesn't happens. Same in Clone method. With current implementation we simple reference elements of old HeaderCell and this lead to impossibility of GC it.

In constructor we need to clone base DataGridViewColumnHeaderCell and if HeaderCell is ColumnHeaderCell clone new elements too (including MenuStrip). This is not very trivial task and not possible without reflection (needed for cloning event handlers) :(

I'll be back as soon as I can (still troubles at work), hopefully in a few days.

davidegironi commented 3 years ago

Thank you @kirsan31 .

You can find the file here: https://file.re/2021/04/19/advanceddatagridview/

About the Memory Leak, I've built after this issue https://github.com/davidegironi/advanceddatagridview/issues/41 the "Memory Test" button. I've checked it with net40, no problem at all. The behaviour under net5.0 indeed is different. This is a bit wired. I thought https://github.com/davidegironi/advanceddatagridview/blob/f6fdac4e9e933dffa227e7cfdaeb2001ccb5e64b/AdvancedDataGridView/AdvancedDataGridView.cs#L994 should fix the problem but it's not like this. I have to investigate on this. If you find a solution please share it with me, using the last code I send you. Thanks!

kirsan31 commented 3 years ago

@davidegironi Sorry, I haven't checked this yet :( Still have very little free time. Demo app is leaking itself, net40 after 1 memory test: image I have fixed it here (with some small additional improvements). Also some small memory fixes.

About huge memory leak on .Net5. The main problem of it I described in previous post. Now why on net4 we have only small leak and on .Net5 huge. This workaround: https://github.com/davidegironi/advanceddatagridview/blob/f6fdac4e9e933dffa227e7cfdaeb2001ccb5e64b/AdvancedDataGridView/AdvancedDataGridView.cs#L190-L206 Not working on .Net5 because Columns already empty here. They are cleared here, with disposing of bindingSource_main.

We need properly dispose ColumnHeaderCell in OnColumnRemoved as your originally discuss. But the main problem prevent doing it. I will think of some workaround, because doing it right a bit challenging :) Will be back as soon as I can.

----UPD----

It seems to me that the memory leak has been fixed. Your can check it here.

You can find the file here: https://file.re/2021/04/19/advanceddatagridview/

This file is gone too :( Why don't you just push a new branch?

davidegironi commented 3 years ago

Hello @kirsan31 Thank you for your job, I wasn't that much interested in the sample project FormMain investingation, but as you pointed out it turns out it has big memory leak. In a few days I'll publish an update containing

It will be a major number update. Up to my test all works. Here is the version I'll publish, If you want to check it out: https://file.io/NI8xeXxT2RVg Next version will come out in a few weeks. I'll add the fix you suggested here: https://github.com/davidegironi/advanceddatagridview/issues/60