x3dom / x3dom

X3DOM. A framework for integrating and manipulating X3D scenes as HTML5/DOM elements.
http://x3dom.org
Other
815 stars 271 forks source link

How can I judge that an x3domNode is no longer used? #1220

Open microaaron opened 1 year ago

microaaron commented 1 year ago

The USE node is still valid after the DEF node is removed. Like this: https://jsitor.com/S7EVHXwFB. Shape1Node still works on Gp2 and Gp3 but its _nameSpace and _xmlNode have been emptied. It looks a little weird but maybe it matches the design. This brings a new problem. How can I judge that an x3domNode is no longer used? Neither onRemove() nor parentRemoved() seem to be able to judge.

microaaron commented 1 year ago

I think it is necessary to judge the quantity of _parentNodes when executing parentRemoved(), if the quantity is zero, remove all childNodes.

https://jsitor.com/MbzdqNpih

parentRemoved : function ( parent )
{
          if(this._parentNodes.length == 0)
          {
              console.log(this._DEF+" is no longer used.");
              for (var i=this._childNodes.length-1;i>=0;i--) {
                this.removeChild(this._childNodes[i]); //remove this node from child's _parentNodes then trigger child.parentRemoved()
              }
          }
}
onRemove : function ()
            {
                if(this._parentNodes.length == 0)
                {
                  console.log(this._DEF+" is destroyed");
                }
            }
andreasplesch commented 1 year ago

In X3D that should not happen. The x3dom behaviour does not seem correct. A USE node is supposed to be actually 'physically' the same node as the DEF node, just appearing somewhere else, under another parent. This needs to be treated carefully.

The DOM cannot represent USE X3D nodes properly since in the DOM a child can only have one parent. Does Gp2 and Gp3 still have their _x3domNode after the DEF node is removed ?

Perhaps onRemove should also remove the DOM node of any USE node although this would probably surprising and drastic to most web programmers.

microaaron commented 1 year ago

Yes, Gp2 and Gp3 still have their _x3domNode after the DEF node is removed. "document.getElementById("Gp1").remove()" just cuts off the parent-child relationship between SCENE and Gp1 by removeChild().

Removing USE nodes may also trigger onRemove() and parentRemoved(). OnRemove() and parentRemoved()don't know which DOM node triggered them, nor can they see the relationship between DOM nodes. They can only handle relationships between x3d nodes.

andreasplesch commented 1 year ago

I think in X3D removing a USE node in the scene graph means actually removing the node everywhere because there is only one node (the original DEF node which can have multiple parents). It may make sense to look into the SAI documentation/specification, and ask on x3d-public.

https://www.web3d.org/documents/specifications/19777-1/V3.3/Part1/functions.html#t-ExecutionContextFunctions

lists removeNamedNode(string) as an execution context (scene) function.

This means that only nodes with a DEF name can be removed. That must mean that they are removed everywhere. So X3D does not seem to allow removing just a single child node, USE or DEF.

The abstract SAI:

https://www.web3d.org/documents/specifications/19775-2/V3.3/Part02/servRef.html#NamedNodeHandling

There are three distinct remove named node actions: one for DEF, one for IMPORT and one for EXPORT semantics. Probably each needs to be handled slightly differently.

microaaron commented 1 year ago

I submit this issue because of a memory release problem. My solution is to remove all child nodes of an x3d node when its quantity of parents is zero, making this x3d node unreferenced.

As for how to automatically remove all DEF and USE nodes, I have no idea. Maybe it's not bad to let web programmers choose whether to delete them or not, even if it doesn't conform to the x3d specification.

Maybe we can add support for standard function definitions in the future, but we still can't prevent web programmers from directly manipulating DOM nodes at will. We should tolerate any operation as much as possible.

andreasplesch commented 1 year ago

This is very reasonable. What happens currently if all DOM nodes using the same DEF and USE names are manually removed, eg. Gp1, Gp2 and Gp3 ? Memory is not released ?

microaaron commented 1 year ago

Yes, when Gp1's DOM node is removed, Gp1's x3dom node is just cut off from its parent-child relationship with SCENE, but it is still the parent of GP1_1, and Gp2 is also the parent of GP1_1, and Gp2's parent is SCENE. So Gp1 will not be released. I think since Gp1 has no parent, it should cut off the parent-child relationship with GP1_1.

          remove Gp1->        remove Gp2->        remove Gp3->  
    SCENE               SCENE               SCENE               SCENE           
    / | \                 | \                   \                               
   /  |  \                |  \                   \                              
 Gp1 Gp2 Gp3         Gp1 Gp2 Gp3         Gp1 Gp2 Gp3         Gp1 Gp2 Gp3        
  |  /   /            |  /   /            |  /   /            |  /   /          
  | /   /             | /   /             | /   /             | /   /           
Gp1_1  /            Gp1_1  /            Gp1_1  /            Gp1_1  /            
  |   /               |   /               |   /               |   /             
  |  /                |  /                |  /                |  /              
Shape1              Shape1              Shape1              Shape1              

↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑ current ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑
↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓ my expectation ↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓

          remove Gp1->        remove Gp2->        remove Gp3->  
    SCENE               SCENE               SCENE               SCENE           
    / | \                 | \                   \                               
   /  |  \                |  \                   \                              
 Gp1 Gp2 Gp3         Gp1 Gp2 Gp3         Gp1 Gp2 Gp3         Gp1 Gp2 Gp3        
  |  /   /               /   /                   /                              
  | /   /               /   /                   /                               
Gp1_1  /            Gp1_1  /            Gp1_1  /            Gp1_1               
  |   /               |   /                   /                                 
  |  /                |  /                   /                                  
Shape1              Shape1              Shape1              Shape1              
microaaron commented 1 year ago

Perhaps according to the standard, removing an x3d node does not mean removing its children, which can still be children of other nodes. But removing a DOM necessarily removes its children.

andreasplesch commented 1 year ago

It turns out I was wrong. Removing a DEF node does not remove it everywhere in the scene, in X3D. It removes only one occurrence. Similarly, removing a USE node removes only one occurrence.

So it looks like x3dom is doing it correctly ?

microaaron commented 1 year ago

Yes, I also think x3dom is doing it correctly. The next step is to deal with the memory release problem.

andreasplesch commented 1 year ago

In addition, X3D has a function to make a node unnamed, so it cannot be referenced anymore in the future. This does not affect existing occurrences. I do not think this function is currently implemented.

It also appears that a DEF node which became completely unreferenced and unlinked to a scene can still later be readded as a USE node. This means its composition cannot be deleted from browser memory (unless it also becomes unnamed).

microaaron commented 1 year ago

Currently, when a DEF node is removed, its name is removed from the defMap. It can no longer be readded as a USE node, unless it has already been read as a USE node. x3dom.X3DDocument.prototype.removeX3DOMBackendGraph

microaaron commented 1 year ago

It may not be necessary to consider whether x3d nodes affect DOM nodes, just like setFieldValue doesn't affect DOM node, but setAttribute does.