- added caching for true color images as well

src/f_wipe.cpp

@@ -41,10 +41,13 @@ public:
 		Height = h;
 	}
 	
-	int CopyPixels(FBitmap *bmp) override
+	FBitmap GetBgraBitmap(PalEntry*, int *trans) override
 	{
-		bmp->CopyPixelDataRGB(0, 0, (uint8_t*)WorkBuffer.Data(), Width, Height, 4, Width*4, 0, CF_RGBA, nullptr);
-		return 0;
+		FBitmap bmp;
+		bmp.Create(Width, Height);
+		bmp.CopyPixelDataRGB(0, 0, (uint8_t*)WorkBuffer.Data(), Width, Height, 4, Width*4, 0, CF_RGBA, nullptr);
+		if (trans) *trans = 0;
+		return bmp;
 	}
 	
 	uint32_t *GetBuffer()

src/swrenderer/r_swscene.cpp

@@ -51,14 +51,17 @@ public:
 		UseType = ETextureType::MiscPatch;
 	}
 
-	int CopyPixels(FBitmap *bmp)
+	FBitmap GetBgraBitmap(PalEntry *, int *trans) override
 	{
-		PalEntry *pe = (PalEntry*)bmp->GetPixels();
+		FBitmap bmp;
+		bmp.Create(256, 1);
+		PalEntry *pe = (PalEntry*)bmp.GetPixels();
 		for (int i = 0; i < 256; i++)
 		{
 			pe[i] = GPalette.BaseColors[i].d | 0xff000000;
 		}
-		return 0;
+		if (trans) *trans = 0;
+		return bmp;
 	}
 };
 

src/textures/bitmap.h

@@ -120,7 +120,7 @@ public:
 		other.FreeBuffer = false;
 	}
 
-	FBitmap &operator=(const FBitmap &other) = delete;	// disallow because in nearly all cases this creates an unwanted copy.
+	FBitmap &operator=(const FBitmap &other) = delete;	// disallow because in nearly all cases this creates an unwanted copy. Use Copy instead.
 
 	FBitmap &operator=(FBitmap &&other)
 	{
@@ -136,7 +136,27 @@ public:
 		return *this;
 	}
 
-	virtual ~FBitmap()
+	void Copy(const FBitmap &other, bool deep = true)
+	{
+		if (data != nullptr && FreeBuffer) delete[] data;
+		Pitch = other.Pitch;
+		Width = other.Width;
+		Height = other.Height;
+		FreeBuffer = deep;
+		ClipRect = other.ClipRect;
+		if (deep)
+		{
+			data = new uint8_t[Pitch * Height];
+			memcpy(data, other.data, Pitch * Height);
+		}
+		else
+		{
+			data = other.data;
+		}
+	}
+
+	
+	~FBitmap()
 	{
 		Destroy();
 	}
@@ -210,10 +230,10 @@ public:
 	void Zero();
 
 
-	virtual void CopyPixelDataRGB(int originx, int originy, const uint8_t *patch, int srcwidth, 
+	void CopyPixelDataRGB(int originx, int originy, const uint8_t *patch, int srcwidth,
 								int srcheight, int step_x, int step_y, int rotate, int ct, FCopyInfo *inf = NULL,
 		/* for PNG tRNS */		int r=0, int g=0, int b=0);
-	virtual void CopyPixelData(int originx, int originy, const uint8_t * patch, int srcwidth, int srcheight, 
+	void CopyPixelData(int originx, int originy, const uint8_t * patch, int srcwidth, int srcheight,
 								int step_x, int step_y, int rotate, PalEntry * palette, FCopyInfo *inf = NULL);
 
 

src/textures/formats/brightmaptexture.cpp

@@ -77,4 +77,4 @@ int FBrightmapTexture::CopyPixels(FBitmap *bmp, int conversion)
 FTexture *CreateBrightmapTexture(FImageSource *tex)
 {
 	return new FImageTexture(new FBrightmapTexture(tex));
-}
+}

src/textures/formats/fontchars.cpp

@@ -73,7 +73,7 @@ TArray<uint8_t> FFontChar1::CreatePalettedPixels (int)
 	// Make the texture as normal, then remap it so that all the colors
 	// are at the low end of the palette
 	// Why? It only creates unnecessary work!
-	auto Pixels = BaseTexture->CreatePalettedPixels(normal);
+	auto Pixels = BaseTexture->GetPalettedPixels(normal);
 
 	if (SourceRemap)
 	{

src/textures/formats/multipatchtexture.cpp

@@ -315,9 +315,7 @@ int FMultiPatchTexture::CopyPixels(FBitmap *bmp, int conversion)
 			}
 		}
 
