Can a node have two parents?


#2

Yes, a node can have multiple parents. I think using node().addChild(…) will not remove the node from its previous parent, but check the list of parents to be sure.

Of course, this results in the node being visited more than once during rendering, if both cells are being rendered, causing it to be rendered twice. This is how instancing works.

It also means there are two possible NodePaths to the node, which will give different properties depending on the properties assigned to the parent.


#3

I see–thank you.

Hmm… The possibly-variable properties shouldn’t be a major issue in my case, I think. There are some potential problems, but they might be solved in the design of the levels, I imagine.

Is there a way around the “double rendering” issue? If not, is there another potential solution to the issue of objects straddling cell-borders?


#4

I had an idea regarding this: How feasible would it be for me to add a new flag to NodePaths/Nodes (the latter, I imagine, but accessible through the former) that indicates whether a Node may be multiply-rendered, with a guard added during scene-graph traversal that skips a node if it has been rendered before?

Something like this, in pseudocode:

Node:

bool mayRenderMoreThanOnce;
bool hasBeenRenderedThisPass;

In the scene-graph code:

// While traversing the graph...
if (currentNode.mayRenderMoreThanOnce || !currentNode.hasBeenRenderedThisPass)
 {
  currentNode.hasBeenRenderedThisPass = true;
  // Render the node as usual...
 }

Of course, this would presumably call for some means of clearing the “hasBeenRenderedThisPass” flag… Perhaps

I’m not currently using instancing, so there’s no conflict there. (In any case, the “mayRenderMoreThanOnce” flag could simply be set for instanced nodes.)

Thoughts?


#5

Instead of a boolean, you could make it a frame number: if the frame number matches the current frame being rendered, you know it’s already been rendered, if it’s less, it still needs to be rendered. That prevents you from having to traverse all the nodes to clear it after each rendering.

If you have to render the scene multiple times from multiple points of view (eg. with shadows or reflections), a frame index wouldn’t do, and you would instead have to assign a global number that you increase every time Panda is about to traverse the scene for rendering.

As for instancing, that does break with this approach. It is not easy to determine the value of mayRenderMoreThanOnce, but the way to determine it would be to iterate the graph upwards until you find a node with more than one parent. However that means that in your use case it would return true, since that’s exactly what you’re telling Panda to do. I think what you really want is to prevent nodes from rendering multiple times with the same transform and state.

So what you could do is, instead of storing a “mayRenderMoreThanOnce” flag, store the accumulated transform and render state of that node in a variable when the traversal gets to it. Then the next time the traverser encounters it, it can check whether it matches the state of the last time you rendered that object, and if so, skip it.

A completely different approach to doing all this would be after traversal, by adding code to the relevant CullBin* classes to filter out duplicate CullableObject entries from the lists during sorting. Alternatively, one could change the CullBin*::add_object calls to check whether the object already exists with these parameters before adding it to the list of objects to draw.

This does add additional overhead, of course. However since CullBinStateSorted is the most commonly used bin, and it already sorts the results, it might be simply a matter of checking in draw() whether the object matches the properties of the previous one, and skip it if that’s the case. Or, switching it to an inherently sorted and uniquifying container like pset or ov_set.


#6

A frame number is a good idea, I think. :slight_smile:

However, I am rendering shadows, so as you say, that perhaps won’t work. But see below…

As I said above, I’m not using instancing, so I’m happy to leave it broken for this project. If a later project calls for it, I can perhaps revisit the problem–presuming that subsequent changes to the engine between here and there don’t obviate doing so, of course.

Something along these lines occurred to me after I posted–although I didn’t at the time know enough to have much more than an inchoate concept. I suspect that this may actually be a better idea.

What about this: use a boolean, as mentioned above. While traversing, set the “hasBeenRenderedThisPass” flag to “false”–this is the step at which it’s cleared. In CullBin*–“addObject” looks like a good spot, intuitively–the flag is checked. If it’s “false”, the object is added, and the flag is set to “true”. Otherwise, skip the object and move on.

You mentioned rendering the scene multiple times; when doing so, is the scene graph traversed multiple times? I would imagine so, as different views would result in different culls. If so, and I’m not much mistaken, then this version may well work with multiple renders, as each subsequent traversal clears the flag. If not, it remains an issue…

What do you think?


#7

Possibly, but this would have to be on the Geom then, not on the PandaNode, since the original PandaNode isn’t stored in the cull process. And it can’t be in CullBin*–addObject, because that is called during the cull traversal, so it has to be called during sort or draw.

This isn’t necessary though, because instead of a frame index, you could simply use a “pass index”; a number you increment for each cull pass you do.


#8

Hmm… That makes sense, I think. It wouldn’t call for clearing, either; indeed, even having the number eventually overflow would be fine, as we would only be checking for equality.

So, does the following rough code look good to you?

In Node

static int globalPassCounter;

int passCounter;

Node()
 {
  passCounter = globalPassCounter;
 }

In the traversal code:

// Before traversal
Node::globalPassCounter++;

// ...
// During traversal, presumably when considering whether to add the node to the bins
if (currentNode.passCounter != Node::globalPassCounter)
 {
  // Add node as per usual
  currentNode.passCounter = Node::globalPassCounter;
 }

