[fixed] MRT on D3D9 bug or not ?

wing64 · Post by **wing64** » Fri Aug 06, 2010 3:24 pm

Hello
I try use MRT on D3D9 driver and result is wrong in RT2 and RT3 (RT1 good result) but in OGL work great all in RT.

I very confuse about that. Texture format & size -> 512x512, ECF_A8R8G8B8 all in render texture. (OpenGL run perfectly in same irrlicht sourcecode) I'm not sure my program is wrong or miss some MRT process in D3D9

Please suggest me. (Program run on i7-930, ATI RadeonHD 4670 with lastest driver.)

Pixel shader here D3D9:

Code: Select all

struct PS_INPUT
{
	float4 Position		: POSITION0;	
};

struct PS_OUTPUT
{
	float4	Color		: COLOR0;
	float4	Normal		: COLOR1;
	float4	Depth		: COLOR2;
};

PS_OUTPUT pixelMain( PS_INPUT Input )
{
	PS_OUTPUT Output;
	
	Output.Color = float4(1.0,1.0,1.0,1.0);
	Output.Normal = float4(0.0,1.0,0.0,1.0);
	Output.Depth = float4(0.0,0.0,1.0,1.0);
	
	return Output;
}//*/

result here D3D9 (wrong!!! in RT2 and RT3):

Pixel shader here OGL:

Code: Select all

void main(void)
{
	gl_FragData[0] = vec4(1.0,1.0,1.0,1.0);
	gl_FragData[1] = vec4(0.0,1.0,0.0,1.0);
	gl_FragData[2] = vec4(0.0,0.0,1.0,1.0);

}

result here OGL (right all RT):

sourcode:

Code: Select all

// variable
ITexture*		gbuffer[3];
array<IRenderTarget> gbufferlist;

// allocate buffer
gbuffer[0] = driver->addRenderTargetTexture( texsize, "rta", ECF_A8R8G8B8 );
gbuffer[1] = driver->addRenderTargetTexture( texsize, "rtb", ECF_A8R8G8B8 );
gbuffer[2] = driver->addRenderTargetTexture( texsize, "rtc", ECF_A8R8G8B8 );
for( u32 i = 0; i < 3; ++i )
	gbufferlist.push_back( IRenderTarget(gbuffer[i]) );

// render
driver->setRenderTarget( gbufferlist );
smgr->drawAll();

// draw debug rt
driver->draw2DImage(gbuffer[0], core::position2d<s32>(0,0));
driver->draw2DImage(gbuffer[1], core::position2d<s32>(256,0));
driver->draw2DImage(gbuffer[2], core::position2d<s32>(512,0));

wing64 · Post by **wing64** » Mon Aug 09, 2010 3:28 am

Now i find out a problem MRT on D3D9 driver at setRenderTarget() function, Engine check IRenderTarget alway if haven't texture pointer then return but D3D need set MRT each layer to NULL before next render.

Solution is:

CD3D9Driver.cpp [changed]:

Code: Select all

