Depth sharing Design

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.

Depth sharing Design

Postby dark_sylinc » Thu Nov 05, 2009 8:01 pm

Updates
This thread is becoming large, so first a quick summary of latest changes:

TODO (besides polishing and testing):
* Implement DepthSharing base system (DONE)
* Implement D3D9 & OGL FBO systems (DONE)
* Compositor script tokens for setting the pool ID (should be easy) (DONE)
* D3D10 Render System (if not too different from D3D9, also easy. Must get Vista or 7 machine w/ DX10 HW) (DONE)
* Write documentation for the compositor manual (DONE)
* Check multiple monitor support for D3D9 isn't broken (DONE)
* Check OGL support in Linux isn't broken (DONE)
* Check OGL support in Mac isn't broken (I don't have a Mac)
* Write wiki example showing how to use it in C++ code

Downloads:
Patch from SourceForge.Net
Patch from a mirror.
D3D10 & D3D11 patch from mirror.
Apply this patch against Rev 9725

Simple test to verify it works here.

Note about D3D10/11 patch:
The D3D10/11 patch also fixes very relevant issues:
Code: Select all
* Going to fullscreen could cause to switch to a different resolution due to wrong refresh rate parameter. Symptoms varied from wrong resolutions, stretched images, to crashes.
* FSAA was not working at all. The code was all screwed. Fixed.
* Depth buffers weren't being released, causing occasional crashes when switching from fullscreen to windowed mode.
* Depth buffers weren't being shared at all (very different behavior from D3D9 & OGL) Fixed with the new system.


Other RenderSystems:
For those render systems that aren't using this new system yet (OGL ES), they won't compile because _createDepthBufferFor is pure virtual. Just overload it and return null to workaround this problem.

Original thread:
Hi all!

Following recent issues regarding depth sharing problems, I thought it would be ideal to start a forum thread where the design of the depth sharing could be discussed to solve the root problem for once and for all.
Right now we're patching over patches, and we're just losing consistency between OpenGL & Direct3D implementations.

I come from a D3D background, so please feel free to criticize my approach if you feel it's going to cause trouble with OGL.

My proposal is that DepthBuffers should be able to attach to RenderTargets.
DepthBuffers would be RenderSystem agnostic, and have to be derived by the RenderSystem implementation to store the actual depth buffer (i.e. in D3D9 the IDirect3DSurface9)

DepthBuffers will have 4 boolean flags:
Code: Select all
bool mShareable
bool mForRenderWindow
bool mForRTT
bool mForShadowTexture


When mShareable is true, this DepthBuffer will be stored in a pool in the RenderSystem (the base class, not the API-specific) so it is available for automatic sharing in any RenderTarget that didn't have a manual DepthBuffer specified. This would mimic the current behavior.
When this value is false, it can only be used when the user manually assigns this DepthBuffer to a RenderTarget.

* When mForRenderWindow is true, this depth buffer can be used with the RenderWindow (commonly, the backbuffer).
* When mForRTT is true, this depth buffer can be used with any Render Texture Target (excluding the render window). I'm having the compositor here in thought, and covers the MRTs too.
* When mForShadowTexture is true, this buffer can be used with a RTT that is used for shadowing.

When any of the 3 last values are false, that DepthBuffer would fail to attach to it's corresponding RenderTarget. This ensures that the DepthBuffer is not accidentally used where it shouldn't.

By default, the main depth buffer that is created (if created) with the Render Window should have all of it's flags set to true. This behavior could be overridden by the application.

Note: It doesn't have to be 4 boolean values, an 8-bit value and a simple AND bitwise operation ought to be enough.

Compositor Script:
I thought it would be great to see this from a high level perspective, and what's better than seeing this in Compositor script code. This is how a depth buffer could be created and handled in a compositor:

Code: Select all
compositor DeferredShading/GBuffer
{
   technique
   {
      // temporary textures
      texture myRTT target_width target_height PF_R8G8B8A8
      depth_buffer myDepth <myRTT|target_output> <target_width|#width> <target_height|#height> <default|#msaa> <default|D24S8|D24X8|D16> <Shareable true|false> <RWindow true|false> <RTT true|false> <ShadowTexture true|false>
      
      target myRTT
      {
         input none

         depth_buffer <myDepth|target_depth_buffer>
         shadows_depth_buffer <myDepth|target_depth_buffer>

         pass clear
         {
         }

         pass render_scene
         {
         }
      }
   }
}


depth_buffer
Declares a new depth buffer (Mr obvious).
Parameters:

<myRTT|target_output|none>
Specifies an existing RTT (or render window) from which this depth buffer will be based upon as a reference. (width, height, bit depth, msaa setting, etc). These values can be overridden in the optional parameters that follows.
Default: target_output

