memleak in dx8/9 if no lower mipmap found!!!

[sdi2000]

hi all
the subject says all =)
In the dx drivers.
if there is no lower mipmap level in the texture the function
"createMipMaps" returns false without a "Release()" of the upperSurface.
But the function "GetSurfaceLevel" increments a refcounter!!!!

change from:

Code: Select all

	if (FAILED(hr) || !lowerSurface)
	{
		os::Printer::log("Could get lower surface level for mip map generation", ELL_WARNING);
		return false;
	}

to:

Code: Select all

	if (FAILED(hr) || !lowerSurface)
	{
		if(upperSurface)
			upperSurface->Release();
		os::Printer::log("Could get lower surface level for mip map generation", ELL_WARNING);
		return false;
	}

well i dont know if anyone has fix that... =)

[hybrid]

Ok, that one's fixed. But I assume there are lots of similar problems as the code continues with this lock and return style. Ayn hints on what to fix?

[sdi2000]

if the "GetSurfaceLevel" call failed the surface pointer is not valid so we dont need to check it...
a "lockRect" call doesnt increment any refcounter so we dont need an unlock call if any failed
okay to make this function safe release the surfaces before return.
an example function but it is manipulated for my multihead impl (multiple textures counted by the idx Index counter... but all the other calls are the same as before i adaped

Code: Select all

bool CD3D9Texture::createMipMaps(s32 level, u32 idx)
{
	if (HardwareMipMaps && Textures[idx])
	{
		// generate mipmaps in hardware

		Textures[idx]->GenerateMipSubLevels();
		return true;
	}
	//os::Printer::log("manual mipmap");

	// The D3DXFilterTexture function seems to get linked wrong when
	// compiling with both D3D8 and 9, causing it not to work in the D3D9 device.
	// So mipmapgeneration is replaced with my own bad generation here when
	// compiling with both D3D 8 and 9.

	IDirect3DSurface9* upperSurface = 0;
	IDirect3DSurface9* lowerSurface = 0;

	// get upper level
	HRESULT hr = Textures[idx]->GetSurfaceLevel(level-1, &upperSurface);
	if (FAILED(hr))
	{
		os::Printer::log("Could get upper surface level for mip map generation", ELL_WARNING);
		return false;
	}

	// get lower level
	hr = Textures[idx]->GetSurfaceLevel(level, &lowerSurface);
	if (FAILED(hr))
	{
		if(upperSurface)
			upperSurface->Release();
		os::Printer::log("Could get lower surface level for mip map generation", ELL_WARNING);
		return false;
	}

	D3DSURFACE_DESC upperDesc, lowerDesc;
	if(FAILED(upperSurface->GetDesc(&upperDesc)))
	{
		if(upperSurface)
			upperSurface->Release();
		if(lowerSurface)
			lowerSurface->Release();
		return false;
	}
	if(FAILED(lowerSurface->GetDesc(&lowerDesc)))
	{
		if(upperSurface)
			upperSurface->Release();
		if(lowerSurface)
			lowerSurface->Release();
		return false;
	}


	D3DLOCKED_RECT upperlr;
	D3DLOCKED_RECT lowerlr;

	// lock upper surface
	if (FAILED(upperSurface->LockRect(&upperlr, NULL, 0)))
	{
		os::Printer::log("Could not lock upper texture for mip map generation", ELL_WARNING);
		if(upperSurface)
			upperSurface->Release();
		if(lowerSurface)
			lowerSurface->Release();
		return false;
	}

	// lock lower surface
	if (FAILED(lowerSurface->LockRect(&lowerlr, NULL, 0)))
	{
		os::Printer::log("Could not lock lower texture for mip map generation", ELL_WARNING);
		if(upperSurface)
			upperSurface->Release();
		if(lowerSurface)
			lowerSurface->Release();
		return false;
	}

	if (upperDesc.Format != lowerDesc.Format)
	{
		os::Printer::log("Cannot copy mip maps with different formats.", ELL_WARNING);
	}
	else
	{
		if (upperDesc.Format == D3DFMT_A1R5G5B5)
			copy16BitMipMap((char*)upperlr.pBits, (char*)lowerlr.pBits,
					lowerDesc.Width, lowerDesc.Height,
					upperlr.Pitch, lowerlr.Pitch);
		else
		if (upperDesc.Format == D3DFMT_A8R8G8B8)
			copy32BitMipMap((char*)upperlr.pBits, (char*)lowerlr.pBits,
					lowerDesc.Width, lowerDesc.Height,
					upperlr.Pitch, lowerlr.Pitch);
		else
			os::Printer::log("Unsupported mipmap format, cannot copy.", ELL_WARNING);
	}

	// unlock
	if (FAILED(upperSurface->UnlockRect()))
	{
		if(upperSurface)
			upperSurface->Release();
		if(lowerSurface)
			lowerSurface->Release();
		return false;
	}
	if (FAILED(lowerSurface->UnlockRect()))
	{
		if(upperSurface)
			upperSurface->Release();
		if(lowerSurface)
			lowerSurface->Release();
		return false;
	}

	// release
	upperSurface->Release();
	lowerSurface->Release();

	if (upperDesc.Width <= 2 || upperDesc.Height <= 2)
		return true; // stop generating levels

	// generate next level
	return createMipMaps(level+1, idx);
}

i hope that i have nothing forget to check... hmm