[apologies if you are getting this twice, I fat fingered the first attempt]
On 08/21/2013 06:49 PM, Carsten Neumann wrote:
The easy fix is to not do the optimisation in
MaterialGroup::renderEnter, that will fix the bug at the cost of more
nodes being cull tested.
I later realized that this would completely disable culling, so that's
not so good :-/
To make this behave somewhat reliable we'd need to:
- Actions call the leave function even on a Action::Skip result
- all existing enter functions need to be modified to perform their
"state changing" even if all children are culled
I'm attaching a (untested) patch that implements that, but since it
makes changes to what callbacks are invoked during traversal there is
potential for ugly fallout [2]
Even with these changes MultiCore is not fully reliable, because it does
not call the contained cores' leave functions in reverse order - and of
course if not all cores were entered it should only leave those that were...
Hmm, I suppose that means my understanding of what MultiCore is for is
completely wrong [1]... or the thing is a bit buggy ;(
@Gerrit: Can you give me hint how MultiCore is intended to work?
While looking for other Group derived cores that might suffer from the
same problem, I also found what looks like a bug in
ChunkOverrideGroup::renderEnter: This function installs the override
chunks and then does a return Group::renderEnter() - meaning if all
children are culled the overrides are installed, but never removed
because the corresponding renderLeave() is never done :(
Cheers,
Carsten
[1] I *thought* it was meant to be the same as a linear sequence of
Nodes with the cores that the MultiCore stores.
[2] It also does not yet fix problems in MaterialChunkOverrideGroup with
respect to Action::Skip handling.
diff --git a/Source/System/Action/Base/OSGAction.cpp b/Source/System/Action/Base/OSGAction.cpp
index b3dfa3c..6197df9 100644
--- a/Source/System/Action/Base/OSGAction.cpp
+++ b/Source/System/Action/Base/OSGAction.cpp
@@ -326,7 +326,6 @@ Action::ResultE Action::recurse(Node * const node)
_actParent = node;
_newList.clear();
-
_useNewList = false;
if(_nodeEnterCB != NULL)
@@ -345,44 +344,47 @@ Action::ResultE Action::recurse(Node * const node)
_actNode = node;
_actParent = node;
- if(result != Continue)
- {
- if(result == Skip)
- return Continue;
-
- return result;
- }
-
- if(! _newList.empty())
- {
- result = callNewList();
- }
- else if(! _useNewList) // new list is empty, but not used?
+ if(result == Continue)
{
- Node::MFChildrenType::const_iterator cIt =
- node->getMFChildren()->begin();
- Node::MFChildrenType::const_iterator cEnd =
- node->getMFChildren()->end ();
+ // visit children
- for(; cIt != cEnd; ++cIt)
+ if(!_newList.empty())
{
- result = recurse(*cIt);
+ result = callNewList();
+ }
+ else if(!_useNewList)
+ {
+ // do not use the list, visit all children
+ Node::MFChildrenType::const_iterator cIt =
+ node->getMFChildren()->begin();
+ Node::MFChildrenType::const_iterator cEnd =
+ node->getMFChildren()->end ();
- if(result != Continue)
- break;
+ for(; cIt != cEnd; ++cIt)
+ {
+ result = recurse(*cIt);
+
+ if(result != Continue)
+ break;
+ }
}
}
_actNode = node;
_actParent = node;
- if(result == Continue)
- {
- result = callLeave(node->getCore());
- }
- else
+ if(result != Quit)
{
- callLeave(node->getCore());
+ // leave node
+
+ if(result == Continue)
+ {
+ result = callLeave(core);
+ }
+ else
+ {
+ callLeave(core);
+ }
}
_actNode = node;
@@ -392,7 +394,7 @@ Action::ResultE Action::recurse(Node * const node)
_nodeLeaveCB(node, this);
if(result == Skip)
- return Continue;
+ result = Continue;
return result;
}
@@ -430,7 +432,6 @@ Action::ResultE Action::recurseNoCallback(Node * const node)
{
Node::MFChildrenType::const_iterator cIt =
node->getMFChildren()->begin();
-
Node::MFChildrenType::const_iterator cEnd =
node->getMFChildren()->end ();
@@ -447,7 +448,7 @@ Action::ResultE Action::recurseNoCallback(Node * const node)
_actParent = node;
if(result == Skip)
- return Continue;
+ result = Continue;
return result;
}
diff --git a/Source/System/NodeCores/Groups/Base/OSGChunkOverrideGroup.cpp b/Source/System/NodeCores/Groups/Base/OSGChunkOverrideGroup.cpp
index 14286eb..e7f11bd 100644
--- a/Source/System/NodeCores/Groups/Base/OSGChunkOverrideGroup.cpp
+++ b/Source/System/NodeCores/Groups/Base/OSGChunkOverrideGroup.cpp
@@ -225,7 +225,7 @@ Action::ResultE ChunkOverrideGroup::renderEnter(Action *action)
{
if(*chIt != NULL && (*chIt)->getIgnore() == false)
pAction->addOverride(uiSlot, *chIt);
-
+
++uiSlot;
++chIt;
}
@@ -244,7 +244,6 @@ Action::ResultE ChunkOverrideGroup::renderLeave(Action *action)
pAction->popState();
}
-
return Inherited::renderLeave(action);
}
diff --git a/Source/System/NodeCores/Groups/Base/OSGGroup.cpp b/Source/System/NodeCores/Groups/Base/OSGGroup.cpp
index c3a5636..5cfd809 100644
--- a/Source/System/NodeCores/Groups/Base/OSGGroup.cpp
+++ b/Source/System/NodeCores/Groups/Base/OSGGroup.cpp
@@ -103,7 +103,6 @@ Action::ResultE Group::renderEnter(Action *action)
{
if(ra->selectVisibles() == 0)
{
- ra->popVisibility();
return Action::Skip;
}
}
@@ -116,7 +115,7 @@ Action::ResultE Group::renderLeave(Action *action)
RenderAction *ra = dynamic_cast<RenderAction *>(action);
ra->popVisibility();
-
+
return Action::Continue;
}
diff --git a/Source/System/NodeCores/Groups/Base/OSGLight.cpp b/Source/System/NodeCores/Groups/Base/OSGLight.cpp
index 02e5f36..ea88658 100644
--- a/Source/System/NodeCores/Groups/Base/OSGLight.cpp
+++ b/Source/System/NodeCores/Groups/Base/OSGLight.cpp
@@ -184,7 +184,6 @@ Action::ResultE Light::renderEnter(LightEngine::LightTypeE eType,
{
if(action->selectVisibles() == 0)
{
- action->popVisibility();
return Action::Skip;
}
}
diff --git a/Source/System/NodeCores/Groups/Base/OSGMaterialGroup.cpp b/Source/System/NodeCores/Groups/Base/OSGMaterialGroup.cpp
index 643fa35..aecbb83 100644
--- a/Source/System/NodeCores/Groups/Base/OSGMaterialGroup.cpp
+++ b/Source/System/NodeCores/Groups/Base/OSGMaterialGroup.cpp
@@ -98,13 +98,6 @@ Action::ResultE MaterialGroup::renderEnter(Action *action)
RenderAction *pAction =
dynamic_cast<RenderAction *>(action);
- Action::ResultE r = Group::renderEnter(action);
-
- // ok all children are culled away so we leave
- // immediately and don't set the material!
- if(r == Action::Skip)
- return r;
-
if(pAction != NULL &&
this->getMaterial() != NULL )
{
@@ -112,7 +105,7 @@ Action::ResultE MaterialGroup::renderEnter(Action *action)
pAction->getActNode());
}
- return r;
+ return Group::renderEnter(action);
}
Action::ResultE MaterialGroup::renderLeave(Action *action)
diff --git a/Source/System/NodeCores/Groups/Base/OSGRootGroup.cpp b/Source/System/NodeCores/Groups/Base/OSGRootGroup.cpp
index 6d911f4..c0c3fb6 100644
--- a/Source/System/NodeCores/Groups/Base/OSGRootGroup.cpp
+++ b/Source/System/NodeCores/Groups/Base/OSGRootGroup.cpp
@@ -103,7 +103,6 @@ Action::ResultE RootGroup::renderEnter(Action *action)
{
if(ra->selectVisibles() == 0)
{
- ra->popVisibility();
return Action::Skip;
}
}
diff --git a/Source/System/NodeCores/Groups/Misc/OSGSwitch.cpp b/Source/System/NodeCores/Groups/Misc/OSGSwitch.cpp
index e09436b..f3436bb 100644
--- a/Source/System/NodeCores/Groups/Misc/OSGSwitch.cpp
+++ b/Source/System/NodeCores/Groups/Misc/OSGSwitch.cpp
@@ -113,7 +113,6 @@ Action::ResultE Switch::renderEnter(Action *action)
}
else
{
- ra->popVisibility();
returnValue = Action::Skip;
}
}
@@ -121,13 +120,11 @@ Action::ResultE Switch::renderEnter(Action *action)
{
if(ra->selectVisibles() == 0)
{
- ra->popVisibility();
returnValue = Action::Skip;
}
}
else
{
- ra->popVisibility();
returnValue = Action::Skip;
}
}
------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and
AppDynamics. Performance Central is your source for news, insights,
analysis and resources for efficient Application Performance Management.
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Opensg-users mailing list
Opensg-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensg-users