<target_width|#width> <target_height|#height>
Specifies the width & height for this depth buffer. The same values as the "texture" keyword should be accepted. (i.e. target_height_scaled, etc). target_width & co. should be based from "<myRTT|target_output>" parameter specified at the beginning, it is invalid when the reference RTT is 'none'
Default: target_width target_height

<default|#msaa>
Specifies the MSAA setting for this depth buffer based on the RTT reference, or manually controlling the value. The number of msaa samples has to be explicitly specified if the RTT ref. is 'none'
Default: default

<default|D24S8|D24X8|D16>
The depth buffer format. I'm just putting it for maximum flexibility, but I don't know why someone would like something other than default (which lets Ogre decide).
The format has to be explicitly specified if the RTT ref. is 'none'
Default: default

Shareable
See mForRenderWindow above.
Default: true

RWindow
See mForRenderWindow above.
Default: true

RTT
See mForRTT above.
Default: true

ShadowTexture
See mForShadowTexture above.
Default: true

depth_buffer <myDepth|target_depth_buffer>
Specified inside the target. It can be either the depth buffer that the output has, or a manually specified one.
Default target_depth_buffer

shadows_depth_buffer <myDepth|target_depth_buffer>
Same as depth_buffer


As you can see, the following compositor would cause a fatal exception (or a log error) but it would be completely valid when parsing:
Code: Select all
compositor DeferredShading/GBuffer
{
   technique
   {
      // temporary textures
      texture myRTT target_width target_height PF_R8G8B8A8
      depth_buffer myDepth myRTT target_width target_height target_msaa false false false false
      
      target myRTT
      {
         input none

         depth_buffer myDepth         //Error mForRTT is false
         shadows_depth_buffer myDepth   //Error mForShadowTexture is false

         pass render_scene
         {
         }
      }
   }
}



DepthBuffer class as a derived class from Texture
I'm leaving this for discussion:
The DepthBuffer class should ideally derive from Ogre::Texture (or similar) for the APIs that support the depth buffer as a texture.
OpenGL & Direct3D10 support this, Direct3D9 requieres a hack (which is different in ATI & NVIDIA drivers) in order to work. May be another boolean flag could be specified to tell that we want this depth_buffer to be used as a texture too.
I'm not very experienced with HW depth textures so I'm leaving the feedback for someone else who wants to join.

I'll be waiting for feedback.
What do you all think?
Cheers
Dark Sylinc


Edit: Ooops, forgot how the class should look somehow (big overview):

Code: Select all
class DepthBuffer //Derived classes skipped for now
{

//Note no Direct3DSurface9 here, or similar

int width
int height

DBFormat format; //Assuming this a valid enumeration of bit depth

bool mShareable
bool mForRenderWindow
bool mForRTT
bool mForShadowTexture

public:
virtual void baseTexture( RenderTarget *renderTarget ) ; //RenderTarget to base our depth from
virtual bool isCompatible( RenderTarget *renderTarget ) = 0; //Must be implemented by the API specific code
}
Last edited by dark_sylinc on Tue Feb 23, 2010 4:09 am, edited 11 times in total.

For this message the author dark_sylinc has received kudos
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby Noman » Fri Nov 06, 2009 2:09 pm

This is a natural discussion after the last SoC, I think I have some input on this one, mainly questions...

A. DirectX has tight coupling between depth and stencil buffers. OpenGL has looser coupling. Should Ogre address them as one or as separate entities?
B. I'm not sure if all of those flags are really needed by the depth buffer. Perhaps a more general pooling mechanism would do the trick. Why should a depth buffer care if its for a render window, RTT or shadow texture? It just needs to know its pixel format and dimensions, and thats it IMO. Why should a certain depth texture be only for RTTs?
C. Should depth textures be declarative or just request based? Do we really need
depth_buffer myDepth <myRTT|target_output> <target_width|#width> <target_height|#height> <default|#msaa> <default|D24S8|D24X8|D16> <Shareable true|false> <RWindow true|false> <RTT true|false> <ShadowTexture true|false>

type declarations, or perhaps something like
own_depth_buffer true

will be enough to do the trick?

I'm not sure what I think yet, I'd just like to hear opinions on these matters (like use cases where the simple solutions aren't good enough).

Of course one of the things we definitely need to do is to sync the rendersystems with each other, no matter what we decide...
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
 
Posts: 714
Kudos: 2
Joined: 31 Jan 2005
Location: Israel

Re: Depth sharing Design

Postby sinbad » Fri Nov 06, 2009 3:28 pm

An important thing to be aware of is that OpenGL has only allowed sharing of depth buffers since the Frame Buffer Object (FBO) extension. Anyone using RTTs on a device that doesn't support FBO will never be able to share depth buffers between targets. This is precisely why this has never been a first-party, top-level structure in OGRE because GL did not universally support it. It's also why the GL rendersystem lags here, all of GL's separate RTT extensions have been a total PITA for years.

I also don't like the 'mForRenderWindow' et al flags, this is too specific. If you wanted to have specific control over what depth buffers can be used for what beyond just dedicated / shared (and I'm not necessarily seeing the use case for that yet), a much better approach would simply be too have a 'pool number'. Pool 0 could be the default, any other pool above that is custom. Buffers are added to the pool they are created with, and returned to that when done. RenderTargets could have a pool assigned to them to so they know which one to look in. That's far more generic. You could have a pooling flag for the depth buffer just like the pool flag for the texture in the compositor system, except that it defaults to true (& pool number 0).