#9

That would be the general idea, yes.

You could probably put this code in CullTraverser::traverse_below(), since that’s where it analyzes whether the node should be rendered (and calls node->add_for_draw(this, data)) and whether its children should be visited. You can just check whether the counter matches, and if so, return early from that function.


#10

Excellent, and thanks. :slight_smile:

Okay, once I’ve decided whether I’m moving to 1.10 or attempting to stick with 1.9.4 (as per the “Heap Size” thread), I’ll likely take a shot at this. I’ll hopefully update this thread once I’ve done so and tested the result!


#11

I do want to point out, in case it wasn’t yet clear, that such a change could not make it into the main Panda codebase since it would break quite a bit of Panda’s design, particularly wrt pipelined rendering and instancing.


#12

Don’t worry, I do recognise that. It’s intended for my own purposes, where instancing isn’t called for.

Could you expand on the point regarding pipelined rendering, please? Being a little unfamiliar with that, I would like to check that this change won’t break anything that I am using…


#13

ie. setting threading-model. There are additional considerations that would need to be taken into account if multiple threads are doing rendering.


#14

Ah, I see–thank you. So, as long as I’m not setting the “threading-model” parameter to something–and thus leaving it in the default state–I should be fine?


#15

Yes.


#16

Excellent, and thank you. :slight_smile:


#17

I’ve started working on this, and I’ve hit a snag. (For the moment I’m just implementing the multiple-parenting; if that works well, I may decide to leave aside the proposed rendering alterations.)

“PandaNode.addChild(…)” seems to work in that the child-object is indeed rendered more than once. However, the object’s position doesn’t seem to be as expected. Whether I leave the position unaltered or attempt to get a NodePath by calling “NodePath.find(…)” from the new parent-node, I seem to end up with one or another of the rendered objects being misplaced.

Here is what I have thus far:

            # "np" = the NodePath of the object to be parented
            # "node" is "np.node()"
            # "pos" is "np.getPos(render)"
            #
            # Omitted for brevity: building the list of cells
            #  to which to parent the object

            if len(cellList) > 0:
                np.wrtReparentTo(cellList[0].geometryRoot)
                for cell in cellList[1:]:
                    cell.geometryRoot.node().addChild(np.node())
                    newNP = cell.geometryRoot.find("**/" + np.getName())
                    newNP.setPos(render, pos)

How might I get around this? Should I perhaps use actual instancing instead, under the assumption that this would build better NodePaths and thus better allow for positioning?


#18

Remember, a NodePath is a particular path to follow from the root to a node, and there are now two ways to reach it your node now that it has two parents. You are using ‘find’, and you are getting only one of the NodePaths pointing to that node. Furthermore, setPos is being applied to the underlying node, thereby inadvertently affecting both NodePaths. This is not what you want.

Assuming that each of your cells has a different coordinate frame, you need to be able to enter a position that’s different for each instance of your node. This means you need to insert an intervening node for every instance containing the position of the node relative to the cell position.

If you did this right, every NodePath pointing to that node will have the same getPos(render) as a result. This also means you’re able to take an arbitrary NodePath to reposition it using setPos(render, pos).

Alternatively, you can leave the cells without a position, which obviates the need of inserting intermediate “instance” nodes altogether, since you can then just use setPos(pos) to set the local transformation.

On a sidenote, you mention “actual instancing”; however, there is no functional difference between what you are doing with addChild versus what instanceTo does. The only thing that instanceTo does is automatically insert an intervening node and returns a NodePath to this intervening node, allowing you to do setPos(render, pos) on the result, similar to what I described above.


#19

Hmm… I see. I was trying to get that alternative path to the destination node by calling “find” from the current root-cell, but I realise now that there was no individual node in each path on which to place separate transformation data.

It’s tempting to simply set all of my cell root-nodes to have a position of (0, 0, 0) (which is what I presume that you mean by having them be “without a position”–if I’m mistaken, please correct me!). Intuitively, it makes sense for the cell-roots to have a position, so that their level of the scene-graph corresponds roughly to the layout of the cells. However, since they’re being portal-culled, perhaps that’s superfluous, with the positions of their child-objects being enough for the remaining scene-culling…

As to “instanceTo”, since the approaches are so similar, is there any reason to not use that method rather than attaching them by hand via “addChild”? I imagine that, even if I did go through with the culling change discussed above, “instanceTo” would still produce the intended scene-graph, and so should still work for my purposes…


#20

Yes, that’s what I meant with “without a position”.

I suppose if you need an intermediate node to be able to set state and transform on, use instanceTo. If not, you can just use addChild.


#21

Hmm… I’ll have to think about it–in short, whether I want to take the simpler route of keeping things as they are and using “instanceTo” (since my cell-roots currently have positions, and thus “addChild” results in multi-parented objects being rendered at different positions for different cells), or the more complex route of reworking things such that cells have zeroed positions (allowing “addChild” to work). The latter might be more efficient, but the former would be easier, I daresay.

Anyway, thank you for your advice! :slight_smile: