cobweb_dom integration

[andreasplesch]

An issue for tracking inclusion of cobweb_dom into a cobweb release

[andreasplesch]

Let me first refactor a bit.

The documentation should include that while attribute manipulation and node creation work as expected and are unlikely to change in the future, event handling is subject to change

[andreasplesch]

I clean-up a bit and made some minor improvements for 0.2 release:

https://github.com/andreasplesch/cobweb_dom/commit/363cb7ef5e75dca404473bed079d1fa5dbd71e53

I also took a stab at documentation:

https://github.com/andreasplesch/cobweb_dom/blob/master/README.md

Andreas

[create3000]

Note X3DBaseNode .getField (name) can throw an exception if the field with name does not exists.

[create3000]

I think, I do not directly include cobweb_dom into Cobweb, which makes development for you easier, but make a separate paragraph on the Cobweb web-page, with instructions how to include and so on, as it is very straightforward to include cobweb_dom as separate script element for users who want it.

[create3000]

If cobweb_dom is not directly integrated in Cobweb, i see that Cobweb has to be modified that X3D dom content within the X3D tag has to be parsed. That would require a merge.

[andreasplesch]

getField(name) exception: https://github.com/andreasplesch/cobweb_dom/blob/master/cobweb/cobweb_dom.js#L25 I think you are thinking about the case when an attribute was added which is not a field. https://github.com/create3000/cobweb/blob/master/cobweb.js/cobweb/Parser/XMLParser.js#L552 catches a thrown exception in https://github.com/create3000/cobweb/blob/master/cobweb.js/cobweb/Basic/X3DBaseNode.js#L364 but cobweb_dom.js#L25 does not. I could add a try catch but there may be performance implications since for animations this would be called a lot: http://stackoverflow.com/questions/19727905/in-javascript-is-it-expensive-to-use-try-catch-blocks-even-if-an-exception-is-n

I tested by creating and setting a new attribute, and indeed the exception is thrown. It may be appropriate that there is an error and the script is stopped at this point. In any case, the installed mutation observer keeps working, and will continue to call the processMutation() function.

[andreasplesch]

DOM content in X3D tag on page: For now, it may not be necessary for cobweb to parse the DOM elements inside the X3D tag on the page since cobweb_dom can do it:

https://github.com/andreasplesch/cobweb_dom/blob/master/cobweb/cobweb_dom.js#L59

This only works if the page is xhtml encoded.

This brings up another point. It may be a little cleaner and more spec, conforming to not use the X3D tag as a placeholder or trigger for including X3D content since the X3D element is specified in the X3D XML encoding standard. So instead <X3DCanvas> or similar could be used with a url attribute, or a full X3D doc. inside like so:

<X3DCanvas url='"actual x3d file"' ></X3DCanvas>

<X3DCanvas>
  <X3D>
   <Scene>
   ...
    </Scene>
  </X3D>
</X3DCanvas>

[andreasplesch]

released v0.3 with modified x3d event to dom event translation.

[andreasplesch]

released v0.4 for cobweb > v2.1 with inline access support and other improvements.

Updated https://andreasplesch.github.io/cobweb_dom/tests/inline_Mushrooms.xhtml to use new release.

[andreasplesch]

released v0.5 with changes in PR#11

[create3000]

1. Route deletion should be very stable - although a test would be very appreciated - as I recommended the use of X3DRoute.dispose which should ALWAYS do what it is has to do, as the route knows the right execution context to tell them: delete me.
2. That was the point I didn't understand why only sensors need to dispatch events, where all outputOnly and inputOutput field can send events. Yes, a more common approach would be nice. Do you have to observe all node creations, even from X3DExecutionContext.createNode called inside a Script node? I don't see how this would be possible for you.

To detect all outputOnly and inputOutput you could use a function like below, the access type is a bit mask, and this will never change anymore, ie. a simple if statement like below is enough.

function foo (baseNode)
{
   var fieldDefinitions = baseNode .getFieldDefinitions ();

   for (var i = 0, length = fieldDefinitions .length; i < length; ++ i)
   {
      var fieldDefinition = fieldDefinitions [i];

      if (fieldDefinition .accessType & X3DConstants .outputOnly)
      {
         var field = baseNode .getField (fieldDefinition .name);

         // Do somethin with the outputOnly OR inputOutput field
      }
}

Best regards, Holger

[andreasplesch]

1) https://andreasplesch.github.io/cobweb_dom/tests/inline_route.xhtml is a simple test. It works fine. 2) Sensors are just the most obvious events which one would to use, and the one I wanted to think about first. Glad you agree that all nodes' output fields could dispatch dom events.

