[2.1] Iterator bug caused by latest UAV branch merge

[al2950]

On lines 229, 319 & 376 in OgreCompositorPass.cpp a find is done on the resourceLayout map, however no check are done after to see if the entry was found. Is this a case of just adding the following line in the appropriate place

Code: Select all

 ResourceLayoutMap::iterator currentLayout = resourcesLayout.find( mTarget );
 if (currentLayout != resourcesLayout.end())

Or is there a bigger issue here where the resourcesLayout maps are not being generated properly?

**EDIT**
Apologise I have not got up to speed with the UAV branch changes yet

[dark_sylinc]

Or is there a bigger issue here where the resourcesLayout maps are not being generated properly?

I can't take a look now, but that is likely the case. They should always exists, their initial state is often "Undefined".

In OgreCompositorWorkspace.cpp:

Code: Select all

resourcesLayout[mRenderWindow.target] = ResourceLayout::RenderTarget;
CompositorNode::fillResourcesLayout( resourcesLayout, mGlobalTextures,
									 ResourceLayout::Undefined );

Sets the layouts of all global textures (+ final render window) to undefined.

And then in OgreCompositorNode.cpp:

Code: Select all

fillResourcesLayout( resourcesLayout, mLocalTextures, ResourceLayout::Undefined );

Sets the layouts of all local textures from each node to undefined.
Then a little bit later _placeBarriersAndEmulateUavExecution for each pass is called.

So, the question is why are you getting a seemingly impossible scenario

[al2950]

Ok, thanks for the info, ill take a look on Sunday.

[al2950]

Ok I have had a look at it and the problem is due to the way cubemaps are dealt with. I however due not understand enough about UAVs and how the barriers are meant to work to fix.

The compositorPass stores a pointer its render target in mRenderTarget and it uses that point to look up the resource in resourcesLayout in this line;

Code: Select all

            //Check <anything> -> RT
            ResourceLayoutMap::iterator currentLayout = resourcesLayout.find( mTarget );
            if( (currentLayout->second != ResourceLayout::RenderTarget && explicitApi) ||
                currentLayout->second == ResourceLayout::Uav )
            {
                addResourceTransition( currentLayout,
                                       ResourceLayout::RenderTarget,
                                       ReadBarrier::RenderTarget );
            }

The problem is mRenderTarget is different for each face of a cubemap, however resourceLayout only contains a single entry to the parent cubemap render target. So how should it be fixed, should resourcesLayout contain an entry for each face of a cubemap, or should CompositorPass::mRenderTarget point to the top level cubemap render target instead of each face??

[dark_sylinc]

I take the code that is causing this issue is very similar to the old cubemapping sample from the 2.0 samples?

I'm almost sure there should be one RTT entry for each cube face. I need to debug it since these barriers need to be thought carefully.

Small remarks (for better understanding):
Traditional RTTs are in order. That means the following sequence:

• Write red to RTT A
• Sample from RTT A, write the value to RTT B
• Write yellow to RTT A

At the end of rendering, B = red, A = yellow.

However when using UAVs (using UAV semantics, UAVs can also be used as RTT and then they become in order, assuming the barriers when transitioning from UAV to RTT are correct):

• Write red to UAV A
• Sample from UAV A, write the value to RTT B
• Write yellow to UAV A

The the end of rendering, B could contain red, could contain yellow, or could contain some pixels red and some pixels yellow. 'A' may end up containing red instead of yellow although that's unlikely. Good old race conditions.
That's why UAV barriers are needed for: We need to place a barrier between step 1 and 2 to ensure writes to A finish before we start reading from it, and another barrier between 2 and 3 to guarantee we don't start writing to A before we've done reading from it.

Additionally, in D3D12 and Vulkan we need to tell the API if a resource is going to be transitioned (from being a Texture to being an RTT, from being an RTT to being a Texture, from being a RTT to being... whatever, e.g. a vertex buffer, since D3D12 & Vulkan are very flexible and allow to reinterpret_cast memory); the logic is very similar.

That's all in theory, very easy to understand. In practice it is incredibly difficult to get right (and you learn to appreciate what D3D11 and OpenGL drivers where doing behind your back) because it's all about race conditions and parallel programming. And parallel programming is hard.

The easy way is to issue a barrier every time you detect you're about to transition (e.g. move the logic to setTexture, setRenderTarget, setUav, etc). But if you do that, you're adding all the overhead, throwing away every possible advantage from using D3D12 & Vulkan, you're better off using D3D11 drivers. So we bake the barriers, evaluating the hazards during workspace initialization (also GPUs like that, when possible, we put all barriers together instead of putting unrelated commands in the middle).

[al2950]

I take the code that is causing this issue is very similar to the old cubemapping sample from the 2.0 samples?

Yes that is a correct assumption

I need to debug it since these barriers need to be thought carefully.
).

Ok, and thanks for the additional info.

[dark_sylinc]

Fixed. It was just like you said. We simply weren't adding all the possible RTTs.

Sidenote: Current texture design is getting increasingly annoying.

[al2950]

Cheers

Sidenote: Current texture design is getting increasingly annoying.

I wonder how many times you will say that before you go, sod it, ill just re-write it!