-		FBitmap Pixels;
-		Pixels.Create(Width, Height);
-		ret = Parts[i].Image->CopyPixels(&Pixels, conversion);
+		FBitmap Pixels = Parts[i].Image->GetCachedBitmap(nullptr, conversion, &ret);
 		bmp->Blit(Parts[i].OriginX, Parts[i].OriginY, Pixels, &info);
 		// treat -1 (i.e. unknown) as absolute. We have no idea if this may have overwritten previous info so a real check needs to be done.
 		if (ret == -1) retv = ret;

src/textures/image.cpp

@@ -55,6 +55,8 @@ struct PrecacheDataPaletted
 struct PrecacheDataRgba
 {
 	FBitmap Pixels;
+	int TransInfo;
+	int RefCount;
 	int ImageID;
 };
 
@@ -81,36 +83,32 @@ PalettedPixels FImageSource::GetCachedPalettedPixels(int conversion)
 
 	FString name;
 	Wads.GetLumpName(name, SourceLump);
-	if (name.CompareNoCase("W_136") == 0)
-	{
-		int a = 0;
-	}
 
 	std::pair<int, int> *info = nullptr;
 	auto imageID = ImageID;
 
 	// Do we have this image in the cache?
-	unsigned index = precacheDataPaletted.FindEx([=](PrecacheDataPaletted &entry) { return entry.ImageID == imageID; });
+	unsigned index = conversion != normal? UINT_MAX : precacheDataPaletted.FindEx([=](PrecacheDataPaletted &entry) { return entry.ImageID == imageID; });
 	if (index < precacheDataPaletted.Size())
 	{
 		auto cache = &precacheDataPaletted[index];
 
 		if (cache->RefCount > 1)
 		{
-			Printf("returning reference to %s, refcount = %d\n", name.GetChars(), cache->RefCount);
+			//Printf("returning reference to %s, refcount = %d\n", name.GetChars(), cache->RefCount);
 			ret.Pixels.Set(cache->Pixels.Data(), cache->Pixels.Size());
 			cache->RefCount--;
 		}
 		else if (cache->Pixels.Size() > 0)
 		{
-			Printf("returning contents of %s, refcount = %d\n", name.GetChars(), cache->RefCount);
+			//Printf("returning contents of %s, refcount = %d\n", name.GetChars(), cache->RefCount);
 			ret.PixelStore = std::move(cache->Pixels);
 			ret.Pixels.Set(ret.PixelStore.Data(), ret.PixelStore.Size());
 			precacheDataPaletted.Delete(index);
 		}
 		else
 		{
-			Printf("something bad happened for %s, refcount = %d\n", name.GetChars(), cache->RefCount);
+			//Printf("something bad happened for %s, refcount = %d\n", name.GetChars(), cache->RefCount);
 		}
 	}
 	else