Allowing a user to request a depth buffer of a specific size & format is currently pointless. Most rendersystems will only allow you to create depth / stencil buffers that are tied to the same width/height and even bit depth of the colour surface they are attached to. Thus don't even bother trying to create a 32-bit colour surface with a 16-bit depth surface, D3D & GL will at best just ignore you and give you a matching surface, or at worst blow up in your face. The flexibility just is not there in practice so it's not worth overcomplicating this.

For this message the author sinbad has received kudos
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby dark_sylinc » Fri Nov 06, 2009 4:20 pm

Noman wrote:A. DirectX has tight coupling between depth and stencil buffers. OpenGL has looser coupling. Should Ogre address them as one or as separate entities?

Nice one. I'm really clueless about OpenGL.
Nevertheless, I think at HW level the depth & stencil buffers are still the same buffer ("24 bits for depth, 8 for stencil" says it all.....) but they definitely should be handled in abstract way. Whether Ogre should emulate Ogl's behavior when using Direct3D or emulating D3D's behavior when running in OpenGL I have no idea (like we do with the Right-handed coordinate system in D3D)

Noman wrote:B. I'm not sure if all of those flags are really needed by the depth buffer. Perhaps a more general pooling mechanism would do the trick. Why should a depth buffer care if its for a render window, RTT or shadow texture? It just needs to know its pixel format and dimensions, and thats it IMO. Why should a certain depth texture be only for RTTs?

My reasoning behind those flags was that the current code handles everything automatically. And in some way, we don't want to lose all that automation because there's a lot of user base which doesn't care about depth & stencil buffers, it is too low level for them.
However there are advanced users which want a very tight control over it's usage to achieve certain effects and performance optimizations. If they allow a depth buffer to enter the sharing pool, it may end up being used for shadow textures as well.
This directly conflicts with some implementations because the shadow map pass happens exactly after each compositor's pass. Therefore when we go to the next pass and we want to reuse the depth buffer, it turns out it has garbage (this is what was happening to you, Norman)

Of course, a flag like "own_depth_buffer_shadow true" could be enough, but such flag looks like Ogre should create it's own depth buffer instead of using one for the pool. When writing a compositor, the user may not care about the shadow map depth buffer, he may say "ok, pull a depth buffer from the pool, I don't care as long as you don't use THIS SPECIFIC depth buffer I'm using (and re-using) for the RTTs"

Noman wrote:C. Should depth textures be declarative or just request based? Do we really need
type declarations, or perhaps something like
own_depth_buffer true

will be enough to do the trick?

No.

This goes against re using a depth buffer. If each RTT uses it's own depth buffer, the user may not be able to reuse it freely in multiple passes. In fact, he may have 2 or 3 different depth buffers which uses & reuses in multiple passes and mix the results, which require that each depth buffer is used specifically used in some of those passes.

An examples of this is Deferred shading front & back object collision culling using the depth & stencil buffer.
Another example I went at some time long ago was that I was doing Localized object motion blur & I could have reused the Depth buffer for the objects that have to be blurred, but instead I had to render the whole scene again.

Also, why I put all those flags?
a) Buffers can be put in a pool to be shared, but ensure that it isn't used in some render passes which may conflict with the current render pass
b) Ensure a buffer isn't "accidentally" used where the developer didn't expect. Remember there is still automatic control over the buffers, which may do for the developer something he doesn't want.
c) Remember we may have multiple depth buffers because of different resolution & msaa settings. On some cases the developer may specifically request a specific MSAA setting to achieve certain quality control, in which he wants to override the default setting. Or perhaps enabling MSAA may cause multiple depth buffers to be in the pool, mainly because some RTTs (like MRT in many GPUs) don't support MSAA at all, therefore causing lots of depth buffers to wander about. Having flags gives hint to the automatic process on when to use them.

Don't forget many of those options are optional, sometimes the user just wants to create a depth buffer for the RTT, so something like this:
Code: Select all
depth_buffer myDepth myRTT

May just work. Most probably the shareable & co flags should be moved to the front because they're most likely to be the most used ones (and less used variables, like msaa setting should be last).