//! Sets multiple render targets
bool CD3D9Driver::setRenderTarget(const core::array<video::IRenderTarget>& targets,
				bool clearBackBuffer, bool clearZBuffer, SColor color)
{
	if (targets.size()==0)
		return setRenderTarget(0, clearBackBuffer, clearZBuffer, color);

	u32 maxMultipleRTTs = core::min_(4u, targets.size());

	/*for (u32 i = 0; i < maxMultipleRTTs; ++i)
	{
		if (targets[i].TargetType != ERT_RENDER_TEXTURE || !targets[i].RenderTexture)
		{
			maxMultipleRTTs = i;
			os::Printer::log("Missing texture for MRT.", ELL_WARNING);
			break;
		}

		// check for right driver type

		if (targets[i].RenderTexture->getDriverType() != EDT_DIRECT3D9)
		{
			maxMultipleRTTs = i;
			os::Printer::log("Tried to set a texture not owned by this driver.", ELL_WARNING);
			break;
		}

		// check for valid render target

		if (!targets[i].RenderTexture->isRenderTarget())
		{
			maxMultipleRTTs = i;
			os::Printer::log("Tried to set a non render target texture as render target.", ELL_WARNING);
			break;
		}

		// check for valid size

		if (targets[0].RenderTexture->getSize() != targets[i].RenderTexture->getSize())
		{
			maxMultipleRTTs = i;
			os::Printer::log("Render target texture has wrong size.", ELL_WARNING);
			break;
		}
	}//*/
	if (maxMultipleRTTs==0)
	{
		os::Printer::log("Fatal Error: No valid MRT found.", ELL_ERROR);
		return false;
	}

	CD3D9Texture* tex = static_cast<CD3D9Texture*>(targets[0].RenderTexture);

	// check if we should set the previous RT back

	bool ret = true;

	// we want to set a new target. so do this.
	// store previous target

	if (!PrevRenderTarget)
	{
		if (FAILED(pID3DDevice->GetRenderTarget(0, &PrevRenderTarget)))
		{
			os::Printer::log("Could not get previous render target.", ELL_ERROR);
			return false;
		}
	}

	// set new render target

	D3DRENDERSTATETYPE colorWrite[4]={D3DRS_COLORWRITEENABLE, D3DRS_COLORWRITEENABLE1, D3DRS_COLORWRITEENABLE2, D3DRS_COLORWRITEENABLE3};
	for (u32 i = 0; i < maxMultipleRTTs; ++i)
	{
		if( targets[i].RenderTexture )
		{
			if (FAILED(pID3DDevice->SetRenderTarget(i, static_cast<CD3D9Texture*>(targets[i].RenderTexture)->getRenderTargetSurface())))
			{
				os::Printer::log("Error: Could not set render target.", ELL_ERROR);
				return false;
			}
		}
		else
			pID3DDevice->SetRenderTarget(i, 0 );

		if (i<4 && (i==0 || queryFeature(EVDF_MRT_COLOR_MASK)))
		{
			const DWORD flag =
				((targets[i].ColorMask & ECP_RED)?D3DCOLORWRITEENABLE_RED:0) |
				((targets[i].ColorMask & ECP_GREEN)?D3DCOLORWRITEENABLE_GREEN:0) |
				((targets[i].ColorMask & ECP_BLUE)?D3DCOLORWRITEENABLE_BLUE:0) |
				((targets[i].ColorMask & ECP_ALPHA)?D3DCOLORWRITEENABLE_ALPHA:0);
			pID3DDevice->SetRenderState(colorWrite[i], flag);
		}
	}

	if (tex == 0)
	{
		if (PrevRenderTarget)
		{
			if (FAILED(pID3DDevice->SetRenderTarget(0, PrevRenderTarget)))
			{
				os::Printer::log("Error: Could not set back to previous render target.", ELL_ERROR);
				ret = false;
			}
			if (FAILED(pID3DDevice->SetDepthStencilSurface(DepthBuffers[0]->Surface)))
			{
				os::Printer::log("Error: Could not set main depth buffer.", ELL_ERROR);
			}

			CurrentRendertargetSize = core::dimension2d<u32>(0,0);
			PrevRenderTarget->Release();
			PrevRenderTarget = 0;
		}
	}
	else
	{
		CurrentRendertargetSize = tex->getSize();

		if (FAILED(pID3DDevice->SetDepthStencilSurface(tex->DepthSurface->Surface)))
		{
			os::Printer::log("Error: Could not set new depth buffer.", ELL_ERROR);
		}
	}

	if (clearBackBuffer || clearZBuffer)
	{
		DWORD flags = 0;

		if (clearBackBuffer)
			flags |= D3DCLEAR_TARGET;

		if (clearZBuffer)
			flags |= D3DCLEAR_ZBUFFER;

		pID3DDevice->Clear(0, NULL, flags, color.color, 1.0f, 0);
	}

	return ret;
}

In program loop you need add empylist:

Code: Select all

 // declare varible
ITexture*		gbuffer[3];
array<IRenderTarget> gbufferlist;
array<IRenderTarget> emptylist;

// allocate mrt
gbuffer[0] = g_engine.driver->addRenderTargetTexture( texsize, "rt", ECF_A8R8G8B8 );
gbuffer[1] = g_engine.driver->addRenderTargetTexture( texsize, "rt", ECF_A8R8G8B8 );
gbuffer[2] = g_engine.driver->addRenderTargetTexture( texsize, "rt", ECF_A8R8G8B8 );
for( u32 i = 0; i < 3; ++i )
{
	gbufferlist.push_back( IRenderTarget(gbuffer[i]) );
	emptylist.push_back( IRenderTarget(0) );
}

