Vectors on schema with explicit properties are used by reference not by value.

Description:

• A-Frame Version: 1.3.0
• Platform / Device: All
• Reproducible Code Snippet or URL: https://glitch.com/edit/#!/vector-by-reference-not-value?path=index.html%3A38%3A9

Also: https://glitch.com/edit/#!/vec3-line-update-issue?path=index.html%3A25%3A53

Slightly complex to explain. Let's start with this code.

          vec = new THREE.Vector3(-1, 0, -4)
          box = document.getElementById("box1")
          box.setAttribute("position", vec)
          vec.set(1, 0, -4)
          box.setAttribute("position", vec)

Where does the box end up? At position 1, 0, 4 - as expected

But what about if we define a component like this:

      AFRAME.registerComponent("position-component", {
        schema: {
          position: { type: "vec3" }
        },
        update() {
          this.el.setAttribute("position", vec)
        }
      })

and use it in the exact same way...

          vec = new THREE.Vector3(-1, 0, -4)
          box = document.getElementById("box2")
          box.setAttribute("position-component", {position: vec})
          vec.set(1, 0, -4)
          box.setAttribute("position-component", {position: vec})

Now, the box ends up at position -1, 0, 4.... !

This happens for reasons described in this comment, which I'll explain again here..

The root cause is this code

function isObjectOrArray (value) {
  return value && (value.constructor === Object || value.constructor === Array) &&
         !(value instanceof window.HTMLElement);
}

this function isObjectOrArray is used to determine whether to copy or deep clone an object. It does not classify Vector3s as objects (because value.constructor is set to Vector3, not Object or Array)

This means that when A-Frame copies data to oldData, it just assigns the Vector3 objects by reference, rather than doing a deep copy.

In the case of the position attribute, which uses an unnamed single property in its schema, this doesn't happen. When we come to that code, the top-level vec3 object passed into setAttribute() has already been decomposed into x, y & z components.

But the problem does occur whenever a vec3 is used as a named property in a schema.

Examples of this causing problems with real components include:

• line component (start & end properties)
• raycaster component (origin & direction properties)

The effects manifest in a couple of ways in these different components.

The raycaster code uses data.origin and data.direction directly in code. As a result, if you write code like this:

vec3 = new Vector3(0, 0, 1)
el.setAttribute("raycaster", {origin: vec3}

then subsequent updates to vec3 result in immediate changes to the raycaster origin, and adjustments to the raycaster origin via setAttribute have no effect - the raycaster update() method doesn't even get called because A-Frame can't see any difference between the old data & the new data - they both refer to the same vec3.

One particular situation where this causes problems is trying to use multiple simultaneous cursors, as described here:
https://github.com/aframevr/aframe/issues/5075#issuecomment-1185701335

For the line component it's a bit different. This doesn't directly use data.start and data.end, but copies the x, y & z data from these on in the update() method.

This means that with a line initialized like this...

vec3 = new Vector3(0, 0, 1)
el.setAttribute("line", {start: vec3}

then subequent updates to vec3 don't have any unexpected immediate effects on the line.

However, subsequent code like this will not update the line, as A-Frame won't even invoke the update() method for the same reasons as with raycaster

vec3.set(1, 1, 1)
el.setAttribute("line", {start: vec3})

See this additional glitch to illustrate the weirdness seen with updates to line start.
https://glitch.com/edit/#!/vec3-line-update-issue?path=index.html%3A25%3A53

I believe all this weirdness could be fixed by extending the checks here to consider Vector2, Vector3 & Vector4 as equivalent to arrays/objects and therefore needing a deep copy.

(I believe these are the only property types supported in the current schema API that would have this problem)

I have wondered whether there are significant (possibly intentional) performance benefits in re-using Vector3s, rather than cloning them, but I'm unconvinced that any derived performance benefits would be significant enough to justify all the counter-intuitive weirdness that arises by not cloning vectirs when they are passed on a component schema.

I'd be happy to take this further into a fix with some UTs (based on the glitches linked above) - but first would appreciate a review of the analysis above, and some agreement that vectors pass into components should be cloned (i.e. passed by value, not be reference).

Start in src/core/component.js at isObjectOrArray and inspect how component data is copied into oldData. Reproduce the named vec3 behavior using the linked Glitch examples, then add unit tests covering independent vector values and updates for schemas such as line and raycaster. Done means named Vector2, Vector3, and Vector4 values no longer cause missed updates or unintended shared mutations.