Though, at a code level, all those settings should be part of the DepthBuffer class.
Then RenderTarget::attachDepthBuffer( myDepthBuffer ) would be called, which also checks they're compatible:

Code: Select all
bool RenderTarget::attachDepthBuffer( DepthBuffer *myDepthBuffer )
{
   bool result = myDepthBuffer->isCompatible( this );
   if( result )
   {
       mDepthBuffer = myDepthBuffer;
   }
   else
   {
      //Error/warning/whatever
   }

   return result;
}


Cheers
Dark Sylinc

Oh: phpBB3 is informing me about sinbad's post while I was writing it. Just read it.
I like his pool # idea better.
As for requesting the specific format, yeah I agree there are so many limitations that it may be pointless in a compositor script, but it could still be in the code as a way of handling it. Perhaps not by trying to comply with it's bit depth, but rather than trying to find a surface that best matches (the D16 parameter would be a 'hint' when calling the function)

As for the FBO limitation we should get over it. There are lots of similar OGL limitations (last time I run an MRT, OpenGL failed miserably because it didn't like mixing floating point buffers with integer buffers, even of the same bit-depth, where the hell did this limitation came from??? Last time I checked, this was even possible when first introduced in the Geforce 5 FX series) also, if the user is using RTTs and custom depth buffers, it could probably mean he's aiming the latest cards, which should already support it. I may be wrong though. I'm not that familiar with OGL
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby Noman » Mon Nov 09, 2009 4:26 pm

I'm voting for the pool number option as well. I thought about proposing a more stack-like solution so that you won't have to be aware of everything thats going on at the same time (ie who's using pool #2?), but in practice you will be. The extra flexibility that the direct numbering approach gives is probably worth it.

The question is, what scope will this pool identifier be in? Viewport? RenderTarget? SceneManager? My current vote goes for viewport, as it will allow to easily switch them in compositors. Perhaps we'll need to expose some access to the shadow RTT viewports if we don't expose enough already, but thats easily achievable.

As for the rendersystem support, I think we have to start treating this as a 'first party' feature, because it is becoming important. Even if GL only supports it through FBO, it is very likely to be supported in modern machines (which are probably the ones that these techniques will run on) as I think FBO is the modern & most widely used RTT method these days. (We can check this against other GL renderers these days). RenderSystemCapabilities can report different capabilities based on the options they were created with, and composition techniques can report themselves as not supported if they use this flag when the rendersystem can't handle it.
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
 
Posts: 714
Kudos: 2
Joined: 31 Jan 2005
Location: Israel

Re: Depth sharing Design

Postby sinbad » Tue Nov 10, 2009 6:45 pm

Noman wrote:The question is, what scope will this pool identifier be in? Viewport? RenderTarget? SceneManager? My current vote goes for viewport, as it will allow to easily switch them in compositors. Perhaps we'll need to expose some access to the shadow RTT viewports if we don't expose enough already, but thats easily achievable.


I think it needs to be on RenderTarget. There's no point putting it on Viewport because the back buffers etc are all shared between all Viewports on the same target so they couldn't have separate options anyway .

As for the rendersystem support, I think we have to start treating this as a 'first party' feature, because it is becoming important. Even if GL only supports it through FBO, it is very likely to be supported in modern machines (which are probably the ones that these techniques will run on) as I think FBO is the modern & most widely used RTT method these days. (We can check this against other GL renderers these days). RenderSystemCapabilities can report different capabilities based on the options they were created with, and composition techniques can report themselves as not supported if they use this flag when the rendersystem can't handle it.


Yes, agreed.
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby dark_sylinc » Fri Nov 13, 2009 5:39 pm

sinbad wrote:
Noman wrote:The question is, what scope will this pool identifier be in? Viewport? RenderTarget? SceneManager? My current vote goes for viewport, as it will allow to easily switch them in compositors. Perhaps we'll need to expose some access to the shadow RTT viewports if we don't expose enough already, but thats easily achievable.


I think it needs to be on RenderTarget. There's no point putting it on Viewport because the back buffers etc are all shared between all Viewports on the same target so they couldn't have separate options anyway.

I agree with sinbad, from a technical point of view, the queue should be in the RenderTarget and it's the RenderTarget who should arrange the DepthBuffers in the same category from each pool entry. This way we also have the chance to manage everything that is API-specific in each plugin. If someone want a specific behavior, they can put one depth buffer per pool entry and problem solved. (Like we do with RenderQueues: if I want a specific draw order, I carefully chose what goes in which RenderQueue #)

Though, perhaps there should be a bridge function between Viewports & RenderSystem in case the Compositor doesn't have access to the render system (which I doubt) so it asks through the Viewport.

Shadow maps are handled from SceneManager, so if someone wants to put a separate DepthBuffer pool # for the shadow maps (so that depth buffers used in shadow maps don't interfere with the scene's one) we should call something like this:

Code: Select all
SceneManager::setShadowTextureDephPool( size_t shadowIndex, size_t poolId );


Last but not least, (IMO) the pool ID should be stored in each RenderTarget class.

This way we also even avoid to manually create depth buffers (both code-wise and in compositor scripts).
In the compositor script, when defining the RTT, an optional value (probably at the end) can be used to specify the pool #.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby Noman » Sun Nov 15, 2009 8:16 am

Reasonable ideas, RenderTarget scope seems alright.
No need for a Viewport<->RenderSystem bridge, the SceneManager can make the pool modifications when setViewport is called, according to the viewport's RenderTarget's pool identifier.

The SceneManager shadow API makes sense too. I thought about exposing the shadow texture / shadow viewport and changing the pool ID through that, but this is also a solution.

I think that the high level design is nearing completion, so here's a question about the implementation - how do we want to implement this? Currently, all depth-buffer related handling is completely inside the rendersystems. We can continue changing them to behave the same way, but perhaps a general approach should be used :
  • Depth buffer creation / destruction calls will be added to RenderSystem
  • Depth buffer binding abilities will be added to RenderTarget (tho, this can be in RenderSystem as well, not sure)
  • This way, we can implement the pooling mechanism once in Ogre's core, and all depth-creating rendersystems will support this automatically. Will be easier to sync behaviors.

What do you think?
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
 
Posts: 714
Kudos: 2
Joined: 31 Jan 2005
Location: Israel

Re: Depth sharing Design

Postby sinbad » Sun Nov 15, 2009 2:51 pm

Implementation-wise, again you have to bear in mind that not all rendersystems can create separable depth/stencil buffers - so GL pre-FBO, GLES 1.1 etc. Therefore it's important that separate depth/stencil buffers not be mandatory, they must be able to be included implicitly in a target. Provided that's the case and that the create/pool/bind/destroy methods are optional in RenderSystem, it sounds fine.
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby dark_sylinc » Tue Nov 17, 2009 7:39 pm

@Noman:
You read my mind.

sinbad wrote:Implementation-wise, again you have to bear in mind that not all rendersystems can create separable depth/stencil buffers - so GL pre-FBO, GLES 1.1 etc. Therefore it's important that separate depth/stencil buffers not be mandatory, they must be able to be included implicitly in a target. Provided that's the case and that the create/pool/bind/destroy methods are optional in RenderSystem, it sounds fine.

I was thinking that create/pool/bind/destroy methods would be mandatory, but when no separable depth/stencil buffers is available (case of GL pre-FBO), the RenderTarget is forced to 'attach' (as in Ogre code, for that old API there wouldn't be a concept of attaching) to the main depth buffer, which will be the only one in the pool.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby sinbad » Wed Nov 18, 2009 6:43 pm

Well, my point is that there simply won't be a correct fallback for those operations so requiring them is somewhat pointless - essentially the depth/stencil is 'attached' by the simple act of setting the active render target. If there's some internal management to do then ok, but implement that in the base RenderSystem and re-use it everywhere if that's the case.
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby iloseall » Tue Dec 08, 2009 3:51 am

I like this very mush.
And my project need this feature to do 3d ui render and without bloom on ui.

Is it perhaps go into 1.7 :D ?
User avatar
iloseall
Gremlin
 
Posts: 160
Kudos: 0
Joined: 14 Sep 2003
Location: Beijing China

Re: Depth sharing Design

Postby sinbad » Wed Dec 09, 2009 5:23 pm

We'll probably save this for 1.8 since we're trying to stablise / finalise for 1.7 now.
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby dark_sylinc » Sat Dec 12, 2009 9:05 pm

Ok, this is a WIP for the curious ones. Download the preview patch

Apply this patch against rev 9402 and add the new files (OgreMain/src/OgreDepthBuffer.cpp & OgreMain/include/OgreDepthBuffer.h) to the Solution file.

Note after applying this patch GL & D3D10 rendersystems won't compile (pure virtual function not overloaded)
I'm taking care of GL now (which is causing me some trouble, as I'm not used to it).

I've removed the depth-buffer-specific code in _pauseFrame & _resumeFrame that Noman wrote, because it isn't needed anymore.
Noman can make his Deferred Shading test work by calling setShadowTextureConfig( idx, width, height, 2 ) where the last value (2) indicates the shadow map should retrieve depth buffers from a different pool, therefore not interfering with the standard rendering process when pausing.

So far so good, everything works as expected. No crashes, no leaks. Even when the device is lost.
Nevertheless, some hard-cases will have to be tried (dual-monitors, multiple D3D devices, conflicting cards, etc) to see if it still work as it should.
I'll be plugin my CRT monitor downstairs in a few days to test for the first time dual monitors, and debug it for myself (though, I've got yet to see if I have a spare DVI-VGA adapter)

What's left are GL & D3D10 rendersystems, plus being able to select the pool ID in the compositor script.
But this is mainly how the base would work.

Edit: The work in the patch is one (almost complete) day of work
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby Noman » Sun Dec 13, 2009 11:16 am

At first glance, it looks good! Nice job!

A few notes :
1) The biggest thing that looks missing is capability testing - I don't see a modification of RenderSystemCapabilities, nor fallback code that says (to the user / logs) "I can't do this" if it isn't supported. Do all DX9 devices support backbuffer<->depth buffer decoupling? If they do, then only OpenGL will need this handling. But it still needs to be there as part of Ogre's API.
2) Should some things be a bit 'lazier' ? IE RenderTarget checking that the poolId is different in setDepthBufferPool before detaching its depth buffer.
3) I think that if DepthBuffer inherits from RenderSysAlloc, its allocation and destruction should be with the OGRE_XXX memory macros.(Not 100% if the rendersystems need to do this as well)
4) A minor technical thing - it's easier if a patch does everything for you - would be nice if the CMakeLists.txt would be updated as well, and maybe even modify the deferred shading demo. That way people can apply the patch, rebuild the library and start seeing it in action instantly.