// render MRT
g_engine.driver->setRenderTarget( gbufferlist );
g_engine.smgr->drawAll();

// render debug mrt
g_engine.driver->setRenderTarget( emptylist );
g_engine.driver->draw2DImage(gbuffer[0], core::position2d<s32>(0,0));
g_engine.driver->draw2DImage(gbuffer[1], core::position2d<s32>(256,0));
g_engine.driver->draw2DImage(gbuffer[2], core::position2d<s32>(512,0));

Result here:

slavik262 · Post by **slavik262** » Mon Sep 13, 2010 4:39 am

@Devs: This still hasn't been patched in the latest build. Would you kindly?

Also, @Wing64: Is there a more elegant solution possible than just using IVideoDriver::setRenderTarget with an empty list?

hybrid · Post by **hybrid** » Mon Sep 13, 2010 7:40 am

slavik262 wrote:@Devs: This still hasn't been patched in the latest build. Would you kindly?

Would you please send in my new developer machine, where I can test those things. Also, please send a pay check to my employer, in order to calm him down about my many side jobs I have to do.
Even though there was only little sarcasm in this reply, I try to give also an answer without any: Just calm down, we will fix as many issues as we can until the new version gets out. This one probably won't make it into the 1.7.2 release anyway, so it has enough time to settle.

slavik262 · Post by **slavik262** » Mon Sep 13, 2010 11:23 am

Heh, sorry. I realize you guys have lives; I just figured this one would get fixed faster than most because Wing64 already isolated it and found a fix, and several other fixes have went out since.

hybrid · Post by **hybrid** » Mon Sep 13, 2010 4:59 pm

No, sorry, that's not a fix but a workaround. The proper fix is to set all non-used render targets to NULL. I'll add this fix later on.
And once again: Please submit bug tickets for those problems. This forum is nice to discuss things. But when it comes to fixing things, I still start at the bug tracker. Always.

slavik262 · Post by **slavik262** » Tue Sep 14, 2010 3:40 am

Would this be desired behavior? If I understand properly, this should clear out any RTs we're not using from a previous call. Initial tests show that this behaves as it should.

Code: Select all

//! sets a render target
bool CD3D9Driver::setRenderTarget(video::ITexture* texture,
		bool clearBackBuffer, bool clearZBuffer, SColor color)
{
	// check for right driver type

	if (texture && texture->getDriverType() != EDT_DIRECT3D9)
	{
		os::Printer::log("Fatal Error: Tried to set a texture not owned by this driver.", ELL_ERROR);
		return false;
	}

	// check for valid render target

	if (texture && !texture->isRenderTarget())
	{
		os::Printer::log("Fatal Error: Tried to set a non render target texture as render target.", ELL_ERROR);
		return false;
	}

	CD3D9Texture* tex = static_cast<CD3D9Texture*>(texture);

	// check if we should set the previous RT back

	bool ret = true;

	if (tex == 0)
	{
		if (PrevRenderTarget)
		{
			if (FAILED(pID3DDevice->SetRenderTarget(0, PrevRenderTarget)))
			{
				os::Printer::log("Error: Could not set back to previous render target.", ELL_ERROR);
				ret = false;
			}
			if (FAILED(pID3DDevice->SetDepthStencilSurface(DepthBuffers[0]->Surface)))
			{
				os::Printer::log("Error: Could not set main depth buffer.", ELL_ERROR);
			}

			CurrentRendertargetSize = core::dimension2d<u32>(0,0);
			PrevRenderTarget->Release();
			PrevRenderTarget = 0;
		}
	}
	else
	{
		// we want to set a new target. so do this.

		// store previous target

		if (!PrevRenderTarget)
		{
			if (FAILED(pID3DDevice->GetRenderTarget(0, &PrevRenderTarget)))
			{
				os::Printer::log("Could not get previous render target.", ELL_ERROR);
				return false;
			}
		}

		// set new render target

		if (FAILED(pID3DDevice->SetRenderTarget(0, tex->getRenderTargetSurface())))
		{
			os::Printer::log("Error: Could not set render target.", ELL_ERROR);
			return false;
		}

		CurrentRendertargetSize = tex->getSize();

		if (FAILED(pID3DDevice->SetDepthStencilSurface(tex->DepthSurface->Surface)))
		{
			os::Printer::log("Error: Could not set new depth buffer.", ELL_ERROR);
		}
	}

	//Clear out any RTs that we don't want
	
	IDirect3DSurface9* temp;
	for(u32 i = 1; i < 4u; ++i)
	{		
		if(!FAILED(pID3DDevice->GetRenderTarget(i, &temp)))
		{
			if(FAILED(pID3DDevice->SetRenderTarget(i, 0)))
			{
				os::Printer::log("Could not clear old render target.", ELL_WARNING);
			}
		}
	}

	if (clearBackBuffer || clearZBuffer)
	{
		DWORD flags = 0;

		if (clearBackBuffer)
			flags |= D3DCLEAR_TARGET;

		if (clearZBuffer)
			flags |= D3DCLEAR_ZBUFFER;

		pID3DDevice->Clear(0, NULL, flags, color.color, 1.0f, 0);
	}

	return ret;
}


