Rework bitmap cache and enable it

Fixes https://gitlab.com/kicad/code/kicad/-/issues/12405
This commit is contained in:
Jon Evans 2023-01-02 22:14:22 -05:00
parent 8ab9934143
commit ac3ed02283
7 changed files with 107 additions and 57 deletions

View File

@ -54,16 +54,36 @@ BITMAP_BASE::BITMAP_BASE( const BITMAP_BASE& aSchBitmap )
if( aSchBitmap.m_image )
{
m_image = new wxImage( *aSchBitmap.m_image );
m_bitmap = new wxBitmap( *m_image );
m_image = new wxImage( *aSchBitmap.m_image );
m_bitmap = new wxBitmap( *m_image );
m_imageId = aSchBitmap.m_imageId;
}
}
void BITMAP_BASE::SetImage( wxImage* aImage )
{
delete m_image;
m_image = aImage;
rebuildBitmap();
}
void BITMAP_BASE::rebuildBitmap()
{
if( m_bitmap )
delete m_bitmap;
m_bitmap = new wxBitmap( *m_image );
m_imageId = KIID();
}
void BITMAP_BASE::ImportData( BITMAP_BASE* aItem )
{
*m_image = *aItem->m_image;
*m_bitmap = *aItem->m_bitmap;
m_imageId = aItem->m_imageId;
m_scale = aItem->m_scale;
m_ppi = aItem->m_ppi;
m_pixelSizeIu = aItem->m_pixelSizeIu;
@ -79,7 +99,7 @@ bool BITMAP_BASE::ReadImageFile( wxInputStream& aInStream )
delete m_image;
m_image = new_image.release();
m_bitmap = new wxBitmap( *m_image );
rebuildBitmap();
return true;
}
@ -97,7 +117,7 @@ bool BITMAP_BASE::ReadImageFile( const wxString& aFullFilename )
delete m_image;
m_image = new_image;
m_bitmap = new wxBitmap( *m_image );
rebuildBitmap();
return true;
}
@ -331,8 +351,8 @@ void BITMAP_BASE::Mirror( bool aVertically )
{
if( m_image )
{
*m_image = m_image->Mirror( not aVertically );
RebuildBitmap();
*m_image = m_image->Mirror( not aVertically );
rebuildBitmap();
}
}
@ -341,8 +361,18 @@ void BITMAP_BASE::Rotate( bool aRotateCCW )
{
if( m_image )
{
*m_image = m_image->Rotate90( aRotateCCW );
RebuildBitmap();
*m_image = m_image->Rotate90( aRotateCCW );
rebuildBitmap();
}
}
void BITMAP_BASE::ConvertToGreyscale()
{
if( m_image )
{
*m_image = m_image->ConvertToGreyscale();
rebuildBitmap();
}
}

View File

@ -47,9 +47,7 @@ PANEL_IMAGE_EDITOR::PANEL_IMAGE_EDITOR( wxWindow* aParent, BITMAP_BASE* aItem )
void PANEL_IMAGE_EDITOR::OnGreyScaleConvert( wxCommandEvent& event )
{
wxImage& image = *m_workingImage->GetImageData();
image = image.ConvertToGreyscale();
m_workingImage->RebuildBitmap();
m_workingImage->ConvertToGreyscale();
m_panelDraw->Refresh();
}

View File

