Geometry shaders merged into 1.6 branch

Discussion area about developing or extending OGRE, adding plugins for it or building applications on it. No newbie questions please, use the Help forum for that.
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 714
Joined: Mon Jan 31, 2005 7:21 pm
Location: Israel
x 2

Geometry shaders merged into 1.6 branch

Post by Noman »

Hi all,

I wanted to let all of you know that I merged the Geometry shader GSoC code base into the 1.6 branch.

This means that in the next stable releases, geometry shaders and render to vertex buffer will be supported in ogre!

However, there are some things that still need to be done
- Build environments. I used VS 2005 for my development, so the VC8 solution and projects are ready. There are still VC7.1, linux and Mac OSX build environments that need to be updated. I currently don't have access to any of them. Build system maintainers - can you please do this?
- Documentation. The code is properly documented, but the Ogre manual doesn't contain any information about the new features. I will do this in the coming days.
- Minor bugs. There are two problems that I know of.
1) Script parsing problem with unsupported ASM shaders. The following material definition will crash Ogre when loaded with the D3D9 rendersystem :

Code: Select all

geometry_program Ogre/GPTest/Swizzle_GP_ASM asm
{
	source Swizzle.gp
	syntax nvgp4
}

material Ogre/GPTest/SwizzleASM
{
	technique
	{
		pass SwizzleASMPass
		{
			geometry_program_ref Ogre/GPTest/Swizzle_GP_ASM
			{
			
			}
		}
	}
}
When Ogre parses the assembly program definition (OgreScriptTranslator.cpp line 3602) and the syntax is not supported, the program object will not be created and will not be registered with the resource manager. So, when the script parses the program reference (OgreScriptTranslator.cpp line 2143) the setGeometryProgram call will throw an exception, which will eventually crash ogre (in the sample framework).
I think the expected behaviour should be that the material should not have any unsupported techniques. For this to happen, the program object needs to be created, but to flag itself as not supported.
In order to work around this, I currently did not add the ASM version of the swizzle test (the material that I attached to this post), as it is an internal playpen test, and GLSL and CG are enough (and the script compiler behaves differently with them, so they don't have this issue).

2) GL render system has some internal errors. I added some flushing glGetError calls so that the log won't have an additional invalid operation record each frame. I intend to research into this a bit and solve it (hopefully).

So, for all of you that use the 1.6 branch, please contact me in this thread or directly (PMs are fine) if you updated and are having problems. For general questions about the project, please use the GSoC project thread

Thanks!
User avatar
Praetor
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 3335
Joined: Tue Jun 21, 2005 8:26 pm
Location: Rochester, New York, US
x 3

Post by Praetor »

What should the plan of action be to remove the unsupported issue before 1.6 release?
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 714
Joined: Mon Jan 31, 2005 7:21 pm
Location: Israel
x 2

Post by Noman »

I tested this small fix and it works :
If you create a GPU program using the create (rather than CreateProgram) function, the source code is not supplied and loading will fail and it will say that it is not supported. This should be the fallback when unknown ASM syntax is encountered.

Code: Select all

Index: OgreScriptTranslator.cpp
===================================================================
--- OgreScriptTranslator.cpp	(revision 7974)
+++ OgreScriptTranslator.cpp	(working copy)
@@ -3602,6 +3602,8 @@
 		if (!GpuProgramManager::getSingleton().isSyntaxSupported(syntax))
 		{
 			compiler->addError(ScriptCompiler::CE_UNSUPPORTEDBYRENDERSYSTEM, obj->file, obj->line);
+			GpuProgramPtr unsupportedProg = GpuProgramManager::getSingleton().create(obj->name, 
+					compiler->getResourceGroup(), translateIDToGpuProgramType(obj->id), syntax);
 			return;
 		}
Apply this patch in the OgreMain source directory and check it out.
The "Correct" solution is probably to create a subclass of GpuProgram called "UnsupportedGpuProgram", and add a method that creates it to the GpuProgramManager (so that the resource gets registered).

I have general commit access (so that I could do the 1.6 merge), but I don't want to change code that isn't in my domain... I'm not sure that this solution would work. The hacky one (the one that I posted the patch of) works for me... Should we use it as a temporary solution?
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 66

Post by sinbad »

If it resolves the issue, go ahead and commit it - you can test by temporarily removing support for the given syntax code or something?

Unfortunately the code doesn't build on gcc, I'm looking at it now. The initial problem is that there is no in-built hash function for uint64, which is used in the GLSLLinkProgramManager to detect unique combinations of vertex, fragment and geometry program. For the moment I'm reverting it just to a std::map instead of a HashMap, which is what it was before.
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 66

Post by sinbad »

gcc build is working again, should fix Linux / Mac problems.
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 714
Joined: Mon Jan 31, 2005 7:21 pm
Location: Israel
x 2

Post by Noman »

Wow. Cross-platform compatibility is a bitch. Wouldn't it be possible to add a hash functor for 64bit uint?
By the way - I copy/pasted the 64 bit uint from another class, because Ogre doesn't define a standard one. Shouldn't we do that?

As for the bug, I committed the change (added a comment explaining why I did it), and added the ASM materials. The playpen GS test now uses it (and works under OpenGL), and the D3D9 render system works properly (in other demos).
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 66

Post by sinbad »

Noman wrote:Wow. Cross-platform compatibility is a bitch. Wouldn't it be possible to add a hash functor for 64bit uint?
Yep, I was just pushed for time. However since you'd be compacting a 64-bit int down to a size_t it's possible you'd have hash collisions anyway and would have to design a hash function specifically for the distribution style. To be honest, I don't think it's going to be a major bottleneck, the map isn't going to be that deep and it's just 2 32-bit comparisons per node.
By the way - I copy/pasted the 64 bit uint from another class, because Ogre doesn't define a standard one. Shouldn't we do that?
Ah, I thought we already had but I see it's only in OgreImageResampler.h. I'll promote it to OgrePlatform.h.