Post-processing effects test (SSAO/HDR/Bloom)

[myconid]

Although I did noticed that disabling the 'Enable Water Reflections' option in the menu causes the black terrain and entities to show regardless of whether the water is in view or not.

Thanks, that was quite helpful! I think the reflections rendering was interfering with the blending mode (so the LOS, meaning the on-screen rendering of what the units see, wasn't the problem in the end).

The solution is to set the blending in Renderer.cpp. Right before the line:

RenderModels(context);

add this:

glEnable(GL_BLEND);
glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);

Hope this fixes it!

[halcyonXIII]

Unfortunately it didn't fix the problem. With that change the terrain/entities are always black, just like if I had turned 'Enable Water Reflections' off.

I saw something else whilst testing. When I rotate the view so that a black entity is in front of some grass, that entity's texture gets blended onto the grass. This happens with and without the changes to Renderer.cpp.

[myconid]

Unfortunately it didn't fix the problem. With that change the terrain/entities are always black, just like if I had turned 'Enable Water Reflections' off.

I saw something else whilst testing. When I rotate the view so that a black entity is in front of some grass, that entity's texture gets blended onto the grass. This happens with and without the changes to Renderer.cpp.

I might have done that backwards. Instead of that previous change, use:

glDisable(GL_BLEND);

There have also been some fixes in the shaders since the last patch and they might be related. If this doesn't fix the problem, then the mistake is/was in the shaders.

[halcyonXIII]

I might have done that backwards. Instead of that previous change, use:

glDisable(GL_BLEND);

This has fixed it, thanks! And I think that's all of them ^^

[myconid]

This has fixed it, thanks! And I think that's all of them ^^

Thanks for testing!

A quick update about what I'm thinking about right now:

How do we implement more advanced texturing..? The key is to let the xml files define how many textures need to be loaded for each actor, instead of hardwiring that into the code. For instance, the engine normally supports a single texture per object, while the normal/specular maps bumped that up to 3.

What I'm considering is to have <sampler> entries in the material xml files that tell the engine how many textures are needed and what their names are in the shaders. e.g.

<sampler name="baseTex"/>
<sampler name="normTex"/>
<sampler name="AOTex"/>

tells the engine that the material/attached shader needs 3 textures, one being the diffuse texture, another being the normal map and the third the precomputed AO texture.

The actor files can then define the actual texture files, and that will not require changes to the existing actor files if the <texture> element is used for this task. e.g.

<texture>base.png</texture>
<texture>norm.png</texture>
<texture>ao.png</texture>

The correspondence to the material entries is given by the order they appear in.

Edited June 10, 2012 by myconid

[zoot]

Letting the correspondence depend on ordering seems a bit dirty - I'd clear that with the team first Sounds interesting nonetheless.

[historic_bruno]

Yeah - don't depend on order, it's not just a bit dirty, that would be a filthy hack There's no reason why we can't change how actors are defined, we're still in alpha. Most artists don't actually mess with the actor XML, they use the actor editor GUI instead.

Why not add a name/id attribute to the actor <texture>s that matches the ones in the material <sampler>s? Explicit naming and behavior is nice, if we do add such an attribute it shouldn't be optional, we can update all our actors and actor editor to comply - it's easy since they only reference one type of texture currently. I think good names would be like "base" (default), "normal", "specular", "ambient" (I was thinking "diffuse" instead of "base" but that might technically include AO too). The best part of that is the type of texture map is independent of both XML element ordering and filename.

[myconid]

Yeah - don't depend on order, it's not just a bit dirty, that would be a filthy hack There's no reason why we can't change how actors are defined, we're still in alpha. Most artists don't actually mess with the actor XML, they use the actor editor GUI instead.

Why not add a name/id attribute to the actor <texture>s that matches the ones in the material <sampler>s? Explicit naming and behavior is nice, if we do add such an attribute it shouldn't be optional, we can update all our actors and actor editor to comply - it's easy since they only reference one type of texture currently. I think good names would be like "base" (default), "normal", "specular", "ambient" (I was thinking "diffuse" instead of "base" but that might technically include AO too). The best part of that is the type of texture map is independent of both XML element ordering and filename.

My thought was to make it like a "procedure call", meaning the material defines an interface and the actor provides the arguments. On second thought that's a really stupid idea.

How about a system similar to the one you're suggesting, where the absence of the attribute is assumed to mean "base"? That way the actors still don't need to be updated, but the semantics are clear enough.

Edit: Or, I can create an entirely new element type that doesn't conflict with the existing <texture> element. <texture> can be kept unchanged but deprecated and replaced in time.

<samplers>
<sampler name="baseTex" file="base.png"/>
<sampler name="normalTex" file="normal.png"/>
</samplers>

Edited June 10, 2012 by myconid

[myconid]

http://trac.wildfiregames.com/ticket/1493

So, there it is.

[historic_bruno]

Edit: Or, I can create an entirely new element type that doesn't conflict with the existing <texture> element. <texture> can be kept unchanged but deprecated and replaced in time.

<samplers>
<sampler name="baseTex" file="base.png"/>
<sampler name="normalTex" file="normal.png"/>
</samplers>

Why deprecate it, why not replace it now? It's not like we have hundreds of people working on actors, maybe a handful of artists and if they get a parse error while testing it, they'll complain to us, then we can tell them how to fix it (We don't have to worry about third party mods either, they should expect breakages of old data files as the engine and gameplay progress, and there's not many around yet anyway.)