@ -56,22 +56,11 @@
#include <functional>
#include <limits>
#include <memory>
#include <list>
using namespace std::placeholders;
using namespace KIGFX;
// Currently the bitmap cache is disabled because the code has serious issues and
// work fine does not work fine:
// created GL textures are never deleted when a bitmap is deleted
// the key to retrieve a CACHED_BITMAP is the BITMAP_BASE items* pointer.
// Because in code many BITMAP_BASE items are temporary cloned for drawing purposes,
// it creates a lot of CACHED_BITMAP never deleted thus created memory leak
// So to reenable the bitmaps cache, serious changes in code are needed:
// At least:
// - use a key that works on cloned BITMAP_BASE items
// - handle rotated bitmaps without create a new cached item
// - add a "garbage collector" to delete not existing BITMAP_BASE items
// After tests, caching bitmaps do not speedup significantly drawings.
#define DISABLE_BITMAP_CACHE
//#define DISABLE_BITMAP_CACHE
// The current font is "Ubuntu Mono" available under Ubuntu Font Licence 1.0
// (see ubuntu-font-licence-1.0.txt for details)
@ -93,7 +82,9 @@ namespace KIGFX
class GL_BITMAP_CACHE
{
public:
GL_BITMAP_CACHE() {}
GL_BITMAP_CACHE() :
m_cacheSize( 0 )
{}
~GL_BITMAP_CACHE();
@ -104,11 +95,18 @@ private:
{
GLuint id;
int w, h;
size_t size;
};
GLuint cacheBitmap( const BITMAP_BASE* aBitmap );
std::map< const BITMAP_BASE*, CACHED_BITMAP> m_bitmaps;
const size_t m_cacheMaxElements = 50;
const size_t m_cacheMaxSize = 256 * 1024 * 1024;
std::map<const KIID, CACHED_BITMAP> m_bitmaps;
std::list<KIID> m_cacheLru;
size_t m_cacheSize;
std::list<GLuint> m_freedTextureIds;
};
}; // namespace KIGFX
@ -124,17 +122,12 @@ GL_BITMAP_CACHE::~GL_BITMAP_CACHE()
GLuint GL_BITMAP_CACHE::RequestBitmap( const BITMAP_BASE* aBitmap )
{
#ifndef DISABLE_BITMAP_CACHE
auto it = m_bitmaps.find( aBitmap );
auto it = m_bitmaps.find( aBitmap->GetImageID() );
if( it != m_bitmaps.end() )
{
// A bitmap is found in cache bitmap. Ensure the associated texture
// is still valid.
// It can be destroyed somewhere or the corresponding bitmap can be
// modifed (rotated)
if( ( it->second.w == aBitmap->GetSizePixels().x ) &&
( it->second.h == aBitmap->GetSizePixels().y ) &&
glIsTexture( it->second.id ) )
// A bitmap is found in cache bitmap. Ensure the associated texture is still valid.
if( glIsTexture( it->second.id ) )
{
return it->second.id;
}
@ -142,6 +135,15 @@ GLuint GL_BITMAP_CACHE::RequestBitmap( const BITMAP_BASE* aBitmap )
{
// Delete the invalid bitmap cache and its data
glDeleteTextures( 1, &it->second.id );
m_freedTextureIds.emplace_back( it->second.id );
auto listIt = std::find( m_cacheLru.begin(), m_cacheLru.end(), it->first );
if( listIt != m_cacheLru.end() )
m_cacheLru.erase( listIt );
m_cacheSize -= it->second.size;
m_bitmaps.erase( it );
}
@ -169,10 +171,21 @@ GLuint GL_BITMAP_CACHE::cacheBitmap( const BITMAP_BASE* aBitmap )
extra_w = 4 - extra_w;
GLuint textureID;
glGenTextures( 1, &textureID );
if( m_freedTextureIds.empty() )
{
glGenTextures( 1, &textureID );
}
else
{
textureID = m_freedTextureIds.front();
m_freedTextureIds.pop_front();
}
// make_unique initializes this to 0, so extra pixels are transparent
auto buf = std::make_unique<uint8_t[]>( ( bmp.w + extra_w ) * bmp.h * 4 );
bmp.size = ( bmp.w + extra_w ) * bmp.h * 4;
auto buf = std::make_unique<uint8_t[]>( bmp.size );
const wxImage& imgData = *aBitmap->GetImageData();
for( int y = 0; y < bmp.h; y++ )
@ -205,7 +218,23 @@ GLuint GL_BITMAP_CACHE::cacheBitmap( const BITMAP_BASE* aBitmap )
bmp.id = textureID;
#ifndef DISABLE_BITMAP_CACHE
m_bitmaps[aBitmap] = bmp;
m_cacheLru.emplace_back( aBitmap->GetImageID() );
m_cacheSize += bmp.size;
m_bitmaps.emplace( aBitmap->GetImageID(), std::move( bmp ) );
if( m_cacheLru.size() > m_cacheMaxElements
|| m_cacheSize > m_cacheMaxSize )
{
KIID last = m_cacheLru.front();
CACHED_BITMAP& cachedBitmap = m_bitmaps[last];
m_cacheSize -= cachedBitmap.size;
glDeleteTextures( 1, &cachedBitmap.id );
m_freedTextureIds.emplace_back( cachedBitmap.id );
m_bitmaps.erase( last );
m_cacheLru.pop_front();
}
#endif
return textureID;

View File

@ -2968,7 +2968,6 @@ SCH_BITMAP* SCH_SEXPR_PARSER::parseImage()
wxMemoryInputStream istream( stream );
image->LoadFile( istream, wxBITMAP_TYPE_PNG );
bitmap->GetImage()->SetImage( image );
bitmap->GetImage()->SetBitmap( new wxBitmap( *image ) );
break;
}

View File

@ -692,7 +692,6 @@ SCH_BITMAP* SCH_LEGACY_PLUGIN::loadBitmap( LINE_READER& aReader )
wxMemoryInputStream istream( stream );
image->LoadFile( istream, wxBITMAP_TYPE_PNG );
bitmap->GetImage()->SetImage( image );
bitmap->GetImage()->SetBitmap( new wxBitmap( *image ) );
break;
}