//! Sets multiple render targets
bool CD3D9Driver::setRenderTarget(const core::array<video::IRenderTarget>& targets,
				bool clearBackBuffer, bool clearZBuffer, SColor color)
{
	if (targets.size()==0)
		return setRenderTarget(0, clearBackBuffer, clearZBuffer, color);

	u32 maxMultipleRTTs = core::min_(4u, targets.size());

	for (u32 i = 0; i < maxMultipleRTTs; ++i)
	{
		if (targets[i].TargetType != ERT_RENDER_TEXTURE || !targets[i].RenderTexture)
		{
			maxMultipleRTTs = i;
			os::Printer::log("Missing texture for MRT.", ELL_WARNING);
			break;
		}

		// check for right driver type

		if (targets[i].RenderTexture->getDriverType() != EDT_DIRECT3D9)
		{
			maxMultipleRTTs = i;
			os::Printer::log("Tried to set a texture not owned by this driver.", ELL_WARNING);
			break;
		}

		// check for valid render target

		if (!targets[i].RenderTexture->isRenderTarget())
		{
			maxMultipleRTTs = i;
			os::Printer::log("Tried to set a non render target texture as render target.", ELL_WARNING);
			break;
		}

		// check for valid size

		if (targets[0].RenderTexture->getSize() != targets[i].RenderTexture->getSize())
		{
			maxMultipleRTTs = i;
			os::Printer::log("Render target texture has wrong size.", ELL_WARNING);
			break;
		}
	}
	if (maxMultipleRTTs==0)
	{
		os::Printer::log("Fatal Error: No valid MRT found.", ELL_ERROR);
		return false;
	}

	CD3D9Texture* tex = static_cast<CD3D9Texture*>(targets[0].RenderTexture);

	// check if we should set the previous RT back

	bool ret = true;

	// we want to set a new target. so do this.
	// store previous target

	if (!PrevRenderTarget)
	{
		if (FAILED(pID3DDevice->GetRenderTarget(0, &PrevRenderTarget)))
		{
			os::Printer::log("Could not get previous render target.", ELL_ERROR);
			return false;
		}
	}

	// set new render target

	D3DRENDERSTATETYPE colorWrite[4]={D3DRS_COLORWRITEENABLE, D3DRS_COLORWRITEENABLE1, D3DRS_COLORWRITEENABLE2, D3DRS_COLORWRITEENABLE3};
	for (u32 i = 0; i < maxMultipleRTTs; ++i)
	{
		if (FAILED(pID3DDevice->SetRenderTarget(i, static_cast<CD3D9Texture*>(targets[i].RenderTexture)->getRenderTargetSurface())))
		{
			os::Printer::log("Error: Could not set render target.", ELL_ERROR);
			return false;
		}
		if (i<4 && (i==0 || queryFeature(EVDF_MRT_COLOR_MASK)))
		{
			const DWORD flag =
				((targets[i].ColorMask & ECP_RED)?D3DCOLORWRITEENABLE_RED:0) |
				((targets[i].ColorMask & ECP_GREEN)?D3DCOLORWRITEENABLE_GREEN:0) |
				((targets[i].ColorMask & ECP_BLUE)?D3DCOLORWRITEENABLE_BLUE:0) |
				((targets[i].ColorMask & ECP_ALPHA)?D3DCOLORWRITEENABLE_ALPHA:0);
			pID3DDevice->SetRenderState(colorWrite[i], flag);
		}
	}

	//Clear out any RTs that we don't want
	
	IDirect3DSurface9* temp;
	for(u32 i = maxMultipleRTTs; i < 4u; ++i)
	{		
		if(!FAILED(pID3DDevice->GetRenderTarget(i, &temp)))
		{
			if(FAILED(pID3DDevice->SetRenderTarget(i, 0)))
			{
				os::Printer::log("Could not clear old render target.", ELL_WARNING);
			}
		}
	}

	CurrentRendertargetSize = tex->getSize();

	if (FAILED(pID3DDevice->SetDepthStencilSurface(tex->DepthSurface->Surface)))
	{
		os::Printer::log("Error: Could not set new depth buffer.", ELL_ERROR);
	}

	if (clearBackBuffer || clearZBuffer)
	{
		DWORD flags = 0;

		if (clearBackBuffer)
			flags |= D3DCLEAR_TARGET;

		if (clearZBuffer)
			flags |= D3DCLEAR_ZBUFFER;

		pID3DDevice->Clear(0, NULL, flags, color.color, 1.0f, 0);
	}

	return ret;
}