We should use whatever structure and names makes the most sense. I think <sampler> sounds more obscure and weird than <texture> and has more to do with the internal implementation than what defines an actor, so it wouldn't be my preference.

[Mythos_Ruler]

Why can't it be something like this?

<Textures>
   <Diffuse></Diffuse>
   <Normal></Normal>
   <Parallax></Parallax>
</Textures>

Make things as intuitive as possible.

[myconid]

Why deprecate it, why not replace it now? It's not like we have hundreds of people working on actors, maybe a handful of artists and if they get a parse error while testing it, they'll complain to us, then we can tell them how to fix it (We don't have to worry about third party mods either, they should expect breakages of old data files as the engine and gameplay progress, and there's not many around yet anyway.)

We should use whatever structure and names makes the most sense. I think <sampler> sounds more obscure and weird than <texture> and has more to do with the internal implementation than what defines an actor, so it wouldn't be my preference.

Well, the difference between deprecating and removing is about 5 lines of code, so it's not like there's code duplication or anything. What I'm concerned about is that if we remove it completely we'll have to modify all the actors before we can do anything else. There are a lot of actors...!

Why can't it be something like this?

<Textures>
   <Diffuse></Diffuse>
   <Normal></Normal>
   <Parallax></Parallax>
</Textures>

Make things as intuitive as possible.

Unfortunately that format isn't flexible enough. I can easily change it to <Textures><Texture name="diffuse" file="x.png"/></Textures> format if you guys think it's more readable.

[zoot]

Why can't it be something like this?

I expect because it clutters the namespace for tags. Imagine someone came up with a graphics technique called "actor maps". You would then need to add a tag to your list of the type "<Actor>". But that tag is already used for something else in the engine, so the parser would not know what the intended meaning is. In short, it's better to be specific from the beginning.

[historic_bruno]

Well, the difference between deprecating and removing is about 5 lines of code, so it's not like there's code duplication or anything. What I'm concerned about is that if we remove it completely we'll have to modify all the actors before we can do anything else. There are a lot of actors...!

Very easy for sed to handle or even Notepad++

[myconid]

Very easy for sed to handle or even Notepad++

If those can do the conversion and things work out smoothly, I don't care at all about the deprecation... I'm that lazy, hehe!

[Mythos_Ruler]

I expect because it clutters the namespace for tags. Imagine someone came up with a graphics technique called "actor maps". You would then need to add a tag to your list of the type "<Actor>". But that tag is already used for something else in the engine, so the parser would not know what the intended meaning is. In short, it's better to be specific from the beginning.

I don't follow. The tag could be <Actormaps> or whatever.

[myconid]

I don't follow. The tag could be <Actormaps> or whatever.

<textures><texture name="xxx" path="yyy"/></texture>

The xml parser needs to know the element and attribute names at compile time, but not the values. (Ok, this is not universally true, but it's how it's set up in the engine).

If someone writes a GLSL shader we haven't thought about, he might require textures whose names we didn't anticipate during development, so he'll need to modify the engine to add them or be forced to use inappropriate texture names in his shader.

Edited June 11, 2012 by myconid

[zoot]

I don't follow. The tag could be <Actormaps> or whatever.

And who will get the blame when people begin complaining about the inconsistency of having to affix some things with "maps" and others not? The programmer.

[historic_bruno]

Think of it as being roughly the same as actor animations. We don't hardcode every possible animation name (<IdleAnimation>, <WalkAnimation>, etc.) , instead they are an attribute of the animation tag so that adding a new animation is very easy. It makes sense to handle textures the same way if only for consistency:

  <animations>
	<animation file="biped/animation.dae" name="Idle" speed="50"/>
	...
  </animations>
   ...
  <textures>
	<texture file="skeletal/texture.png" name="base"/>
	...
  </textures>

[Mythos_Ruler]

And who will get the blame when people begin complaining about the inconsistency of having to affix some things with "maps" and others not? The programmer.

Still not following. We'd just not name the tags in a confusing manner...?

Think of it as being roughly the same as actor animations. We don't hardcode every possible animation name (<IdleAnimation>, <WalkAnimation>, etc.) , instead they are an attribute of the animation tag so that adding a new animation is very easy. It makes sense to handle textures the same way if only for consistency:

  <animations>
	<animation file="biped/animation.dae" name="Idle" speed="50"/>
	...
  </animations>
   ...
  <textures>
	<texture file="skeletal/texture.png" name="base"/>
	...
  </textures>

Yeah, this makes sense.

[zoot]

Still not following. We'd just not name the tags in a confusing manner...?

I guess that is a matter of "easier said than done".

[myconid]

I had some free time earlier today (finally!), so quite a few things got done.

• I made the changes that historic_bruno and Mythos_Ruler wanted for the texture loading: http://trac.wildfire...com/ticket/1493
  
• I added support for multiple UV sets per model, so lightmaps are now possible: http://trac.wildfire...com/ticket/1497
  
• I wrote a simple test that depends on both the above, which shows off precomputed AO.
  

Here's what the last one looks like, if you're interested. The necessary files are attached below, but recall that the two patches linked above are also needed for it to work.

offlineAO.zip

[Wijitmaker]

That is down right awesome, fine work! I knew you could do it

[k776]

Again, nice work! :-) Do you have a list of what is left before you can start cleaning it up for review and including into the game? Think I said this before, but would be nice to have these in Alpha 11.

[plumo]

If it is not too much trouble, would it be possible to put a 'vanilla' screenshot next to a screenshot with your work? Just to put things in perspective.

Also: I'm sure this would be perfect material to put on moddb