Full testing will wait until the work is complete of course, but we need to remember to test capability fallbacks / reporting between different render systems (GL/DX), render system configurations (GL RTT technique - only FBO supports this) and different hardware (especially older stuff)

I'm not a big GL expert but I do have some experience with it from the SoC project, so if you have some questions there ask and I might be able to help.

Keep up the good work!
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
 
Posts: 714
Kudos: 2
Joined: 31 Jan 2005
Location: Israel

Re: Depth sharing Design

Postby dark_sylinc » Sun Dec 13, 2009 5:13 pm

Noman wrote:At first glance, it looks good! Nice job!

Thanks

Noman wrote:A few notes :
1) The biggest thing that looks missing is capability testing - I don't see a modification of RenderSystemCapabilities, nor fallback code that says (to the user / logs) "I can't do this" if it isn't supported. Do all DX9 devices support backbuffer<->depth buffer decoupling? If they do, then only OpenGL will need this handling. But it still needs to be there as part of Ogre's API.

I've been looking into documentation and it seems all DX9-class HW supports decoupling. Furthermore IIRC, this is even supported way back in DX7. Backbuffers have been always decoupled from depth buffers as far as I can remember. I haven't found documentation stating the contrary.
I haven't thought of setting a bit in RenderSystemCapabilities (or logging a warning). All I thought was that on those systems where 1 depth buffer is allowed (pre-FBO OGL) the same DepthBuffer would be returned for all requests, failing to attach when the RTT has bigger resolution than the depth buffer's.
I'll take in mind adding a log warning, but a RenderSystemCapability flag? I'm not sure...