@@ -120,13 +118,13 @@ PalettedPixels FImageSource::GetCachedPalettedPixels(int conversion)
 		if (!info || info->second <= 1 || conversion != normal)
 		{
 			// This is either the only copy needed or some access outside the caching block. In these cases create a new one and directly return it.
-			Printf("returning fresh copy of %s\n", name.GetChars());
+			//Printf("returning fresh copy of %s\n", name.GetChars());
 			ret.PixelStore = CreatePalettedPixels(conversion);
 			ret.Pixels.Set(ret.PixelStore.Data(), ret.PixelStore.Size());
 		}
 		else
 		{
-			Printf("creating cached entry for %s, refcount = %d\n", name.GetChars(), info->second);
+			//Printf("creating cached entry for %s, refcount = %d\n", name.GetChars(), info->second);
 			// This is the first time it gets accessed and needs to be placed in the cache.
 			PrecacheDataPaletted *pdp = &precacheDataPaletted[precacheDataPaletted.Reserve(1)];
 
@@ -162,7 +160,7 @@ TArray<uint8_t> FImageSource::GetPalettedPixels(int conversion)
 
 //===========================================================================
 //
-// FImageSource::CopyPixels 
+// FImageSource::CopyPixels
 //
 // this is the generic case that can handle
 // any properly implemented texture for software rendering.
@@ -196,6 +194,90 @@ int FImageSource::CopyTranslatedPixels(FBitmap *bmp, PalEntry *remap)
 //
 //==========================================================================
 
+FBitmap FImageSource::GetCachedBitmap(PalEntry *remap, int conversion, int *ptrans)
+{
+	FBitmap ret;
+	
+	FString name;
+	int trans = -1;
+	Wads.GetLumpName(name, SourceLump);
+	
+	std::pair<int, int> *info = nullptr;
+	auto imageID = ImageID;
+	
+	if (remap != nullptr)
+	{
+		// Remapped images are never run through the cache because they would complicate matters too much for very little gain.
+		// Translated images are normally sprites which normally just consist of a single image and use no composition.
+		// Additionally, since translation requires the base palette, the really time consuming stuff will never be subjected to it.
+		ret.Create(Width, Height);
+		trans = CopyTranslatedPixels(&ret, remap);
+	}
+	else
+	{
+		if (conversion == luminance) conversion = normal;	// luminance has no meaning for true color.
+		// Do we have this image in the cache?
+		unsigned index = conversion != normal? UINT_MAX : precacheDataRgba.FindEx([=](PrecacheDataRgba &entry) { return entry.ImageID == imageID; });
+		if (index < precacheDataRgba.Size())
+		{
+			auto cache = &precacheDataRgba[index];
+			
+			trans = cache->TransInfo;
+			if (cache->RefCount > 1)
+			{
+				Printf("returning reference to %s, refcount = %d\n", name.GetChars(), cache->RefCount);
+				ret.Copy(cache->Pixels, false);
+				cache->RefCount--;
+			}
+			else if (cache->Pixels.GetPixels())
+			{
+				Printf("returning contents of %s, refcount = %d\n", name.GetChars(), cache->RefCount);
+				ret = std::move(cache->Pixels);
+				precacheDataRgba.Delete(index);
+			}
+			else
+			{
+				// This should never happen if the function is implemented correctly
+				Printf("something bad happened for %s, refcount = %d\n", name.GetChars(), cache->RefCount);
+				ret.Create(Width, Height);
+				trans = CopyPixels(&ret, normal);
+			}
+		}
+		else
+		{
+			// The image wasn't cached. Now there's two possibilities:
+			auto info = precacheInfo.CheckKey(ImageID);
+			if (!info || info->first <= 1 || conversion != normal)
+			{
+				// This is either the only copy needed or some access outside the caching block. In these cases create a new one and directly return it.
+				Printf("returning fresh copy of %s\n", name.GetChars());
+				ret.Create(Width, Height);
+				trans = CopyPixels(&ret, conversion);
+			}
+			else
+			{
+				Printf("creating cached entry for %s, refcount = %d\n", name.GetChars(), info->first);
+				// This is the first time it gets accessed and needs to be placed in the cache.
+				PrecacheDataRgba *pdr = &precacheDataRgba[precacheDataRgba.Reserve(1)];
+				
+				pdr->ImageID = imageID;
+				pdr->RefCount = info->first - 1;
+				info->first = 0;
+				pdr->Pixels.Create(Width, Height);
+				trans = pdr->TransInfo = CopyPixels(&pdr->Pixels, normal);
+				ret.Copy(pdr->Pixels, false);
+			}
+		}
+	}
+	return ret;
+}
+
+//==========================================================================
+//
+//
+//
+//==========================================================================
+
 void FImageSource::CollectForPrecache(PrecacheInfo &info, bool requiretruecolor)
 {
 	auto val = info.CheckKey(ImageID);
@@ -228,12 +310,6 @@ void FImageSource::RegisterForPrecache(FImageSource *img)
 	img->CollectForPrecache(precacheInfo);
 }
 
-FBitmap FImageSource::GetPixelsWithCache(int conversion)
-{
-	return FBitmap();
-}
-
-
 //==========================================================================
 //
 //

src/textures/image.h

@@ -25,7 +25,7 @@ private:
 // All it can do is provide raw image data to its users.
 class FImageSource
 {
-	friend class FBrightmapImage;
+	friend class FBrightmapTexture;
 protected:
 
 	static FMemArena ImageArena;
@@ -38,6 +38,14 @@ protected:
 	bool bUseGamePalette = false;				// true if this is an image without its own color set.
 	int ImageID = -1;
 
+	// Internal image creation functions. All external access should go through the cache interface,
+	// so that all code can benefit from future improvements to that.
+
+	virtual TArray<uint8_t> CreatePalettedPixels(int conversion);
+	virtual int CopyPixels(FBitmap *bmp, int conversion);			// This will always ignore 'luminance'.
+	int CopyTranslatedPixels(FBitmap *bmp, PalEntry *remap);
+
+
 public:
 
 	void CopySize(FImageSource &other)
@@ -59,22 +67,19 @@ public:
 	
 	// 'noremap0' will only be looked at by FPatchTexture and forwarded by FMultipatchTexture.
 
-	// Always creates a new pixel buffer for the texture
-	virtual TArray<uint8_t> CreatePalettedPixels(int conversion);		
-
 	// Either returns a reference to the cache, or a newly created item. The return of this has to be considered transient. If you need to store the result, use GetPalettedPixels
 	PalettedPixels GetCachedPalettedPixels(int conversion);
 
 	// tries to get a buffer from the cache. If not available, create a new one. If further references are pending, create a copy.
 	TArray<uint8_t> GetPalettedPixels(int conversion);
 
-	virtual int CopyPixels(FBitmap *bmp, int conversion);			// This will always ignore 'luminance'.
-	int CopyTranslatedPixels(FBitmap *bmp, PalEntry *remap);
-	static void ClearImages() { ImageArena.FreeAll(); ImageForLump.Clear(); NextID = 0; }
-	static FImageSource * FImageSource::GetImage(int lumpnum, ETextureType usetype);
+	
+	// Unlile for paletted images there is no variant here that returns a persistent bitmap, because all users have to process the returned image into another format.
+	FBitmap GetCachedBitmap(PalEntry *remap, int conversion, int *trans = nullptr);
+
+	static void ClearImages() { ImageArena.FreeAll(); ImageForLump.Clear(); NextID = 0; }
+	static FImageSource * GetImage(int lumpnum, ETextureType usetype);
 
-	// These functions either allocate a new buffer or reuse the last one, if its reference count was greater than 1 when last used.
-	FBitmap GetPixelsWithCache(int conversion);
 
 
 	// Conversion option
@@ -143,9 +148,7 @@ public:
 	}
 
 	FImageSource *GetImage() const override { return mImage; }
+	FBitmap GetBgraBitmap(PalEntry *p, int *trans) override;
 
-protected:
-	int CopyPixels(FBitmap *bmp) override;
-	
 };
 

