ComputerGhost / FaviconFetcher

Scan a webpage for favicons, or just easily download the one you want.
MIT License
5 stars 3 forks source link

IconImage dispose itself without dispose the underlying SKBitmap object #30

Open zhuxb711 opened 5 months ago

zhuxb711 commented 5 months ago

Just set _bitmap into null

https://github.com/ComputerGhost/FaviconFetcher/blob/4d02448fe04ef64a7a72f4a00340bda049ed0b96/FaviconFetcher/Classes/IconImage.cs#L72

If concerning dispose the _bitmap will affect ToSKBitmap(). I suggest to change ToSKBitmap() to:

public SKBitmap ToSKBitmap()
{
    return _bitmap.Copy();
}

or

// Developer should be responsible for disposing an 'Underlying' object which might cause some problems
public SKBitmap GetUnderlyingSKBitmap()
{
    return _bitmap;
}
kiddailey commented 4 months ago

Nice catch, thank you! Would it be simpler to just properly dispose the SKBitmap properly?

Returning a copy of the bitmap could increase memory usage if I'm understanding correctly.

zhuxb711 commented 4 months ago

Yes sure, just dispose the underlying _bitmap is ok for this case. SKBitmap should be disposed when IconImage disposing