View File

@ -27,6 +27,7 @@
#include <wx/bitmap.h>
#include <wx/image.h>
#include <kiid.h>
#include <math/box2.h>
namespace KIGFX
@ -69,27 +70,12 @@ public:
wxImage* GetImageData() { return m_image; }
const wxImage* GetImageData() const { return m_image; }
void SetImage( wxImage* aImage )
{
delete m_image;
m_image = aImage;
}
void SetImage( wxImage* aImage );
double GetScale() const { return m_scale; }
void SetScale( double aScale ) { m_scale = aScale; }
/*
* Rebuild the internal bitmap used to draw/plot image.
*
* This must be called after a #m_image change.
*/
void RebuildBitmap() { *m_bitmap = wxBitmap( *m_image ); }
void SetBitmap( wxBitmap* aBitMap )
{
delete m_bitmap;
m_bitmap = aBitMap;
}
KIID GetImageID() const { return m_imageId; }
/**
* Copy aItem image to this object and update #m_bitmap.
@ -219,6 +205,8 @@ public:
*/
void Rotate( bool aRotateCCW );
void ConvertToGreyscale();
/**
* Plot bitmap on plotter.
*
@ -233,6 +221,13 @@ public:
const KIGFX::COLOR4D& aDefaultColor, int aDefaultPensize ) const;
private:
/*
* Rebuild the internal bitmap used to draw/plot image.
*
* This must be called after a #m_image change.
*/
void rebuildBitmap();
double m_scale; // The scaling factor of the bitmap
// With m_pixelSizeIu, controls the actual draw size
wxImage* m_image; // the raw image data (png format)
@ -242,6 +237,7 @@ private:
// to internal KiCad units
// Usually does not change
int m_ppi; // the bitmap definition. the default is 300PPI
KIID m_imageId;
};

View File

@ -2903,7 +2903,6 @@ PCB_BITMAP* PCB_PARSER::parsePCB_BITMAP( BOARD_ITEM* aParent )
wxMemoryInputStream istream( stream );
image->LoadFile( istream, wxBITMAP_TYPE_PNG );
bitmap->GetImage()->SetImage( image );
bitmap->GetImage()->SetBitmap( new wxBitmap( *image ) );
break;
}