src/textures/imagetexture.cpp

@@ -73,9 +73,9 @@ FImageTexture::FImageTexture(FImageSource *img, const char *name)
 //
 //===========================================================================
 
-int FImageTexture::CopyPixels(FBitmap *bmp)
+FBitmap FImageTexture::GetBgraBitmap(PalEntry *p, int *trans)
 {
-	return mImage->CopyPixels(bmp, bNoRemap0? FImageSource::noremap0 : FImageSource::normal);
+	return mImage->GetCachedBitmap(p, bNoRemap0? FImageSource::noremap0 : FImageSource::normal, trans);
 }	
 
 //===========================================================================

src/textures/skyboxtexture.cpp

@@ -24,6 +24,7 @@
 #include "w_wad.h"
 #include "textures.h"
 #include "skyboxtexture.h"
+#include "bitmap.h"
 
 
 
@@ -60,9 +61,9 @@ TArray<uint8_t> FSkyBox::Get8BitPixels(bool alphatex)
 //
 //-----------------------------------------------------------------------------
 
-int FSkyBox::CopyPixels(FBitmap *bmp)
+FBitmap FSkyBox::GetBgraBitmap(PalEntry *p, int *trans)
 {
-	if (faces[0]) return faces[0]->CopyPixels(bmp);
-	return 0;
+	if (faces[0]) return faces[0]->GetBgraBitmap(p, trans);
+	return FTexture::GetBgraBitmap(p, trans);
 }
 

