Engine Design - Improvements

Graphics and GPU Programming Programming

Started by Migi0027 August 31, 2013 01:35 PM

3 comments, last by Tasaq 10 years, 7 months ago

Migi0027

4,632

Author

August 31, 2013 01:35 PM

Hi guys!

So lately I have been changing my engine design, and how things are created (user mode). So I ask you, if you have time, to review from a users perspective how things are, good, bad, terrible?

Creating a simple scene, 3 Meshes (SkyBox, Diamond, Plane) - 3 Textures (Noise, Diamond Tex, Cubemap) - 2 Materials (Gem, Skybox):


// Initialize materials and textures
CeMatParser<CeMatMesh> *gemMat = new CeMatParser<CeMatMesh>();
gemMat->Bootstrap(engine.dev, CeURL(CEToMedia("Materials\\Gems\\gem_dualpass.cs")));

CeMatParser<CeMatMesh> *skyMat = new CeMatParser<CeMatMesh>();
skyMat->Bootstrap(engine.dev, CeURL(CEToMedia("Materials\\Sky\\sky_01.cs")));

CESDK::Texture2D* noise = new CESDK::Texture2D();
CESDK::Texture2D* reflection = new CESDK::Texture2D();
CESDK::TextureCube* env = new CESDK::TextureCube();

noise->CreateTexture(engine.dev, CEToMedia("Textures\\noise.gif"));
reflection->CreateTexture(engine.dev, CEToMedia("Textures\\reflection.jpg"));

LPCSTR *all = new LPCSTR[6];
all[0] = CEToMedia("Textures\\Skybox\\01\\x+.bmp");
all[1] = CEToMedia("Textures\\Skybox\\01\\x-.bmp");
all[2] = CEToMedia("Textures\\Skybox\\01\\y+.bmp");
all[3] = CEToMedia("Textures\\Skybox\\01\\y-.bmp");
all[4] = CEToMedia("Textures\\Skybox\\01\\z+.bmp");
all[5] = CEToMedia("Textures\\Skybox\\01\\z-.bmp");

env->CreateTexture(engine.dev, all);

// Create all meshes
UMesh *plane = new UMesh();
plane->InitPlane(engine.devcon, engine.dev);
plane->SetMaterial(&engine.Materials.diffuse);
plane->Scale(100, 1, 100);
plane->Translate(-50, 0, -50);

UMesh *gem = new UMesh();
gem->AssimpLoad(CEToMedia("Mesh\\Shiny\\perfect_diamond.obj"), engine.devcon, engine.dev, false, false);
gem->CalculateBuffers(engine.devcon, engine.dev);
gem->Translate(0, 3, 0);
gem->SetMaterial(gemMat);
gem->passes.SetBlend(CEPHASE_BLEND_ADDITIVE);

gem->Parameters.SetTexture(noise, gemMat->getSlot("noise"));
gem->Parameters.SetTexture(reflection, gemMat->getSlot("reflection"));
gem->Parameters.SetTexture(env, gemMat->getSlot("environment"));
	
UMesh *skybox = new UMesh();
skybox->AssimpLoad("sphere.obj", engine.devcon, engine.dev);
skybox->CalculateBuffers(engine.devcon, engine.dev);
skybox->Scale(100, 100, 100);
skybox->SetMaterial(skyMat);
	
skybox->Parameters.SetTexture(env, skyMat->getSlot("skyBox"));
	
// Push all the meshes
engine.Meshes.push_back(plane);
engine.Meshes.push_back(gem);
engine.Meshes.push_back(skybox);

And the shaders are written as following (Shown: Gem shader, just a testing shader):


Any description or text can go here. The parser ignores it.