Yes, Script node generated nodes do not affect the DOM so cannot be observed. But this would be probably not expected to be possible by DOM javascript apps ?

I did think about reflecting back changed x3d field values into the DOM (first just attributes), so a getAttribute would always get the latest value (perhaps changed by Routes and Scripts) but I am not sure if it is a good idea since it makes things complicated. DOM manipulated values are there anyways and x3d changed values can be caught by listening to events. It is to cleaner to allow changes to x3d from the DOM only via mutations, and on the other hand only allow communication from x3d to the DOM via event listeners. At least at first.

[I think it is possible to keep a DOM tree and the live x3d scene graph in perfect sync., so at some point it may be worth exploring. It owuld be probably necessary to keep a backreference from x3d object back to its DOM object (as there is already for inline with its .dom property). Then createNode could check if there is already a corresponding DOM object and otherwiser create and appned it.]

Yes, I was wondering how to best look for output Fields. I will try some things.

[create3000]

            appendInlineDOM: function (element, loaded)
            {
                // Now loaded and in .dom
                // Inline must have Scene
                var
                    node      = element .x3d,
                    watchList = this .loadSensor .getField ("watchList"),
                    isLoaded  = this .loadSensor .getField ("isLoaded");

                // XXX: node .dom .querySelector ('Scene') is always null, probably node .getInternalScene () .dom?

The comment below can be removed!

// XXX: node .dom .querySelector ('Scene') is always null, probably node .getInternalScene () .dom?

[andreasplesch]

OK, done.

[andreasplesch]

I started to add dispatch callbacks to all output fields with some straightforward changes:

https://rawgit.com/andreasplesch/cobweb_dom/609140e93f193ca0aa8061703cee34f72cd91a97/tests/cobweb_d3.xhtml

I discovered field.isOutput() is probably all what is needed .

Looks good sofar, will now also just addDispatchers for all newly added nodes.

[create3000]

This is indeed a handy function, and there to be used, which hides the complex functionality of what accessType really is.

[create3000]

FYI: https://jsperf.com/try-catch-performance-overhead

[andreasplesch]

I get 47% slower with try/catch on Mozilla 49.0.2 .

[andreasplesch]

released cobweb_dom v0.6 with events for all output fields and shortened event names: updated: https://andreasplesch.github.io/cobweb_dom/tests/cobweb_d3.xhtml to show interpolated values using value_changed event new example: https://andreasplesch.github.io/cobweb_dom/tests/interactiveTransformations.xhtml , adopted from x3dom to test sensor events.

[create3000]

You may have well considered if the try catch is really needed and every statement that is essential for the control flow will indeed cost something. The try catch, catches the exception from Parser.attribute and this is how the parser works. I figured out that you create a new instance of the parser every time in processMutation. This can be avoided, the parser can be created once in the setup function and then be used where needed, this would be a possible optimization. Object creation is very slow, design you code with object reuse in mind. Second the processAttributes function is not really needed as the case "attributes" will alway give you just one attribute, although is is in plural, you can directly jump to processAttribute, thus this function call can be avoided.

[create3000]

In line 114 field .addFieldCallback (field .getName (), ...) it would be better to use something like a namespace for the name parameter, ie. field .addFieldCallback ("DomIntegration." + field .getName (), ..) is my recomendation.

[create3000]

line 92: for (var i = 0; i < elements .length; ++i) write: for (var i = 0, length = elements .length; i < length; ++i) a local variable is much faster than a property access for every cycle, especially in this case where it can be many elements.

[andreasplesch]

Thanks. These are helpful.

I think there is a possibility that in a single mutation there are multiple changed attributes. I still need to check this. processAttributes is there just as a reminder or in preparation.

Yes, I was thinking about creating just a single parser. It was safer to create a new one each time for now. Since there already seems to be some GC churn, this is probably a priority.

I think there may be opportunities to explicitly check for undefined or null instead of using try/catch which should be faster. Pretty sure that try/catch is generally discouraged for js.

[andreasplesch]

I am adding support for ProtoInstances in the proto branch:

https://rawgit.com/andreasplesch/cobweb_dom/proto/tests/inline_proto.xhtml

It was necessary to special case fieldValue in processAttribute():

https://github.com/andreasplesch/cobweb_dom/blob/proto/cobweb/cobweb_dom.js#L337

and to add the .x3d property to ProtoInstance nodes in XMLParser.js:

https://github.com/andreasplesch/cobweb/blob/proto/cobweb.js/cobweb/Parser/XMLParser.js#L369

Does this look reasonable ?

With these changes, adding. removing and modifying ProtoInstances via the DOM appears to work well.

I want to put together at least one more test example, using perhaps your gears.x3d example with a simple GUI to define the number of teeth and such.

I am uncertain if it make sense to be able to manipulate ProtoDeclarations via the DOM. The SAI spec. allows for it: http://www.web3d.org/documents/specifications/19775-2/V3.3/Part02/servRef.html#ProtoDeclarationHandling http://www.web3d.org/documents/specifications/19777-1/V3.3/Part1/functions.html#t-ExecutionContextProperties So it looks like it should make sense. Some careful thinking probably required.

[andreasplesch]

In order to deal with the case when the addedNode from a mutation is 'fieldValue' I am using now parser .child() in processAddedNode():

https://github.com/andreasplesch/cobweb_dom/blob/proto/cobweb/cobweb_dom.js#L195

https://rawgit.com/andreasplesch/cobweb_dom/proto/tests/gears_proto.xhtml

instead of parser .statement(). Not sure if parseIntoNode() should handle this.

Currently, I am not sure about allowing dynamic modifications (or deletion) of existing ProtoDeclarations or the addition of new ones. Waiting for feedback on the x3d-public list.

[andreasplesch]

Released v. 0.7 with support for ProtoInstance and a basic event logger. Still need to reuse single parser.

[andreasplesch]

Released v 0.75 with parser function reuse. Simple substitution seems to work well. All examples still work fine.

[andreasplesch]

Just a link explaining .nodeName XHTML is required because .nodeName returns uppercased names for HTML5

http://ejohn.org/blog/nodename-case-sensitivity/

node.nodeName is (almost) always upper case ( <Transform>-> TRANSFORM)

attribute.name is always lower case (DEF -> def)

[create3000]

Finally releases V2.4 of Cobweb. This version handles browser option »enableInlineViewpoints« for cobweb_dom, ie. a construction like below should bind the included Viewpoint nodes and other X3DBindableNode's just fine:

        <X3DCanvas class="browser">
            <X3D xmlns="http://www.web3d.org/specifications/x3d-namespace">
                <Scene>
                    <Inline url='"https://cdn.rawgit.com/create3000/Library/master/Examples/TreasureIsland/index.x3d"'/>
                </Scene>
            </X3D>
            <div class="fallback">
            </div>
        </X3DCanvas>

[andreasplesch]

I am working on allowing html documents for use with cobweb_dom rather than just xhtml. On the cobweb side, the main issue is that the .nodeName property returns names normalized to upper case: Scene becomes SCENE. The other issue is that the attribute.name property is always normalized to lower case: skyColor becomes skycolor.

I modified cobweb to deal with these changes. Take a look at the changes in this PR in my fork: https://github.com/andreasplesch/cobweb/pull/7/files

In the parser, I simply check for upper case nodeNames in addition to the real ones.

In SupportedNodes.js, I add the upper case node names to the list of supported nodes, so they can be created just like the real case ones.

In X3DBaseNode.js in the setField() function I add the lower case names as keys linked to the actual field object. This is what I am least sure about.

With these changes and some minor ones one the cobweb_dom side, most things work. Here are some tests:

https://github.com/andreasplesch/cobweb_dom/tree/master/tests/html5

for example:

https://rawgit.com/andreasplesch/cobweb_dom/master/tests/html5/interactiveTransformations.html

I am not sure if it is a good idea to have two entries in the ._fields[] array for each field. In theory, for xml one probably could have now a situation where the same field is used twice which may or may not be a problem. But I could not think of a better way. Perhaps a special cobwebhtml.js version but that would not be very elegant.

[create3000]

The lower case field names in X3DBaseNode should be avoided, than X3DBaseNode getField could be really strict if in the parser getField (fieldName .toLowerCase ()) is done.

[create3000]

Or if Cobweb should be more lazy, it should be as it is.

[create3000]

If X3DBaseNode has additionally lowercase fields than this would be possible:

function (node : SFNode)
{
   var choiceA = node .whichChoice;
   var choiceB = node .whichchoice;
}

Is this desired?

[create3000]

Cannot see the pull request in my repro. Would have been merged already.

[andreasplesch]

I was not sure if these kind of more about invasive changes are appropriate. I added the PR but feel free to ignore or redo, of course.

[andreasplesch]

The remaining problem was the script node since HTML parses its content differently. There is no cdata section in HTML. But I think by special preprocessing it is possible to treat the mark-up as xml and add the expected child nodes. See html5 branch of Cobweb_dom. This works now. The raw text content is parsed as xml and the resulting nodes appended. The raw text content itself is removed.

[andreasplesch]

Perhaps it suffices to change in the parser .getField(name) to getField( toRealCase(name) ) in the attribute handler . toRealCase would lookup the real name of a field from a long list. Web3d has such a list generated from the xml schema. What do you think ? I think it may work.

I think toRealCase() should only look up the actual field name if the provided name is all lower case. This will preserve error checking for xhtml, eg. Skycolor would not be valid. So it should be called lowerCaseToRealCase():

lowerCaseToRealCase: function (name)
{
  if ( name === name .toLowerCase() )
  { 
     return attributeNameMap [name];
  };
  return name; // unchanged if mixedCase (xhtml)
}

[andreasplesch]

http://www.web3d.org/specifications/AllX3dElementsAttributes3.3.xml is what I had in mind. I can generate a lookup object from that: { 'skycolor': 'skyColor', .. }

See also https://github.com/andreasplesch/X3DCaseFixer/blob/master/maps/attributesMap.dat

[andreasplesch]

I implemented the idea above to look up the actual field name in the parser and therefore not to have to have a lowercase field in addition to the actual field.

Here are the changes:

https://github.com/andreasplesch/cobweb/commit/7346bcb468a83162c82eac364d0fb36009e3d8e2 : add the large look up table. X3DConstants is probably not the right place but it was convenient because it is already used in the parser.

https://github.com/andreasplesch/cobweb/commit/7d2707811dff09bee2fe7a760f42d4aee94217b6 : remove the setField() code to add the lowercase fields.

https://github.com/andreasplesch/cobweb/commit/ec1c9badb614f53b9bfa632782249d9b37e0fa16 : use the look up table in the parser with a small function.

With that, things seem to work:

https://rawgit.com/andreasplesch/cobweb_dom/master/tests/html5/cobweb_d3.html uses now the new aproach, in the lcase branch

PR #25 includes these changes and would be an alternative to PR #24 .

I am not sure which is better . Using the look up table requires a look up each time an attribute is mutated and needs the large table but does avoid doubling the fields .

[create3000]

The attribute lookup table must be a separate file located in the Parser folder. X3DConstants is not appropriate as it is accessible in the Script node. Fell free to find a suitable name for this file.

I would suggest to rename the attrToRealCase function to attributeToCamelCase as it makes more clear what the function does and abbreviations are not allowed for any name except for special cases like i in for loops.

The defined property fieldname_ in X3DBaseNode must be removed since it is not necessary. In _predefinedFields and _fields properties everything looks good.

Toll

[create3000]

The ternary operator is better written like below:

condition
? value_if_true
: value_if_false 

The question mark and the colon must be at front since they are there more a eye catcher. But the ternary operator should be avoided if a simple if statement does the same job since if statement are more readable.

[andreasplesch]

Ok. I will give those directions a try. For the new file, I would follow X3DConstants.js in terms of define, and Object .preventExtensions (X3DConstants); Object .freeze (X3DConstants); Object .seal (X3DConstants); but I am little unsure. What about Parser/htmlAttributeSupport.js ?

Can you clarify your X3DBaseNode.js comment ? X3DBaseNode.js is now unchanged and not affected by this PR #25 ? I think the issue was that the cobweb.uncompressed.js file still had the fieldname_ property defined in X3DBaseNode because I forgot to remove it there. The actual X3DBaseNode.js is not affected. Sorry for the confusion.

[andreasplesch]

PR #25 is updated to follow the recommendations. Please take a look.

[create3000]

Merged both pull requests without changes.

[andreasplesch]

Thanks. I will prepare a new release of cobweb_dom to use with latest cobweb.

[andreasplesch]

I released cobweb_dom 0.8 with support for regular html5 encoded web pages.

[andreasplesch]

Updated cobweb_dom master to adapt to reworking of XMLParser in cobweb master. Tests indicate that all features continue to work.