src/textures/skyboxtexture.h

@@ -17,7 +17,7 @@ public:
 
 	FSkyBox(const char *name = nullptr);
 	TArray<uint8_t> Get8BitPixels(bool alphatex);
-	int CopyPixels(FBitmap *bmp);
+	FBitmap GetBgraBitmap(PalEntry *, int *trans) override;
 
 	void SetSize()
 	{

src/textures/texture.cpp

@@ -202,42 +202,17 @@ void FTexture::SetFrontSkyLayer ()
 
 //===========================================================================
 //
-// FTexture::CopyPixels 
+// FTexture::GetBgraBitmap
 //
-// this is the generic case that can handle
-// any properly implemented texture for software rendering.
-// Its drawback is that it is limited to the base palette which is
-// why all classes that handle different palettes should subclass this
-// method
+// Default returns just an empty bitmap. This needs to be overridden by
+// any subclass that actually does return a software pixel buffer.
 //
 //===========================================================================
 
-int FTexture::CopyPixels(FBitmap *bmp)
-{
-	PalEntry *palette = screen->GetPalette();
-	for(int i=1;i<256;i++) palette[i].a = 255;	// set proper alpha values
-	auto ppix = Get8BitPixels(false);	// should use composition cache
-	bmp->CopyPixelData(0, 0, ppix.Data(), Width, Height, Height, 1, 0, palette, nullptr);
-	for(int i=1;i<256;i++) palette[i].a = 0;
-	return 0;
-}
-
-int FTexture::CopyTranslatedPixels(FBitmap *bmp, PalEntry *remap)
-{
-	auto ppix = Get8BitPixels(false);	// should use composition cache
-	bmp->CopyPixelData(0, 0, ppix.Data(), Width, Height, Height, 1, 0, remap, nullptr);
-	return 0;
-}
-
 FBitmap FTexture::GetBgraBitmap(PalEntry *remap, int *ptrans)
 {
 	FBitmap bmp;
-	int trans;
-
-	bmp.Create(GetWidth(), GetHeight());
-	if (!remap) trans = CopyPixels(&bmp);
-	else trans = CopyTranslatedPixels(&bmp, remap);
-	if (ptrans) *ptrans = trans;
+	bmp.Create(Width, Height);
 	return bmp;
 }
 

src/textures/textures.h

@@ -123,6 +123,8 @@ class FTextureManager;
 class FTerrainTypeArray;
 class IHardwareTexture;
 class FMaterial;
+class FMultipatchTextureBuilder;
+
 extern int r_spriteadjustSW, r_spriteadjustHW;
 
 class FNullTextureID : public FTextureID
@@ -304,7 +306,7 @@ public:
 
 	// Returns the whole texture, stored in column-major order
 	virtual TArray<uint8_t> Get8BitPixels(bool alphatex);
-	/*virtual*/ FBitmap GetBgraBitmap(PalEntry *remap, int *trans = nullptr);
+	virtual FBitmap GetBgraBitmap(PalEntry *remap, int *trans = nullptr);
 
 public:
 	static bool SmoothEdges(unsigned char * buffer,int w, int h);
@@ -385,9 +387,6 @@ protected:
 	// Returns true if GetPixelsBgra includes mipmaps
 	virtual bool Mipmapped() { return true; }
 
-	virtual int CopyPixels(FBitmap *bmp);
-	int CopyTranslatedPixels(FBitmap *bmp, PalEntry *remap);
-
 	void SetSpeed(float fac) { shaderspeed = fac; }
 
 	int GetWidth () { return Width; }