Noman wrote:2) Should some things be a bit 'lazier' ? IE RenderTarget checking that the poolId is different in setDepthBufferPool before detaching its depth buffer.

You mean if( oldId != newId ) detach(); ?
Good point. I've already changed the code.

Noman wrote:3) I think that if DepthBuffer inherits from RenderSysAlloc, its allocation and destruction should be with the OGRE_XXX memory macros.(Not 100% if the rendersystems need to do this as well)

I was unsure. Right now it doesn't inherit from it.
I saw a couple of classes were new'ed with the regular "new" operator in D3D9RenderSystem, so I just skipped OGRE_NEW. I can easily change that.

Noman wrote:4) A minor technical thing - it's easier if a patch does everything for you - would be nice if the CMakeLists.txt would be updated as well, and maybe even modify the deferred shading demo. That way people can apply the patch, rebuild the library and start seeing it in action instantly.

Ahhh, so CMakeLists.txt is the magic file eh?
Thank you, I've been wondering how to add new files with CMake (I've only started using CMake when Ogre switched)

Noman wrote:render system configurations (GL RTT technique - only FBO supports this) and different hardware (especially older stuff)

I'm still analyzing what's OGL doing regarding it. I'm starting to understand how depth buffers assignments work. If it works like I think it does, it shouldn't be much of a problem.

So far I could see that GLRenderSystem delegates DepthBuffer assignment to FBOManager; and the other 2 managers don't handle depthbuffer.
The managers are used from RenderTarget, which has 3 different derived classes, depending on the method (FBO, Pbuffer, copy) so only the FBO RT requests depthbuffers to FBOManager.