shader "Gem_Shader"
{
	// Properties go here
    info = "Gem shader showing the passes feature";
	
    :input
    :{
		Texture2D noise; 
		Texture2D reflection;
		TextureCube environment;
		
		// To simplify
		#define OUT output
		
		#define lighting_intensity 0.7f
		#define NOSHADOWS
    :}
	
	:pass
	{
		:properties {
			Cull [Front];
		}
		
		:linkage
		:{
			float3 Reflect : REF;
		:}

		:vertex
		:{
			float3 ViewDir = normalize(mul(position, worldMatrix) - eyepos);
			output.Reflect = reflect(viewvec, mul(normal, worldMatrix));
			output.Reflect.y = -output.Reflect.y;
		:}

		:pixel
		:{
			float4 colors = reflection.Sample(ss, input.texcoord)/4.0f;
			float4 cubeMap = environment.Sample(ss, input.Reflect);
			
			output.Diffuse = float4( colors.rgb + (cubeMap.rgb / 2.0f) , 1.0f);
		:}
	}
	
	:pass
	{
		:properties {
			Cull [Back];
		}
		
		:linkage
		:{
			float3 Reflect : REF;
		:}

		:vertex
		:{
			float3 ViewDir = normalize(mul(position, worldMatrix) - eyepos);
			output.Reflect = reflect(viewvec, mul(normalize(float4(normal.xyz, 0)), worldMatrix));
			output.Reflect.y = -output.Reflect.y;
		:}

		:pixel
		:{
			float4 colors = reflection.Sample(ss, input.texcoord)/2.0f;
			float4 cubeMap = environment.Sample(ss, input.Reflect);
			
			output.Diffuse = float4( colors.rgb + (cubeMap.rgb)  , 0.6f);
		:}
	}
}

PS. The wierd :x thing will be changed, just not now...

The result isn't really the focus here, just how badly my design is.

So.. What do you think? (Improvements, removal, new way of doing that... Basically anything)

Thanks, as always!

-MIGI0027

FastCall22: "I want to make the distinction that my laptop is a whore-box that connects to different network"

Blog about... stuff (GDNet, WordPress): www.gamedev.net/blog/1882-the-cuboid-zone/, cuboidzone.wordpress.com/

Juliean

7,344

August 31, 2013 05:17 PM

Some thoughts:

- I'd rather add the ability to load most of this at least in form of a gfx resource loader on top of the current framework. Most of this code can be eliminated by loading all those resources from a file like this:


<Resources>
	<Textures path="Textures" >
		<Texture file="noise.gif" >Noise</Texture>
		<Texture file="reflection.gif" />Reflection</Texture>
	</Textures>
	<Materials path="Materials">
		<Material file="Gems\\gem_dualpass.cs" >GemMat</Material>
		<Material file="Sky\\sky_01.cs">SkyMat</Material>
	</Material>
</Resources

The same for meshes etc..., and cross-reference them by a given name. Choose whatever language you'd like, I've heard XML isn't such a hot thing anymore... I'd much rather edit such a file than manage resources directly in code as a user as much as possible, plus this integrates better in future tools and content processing. This brings me to my next point though, which is vital for this to work, namely:

- Implement a cache-system for your engine. Rather than like you do directly create a texture from a file when you need it, create it once, and then later reference it from there by a key (e.g. string). This will save load time, and valueable resource by only loading things once. Even if you don't want to have resource-files yet, this is still a good extention.

- Seperate loading from the classes themselfs. Seeing how you have a CreateTexture method etc on your texture class, makes things less flexible. I'd rather didn't have the texture class know how to load itself, but have a seperate class for loading. Ok, granted textures is not that good of an example, but especially the meshes. If you have all the load-methods on the mesh, whoever wants to implement a new form of loading a mesh (e.g. foreign file format), would need to edit the mesh class or hack some way around it. If you have a seperate loader, and the mesh constructor just take parameters needed for initialization, you have more freedom to that.

- Having a global (?) does-it-all engine-class is a matter of opinion, but I don't like the concept very much personally. From a personal point of view, I'd advice you against using it. I'd much rather have an EngineContext-class, that carries around only class instances you need (like the device, etc...), and have like mesh rendering implemented elsewhere. For example, you might at some point want to use an entity/component like system, which will implement a render loop, or at least link to it itself. Point is, the more "advanced" you go, the less need for a master-class you'll encounter. I don't know how much responsiblity your engine class really has, but from my own experience, I e.g. had a very similar system, where I'd register every mesh to a singleton Graphics3D-class. That class also would create textures, etc... I couldn't think about how things would work without it, but as soon as I implemented both an ECS and a render queue architecture, it became pretty obsolete, and I removed it ASAP.

Hope there is some helpful advice for you.

ongamex92

3,287

August 31, 2013 06:24 PM

CEToMedia - im not shure what that fuction does, but i think that the input parameter is transformed from local path to app global path. Wich is ugly.

//------------------------------------------------------------------

Possible leaks maybe?


CESDK::Texture2D* noise = new CESDK::Texture2D();