hybrid · Post by **hybrid** » Tue Sep 14, 2010 6:55 am

Yeah, this fix comes near to the correct solution. There are two more things which I will approach: The limitation to 4 mrts, which is often not necessary (but also not directly related to this bug) and an optimization which will set only those mrts to 0 which are not already 0.
BTW: Is it possible to use mrts where one of the targets "in between" was not properly set? E.g., if mrt 1 fails to be set, but 0 and 2 are properly enabled?!

slavik262 · Post by **slavik262** » Tue Sep 14, 2010 11:32 am

hybrid wrote:Yeah, this fix comes near to the correct solution. There are two more things which I will approach: The limitation to 4 mrts, which is often not necessary (but also not directly related to this bug)

I was just following the existing code. If you look at the code of the MRT overload of CD3D9Driver::setRenderTarget, it makes the assumption multiple times that the limit is 4 mrts. This is the limit on all cards I'm aware of at the time, anyways.

Hybrid wrote:...and an optimization which will set only those mrts to 0 which are not already 0.

My code already does this. According to the MSDN article, GetRenderTarget will return D3DERR_NOTFOUND if there is no render target at the given index. If you follow the logic of my code, you'll see that because of this SetRenderTarget is only called if the RT is not already 0.

Hybrid wrote:BTW: Is it possible to use mrts where one of the targets "in between" was not properly set? E.g., if mrt 1 fails to be set, but 0 and 2 are properly enabled?!

Not according to my understanding. If I'm reading the MRT overload of CD3D9Driver::setRenderTarget correctly, the function will stop trying to set render targets the first time setting one fails.

hybrid · Post by **hybrid** » Tue Sep 14, 2010 1:34 pm

Yeah, correct. 4 is the maximal number of MRTs in D3D9, but it might be possible that we have less MRTs supported. Not sure if this is a fixed number for all d3d9 cards.
The thing with calls to the d3d9 layer is that those are always slow. So we usually cache as much information as possible. Hence I've added a new member which stores the last set number of MRTs, and clears only up to that limit. Due to the restriction to 4 MRTs, this seems to be not really necessary anymore, but I'm used to work with the OpenGL drivers, where such numbers can vary a lot between cards.
My last question was also regarding to the basic support in D3D. For OpenGL, it's AFAIK possible to use an imcomplete MRT set. The current version of the d3d9 driver in Irrlicht does not allow this, correct. But I'm not sure anymore, if I intended this when writing the MRT code, or if that was just a safe assumption.
Anyway, the code is now in SVN, so at least in IRrlicht 1.8 MRTs should also work for D3D9. I'm not sure if this code will move into 1.7.2, depends on the further actions in this driver.

wing64 · Post by **wing64** » Wed Sep 15, 2010 1:48 am

Thanks slavik262 & hybrid.