It's a bit hard to follow the GLRenderSystem code as it doesn't fully comply the Ogre src guidelines (for instance, there are like 3 different classes defined in the same file, which doesn't comply with 1 class -> 1 cpp file).

One scary thing though, is that it seems (not sure) FBOs don't support using the main framebuffer's depth buffer. If they don't, that would cause inconsistencies with D3D9.
Furthermore, it seems OGL 2.0 FBO (not sure about 3.0) requires the depth buffer to be of the exact width and height as the RT (oposed to D3D9, which should be equal or less)

Does anybody know something regarding this?

Edit: Just found this and this. Regarding width & height, OGL 3.0 works like D3D9 while OGL 2.0 w & h must be of exact same size.
As for using the main depth buffer, OGL 2.0 doesn't allow it, and OGL 3.0 is unknown.
I hate the ARB......
Ok then there's nothing to avoid the inconsistency. We can only reduce the gap. Perhaps a flag in RenderSystem that prevents D3D9 from using the main depth buffer (actually prevent any 'manual' DepthBuffer from being added to the pool) so that it behaves like OGL for those who need consistency?
Also good documentation should be written, so that any inconsistency is well defined and understood by other developers.

Now I'm considering adding the RenderSystemCapabilities flags:
Code: Select all
//Supports a separate depth buffer for RTT. D3D 9 & 10, OGL with FBO (RSC_FBO implies this flag)
RSC_RTT_SEPARATE_DEPTHBUFFER
//Supports using the main depth buffer for RTT. D3D 9 & 10, OGL FBO support unknown (pbuffer and copy ought to support it?)
RSC_RTT_MAIN_DEPTHBUFFER_ATTACHABLE
//Supports attaching a depth buffer to an RTT that has width & height less or equal than RTT. Otherwise must be of exact same resolution. D3D 9 & 10, OGL 3.0
RSC_RTT_DEPTHBUFFER_RESOLUTION_LESSEQUAL


Noman wrote:I'm not a big GL expert but I do have some experience with it from the SoC project, so if you have some questions there ask and I might be able to help.

Thanks, neither am I. The only reason I'm familiar with GL is because I've used Glide2x thoroughly in the old days, which is very similar.
I'm reading docs and websites to catch up with new stuff (FBO...) so I can understand well how to do things and how not do them before I screw up.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby dark_sylinc » Mon Dec 14, 2009 5:43 pm

One think I've realized is that, like Sinbad said, the problem lies in that the APIs don't handle this depth sharing consistently.

So, the idea now is that RenderSystems which have the proposed RSC flags (RSC_RTT_SEPARATE_DEPTHBUFFER & Co.) should behave identically.
That way, RS are consistent, as long as they have the same RSC bits set. And if they don't, the behavior in all cases is known beforehand by the developers.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby sinbad » Tue Dec 15, 2009 5:27 pm

I think this is the right way to go. Sorry I haven't been active in this thread, have quite a lot on my plate right now :) But as a 1.8 direction this looks solid.
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby Noman » Wed Dec 16, 2009 3:56 pm

Right

Consistency should be handled by fallback techniques - if a compositor needs to explicitly share depth buffers between two render targets that the render system can't handle, the compositor technique needs to report itself as not supported and fall back to a different implementation of the technique that does the same thing without this feature.

We need to remember that this feature has two uses - one being opening up new rendering possibilities, the other being performance improvements (mainly saving GPU memory). When something is not supported, can we isolate which one of the two uses it doesn't support? If so, we can report not supported if the first case happens, and just warn if the second case happens.
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
 
Posts: 714
Kudos: 2
Joined: 31 Jan 2005
Location: Israel

Re: Depth sharing Design

Postby sinbad » Wed Dec 16, 2009 5:23 pm

Personally, I would not even try to fall back at the lowest level of this, and expect that the user (or higher-level OGRE code) only tries to use this functionality if it is available, by checking the caps. Otherwise we'll be in a position of trying to second-guess why the depth sharing is required, and I don't think we can reliably do that. The only fallbacks should be if/when OGRE uses this option itself for a particular feature. In the compositor system for example, trying to use this option when the feature is not available should mark the compositor technique as unsupported, and it's up to the content creator to provide an alternate technique which does it a different way. If a user tries to use it at the rendersystem level, it should just fail with an error telling them they can't use it.
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Re: Depth sharing Design

Postby dark_sylinc » Tue Dec 29, 2009 7:09 pm

Hi all!

Sorry for the long silence. I'm doing this in my free time. Work + Study + Personal stuff + OGRE. Sometimes I wish I could clone myself to do multiple stuff at once.

The GL FBO method is almost done. I'm now dealing with some issues regarding memory management (D3D9, the pool is flushed in a device lost, fullscreen switch; in GL the pool grows and buffers stay there forever until shutdown)

I'm having some doubts regarding pbuffers. My NVIDIA drivers don't support them, and since documentation is scarce, it's even harder for me to get them right; so this is how I think OGRE uses them:

* A pbuffer is created when RTT
* The GL context is switched, do the usual rendering.
* Blit the contents from the pbuffer to a regular texture

When multiple RTTs are being used, only one or maybe two pbuffers and multiple regular textures are used.
When an RTT is bigger than the current pbuffer's dimensions, the old one is destroyed and a bigger one is created. Is that correct?

So, in other words, pbuffers work pretty much like Copy method, with the main difference that a different GL context is used, and it overcomes the limitation of RTT resolutions can't be bigger than the render window's. Am I right?

Cheers
Dark Sylinc
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby dark_sylinc » Tue Dec 29, 2009 11:37 pm

OKeeeeyyyy....... I think I've might hit with a different bug (while fixing the "DepthBuffers stay forever in GL plugin" problem, which is, by the way, already fixed)

In Root::~Root(), shutdown() is called; which sets the variable "mIsInitialised" to false.

A bit later, (still in the destructor), unloadPlugins() is called. This one calls dllStopPlugin(). In OgreGLEngineDll.cpp it calls Root::getSingleton().uninstallPlugin(plugin); (as so do many other plugins)

Root::getSingleton().uninstallPlugin(plugin) looks for the iterator holding the plugin in a list, and does the following:

Code: Select all
if (mIsInitialised)
   plugin->shutdown();
plugin->uninstall();


plugin->shutdown() is never called because mIsInitialised was already set to false.
This hasn't shown up as a problem yet because the plugins we have shutdown on uninstall anyway.
I guess the "initalized" flag is there to avoid shutting down when unregistering a plugin which hasn't even been loaded yet; not to avoid shutting down when deleting Root.
uninstallPlugin() only works as desired when calling without deleting Root.

The best way to see this is to place a breakpoint at Root::~Root and single-step from there.
Relevant functions:
Root::~Root
Root::shutdown() (sets mIsInitialised to false)
MyPluginDll::dllStopPlugin() (calls uninstallPlugin)
Root::uninstallPlugin() (doesn't call shutdown because mIsInitialised is false)


The best way I see, to fix this without breaking anything (we can't just set mIsInitialised to true, other code may depend on this) is to add a new boolean "mIsShuttingDown" or similar, so that the code looks like this:

Code: Select all
if (mIsInitialised || mIsShuttingDown)
   plugin->shutdown();
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby Noman » Wed Dec 30, 2009 6:04 am

About the GL notes - we originally said that the only GL RTT mode that will share depth buffers will be FBO.
Its the most common and modern one, so its alright if its the only one that does so, as the techniques that use depth sharing are likely to run on hardware that supports it anyway.

As for the root/plugin thing - doesn't sound related to depth sharing but it might be a general bug in ogre, I'll have a look.
User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
 
Posts: 714
Kudos: 2
Joined: 31 Jan 2005
Location: Israel

Re: Depth sharing Design

Postby dark_sylinc » Wed Dec 30, 2009 7:31 pm

Thanks Noman for the fast answer!

Here's a WIP patch with OGL FBO & D3D9 support. Also updates CMake project files. It doesn't update your Deferred Shading demo yet. I'll upload later a new one with the changes for your demo.

The patch has to be applied to Rev 9402.
I'll check out svn in these days since probably it's out of sync and will probably conflict with the SVN revert from a week ago.

Your words about the FBO OGL was a relief. I'm certainly against putting a lot of hard work into something that is deprecated and well known drivers (NVIDIA) don't even support (pbuffers).
Though RenderSystem::setDepthBufferFor() & ::_createDepthBufferFor may receive non-FBO targets (so far, the most common example is receiving the main render window) so the functions have to handle those non-FBO cases correctly.

Things left to do (besides polishing and testing):
* Compositor script tokens for setting the pool ID (should be easy)
* D3D10 Render System (if not too different from D3D9, also easy. Must get Vista or 7 machine w/ DX10 HW)
* Write documentation for the compositor manual, and a wiki example showing how to use it in C++ code
* Check OGL support in Linux isn't broken (I don't have a Mac)

Edit: Patch updated. Download it again and apply against Rev 9515
Edit2: Patch updated. Download it again and apply against Rev 9615
Last edited by dark_sylinc on Mon Jan 18, 2010 2:18 am, edited 2 times in total.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
 
Posts: 787
Kudos: 160
Joined: 21 Jul 2007
Location: Buenos Aires, Argentina

Re: Depth sharing Design

Postby sinbad » Mon Jan 04, 2010 3:21 pm

Thanks for this. I'm currently somewhat burned out from the 1.6.5 / 1.7.0 RC1 treadmill but once I'm recharged will have a look. Can we move this to the patch list now with a link back here so it doesn't get lost?
User avatar
sinbad
OGRE Founder (Retired)
OGRE Founder (Retired)
 
Posts: 25870
Kudos: 63
Joined: 06 Oct 2002
Location: Guernsey, Channel Islands

Next

Return to Developer talk

Who is online

Users browsing this forum: No registered users and 4 guests