LPCSTR *all = new LPCSTR[6];
all[0] = CEToMedia("Textures\\Skybox\\01\\x+.bmp");

//------------------------------------------------------------------

That is really wrong.

// Push all the meshes

engine.Meshes.push_back(plane);

exposing direct access to containers that hold really important objects may cause you a big trouble.

You need some kind of manager (scene manager, or separate manager for the meshes(the one I'm using)) that will take care only for the mesh object.

When using that method you can provide diffrent loading methods some data sorting, ect ect...

Try with something like this.





//RequestMesh Desc: if mesh already exist an instance pointer is returned
//otherwise the mesh is loaded and later returned 
//note: for different situations you can provide different loading methods(like in another thread, or force relod)
//keep in mind that the MeshManager doesnt know how to pare the mesh files. His job is to call right fuction at the right moment on the right thread


const UMesh* pMyMesh = pMeshManager->RequestMesh("file.bla", REQUEST_MESH_FLAG1 | ... );
if(pMyMesh) ....;


pMeshManager->RequestMeshAsync("file.bla", REQUEST_MESH_FLAG1 | ... , someLambdaOrFuncOrWeverToCallWhenLoadingIsDoneOrNothing);


//and later maybe


enum EMESH_STATE 
{
EMESH_STATE_UNKNOWN = 0,
EMESH_STATE_LOADED,
EMESH_STATE_LOADING,
EMESH_STATE_LOADING_ASYNC,
EMESH_STATE_A_MAGIC_THING
};


const EMESH_STATE state = pMeshManager->GetStateOf("file.bla");//think of better name here

THe same manager is good for any kind of resources.

I'm not going to speak about your shaders. It is OK to use that kind of a structure. I personally prefer to expose my effect description away from the actual shader code:

This is what im using :

some_shaders.xml


<root>
  <!-- Test shaders -->
  <shader name="DiffuseVS" fileName="Diffuse.vs" entryPoint="main" target="SHADER_TYPE_VS" targetMajor="5" targetMinor="0">
    <!-- VERTEX SHADER INPUT ATTRUBUTES DESC -->
    <vsin name="position" semantic="SGE_VERT_POSITION_XYZ"/>
    
    <!-- SHADER INPUT UNIFORMS DESC -->


  </shader>


  <shader name="DiffusePS" fileName="Diffuse.ps" entryPoint="main" target="SHADER_TYPE_PS" targetMajor="5" targetMinor="0">
    <variable name="color" semantic="SGE_SEMANTIC_COLOR"/>
    <variable name="colorPower" semantic="SEMANTIC_UNKNOWN"/>
  </shader> 
</root>

later:

some_effects.xml


<root>
  <effect name="BasicEffect">
    <technique name="tech0">
      <pass>
        <vertexShader name="DiffuseVS"/>
        <pixelShader name="DiffusePS"/>
      </pass>    
    </technique>
  </effect>
</root>

and in C++ code


pShaderManager->AddShadersLibrary("some_shaders.xml", ....);


pEffectManager->SetSHaderManager(pShaderManager);
pEffectManager->AddEffectsLibrary("some_effects.xml", .....);


EffectBase* pEffect = GetEffectManager()->GetEffect("BasicEffect");

But your approch is fine too.

Excuse me for my awful english.

Migi0027

4,632

Author

August 31, 2013 10:17 PM

Thanks, really appreciate the input!

FastCall22: "I want to make the distinction that my laptop is a whore-box that connects to different network"

Blog about... stuff (GDNet, WordPress): www.gamedev.net/blog/1882-the-cuboid-zone/, cuboidzone.wordpress.com/

Tasaq

1,530

August 31, 2013 10:56 PM

Some thoughts:

- I'd rather add the ability to load most of this at least in form of a gfx resource loader on top of the current framework. Most of this code can be eliminated by loading all those resources from a file like this:
<Resources>
	<Textures path="Textures" >
		<Texture file="noise.gif" >Noise</Texture>
		<Texture file="reflection.gif" />Reflection</Texture>
	</Textures>
	<Materials path="Materials">
		<Material file="Gems\\gem_dualpass.cs" >GemMat</Material>
		<Material file="Sky\\sky_01.cs">SkyMat</Material>
	</Material>
</Resources

I double that. I did it on my last engine and I was really satisfied with the results.

Engine Design - Improvements

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Engine Design - Improvements

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines