Media tools followup fixes #463

netpro2k · 2018-07-18T01:31:03Z

This PR addresses a bunch of the fixes outlined in #451, namely:

Fixes for loading google poly urls
Fixes for loading GIFs
Use positional audio for video/audio objects
Destroy objects when they "fall off the world"
Fix bounding boxes for certain models
Add loading indicators while models/images are loading
Unify the code used for media tools and spawners (spawners now can specify the url for the media they wish to spawn)
Support for backend media API changes
Attribution for Giphy urls in the add media dialog
Rotate objects at the end of the cursor to face the user

It would probably have been wise to break this up, but most of these were small fixes. The large majority of the work centered around unifying spawners, adding loading indicator, and fixing bounding boxes. Many underlying bugs and race conditions were discovered in aframe-physics-system and super-hands in the process, many of which exist in production today, but just didn't manifest in very noticeable ways.

There is some additional code cleanup that is probably warranted (particularly position-at-box-shape), though this is in a sufficiently better state than what is in production now that it feels silly to continue holding it back for those.

the farspark url

axis

…into media-tools-followup

resolution and gets loading indicators

…into media-tools-followup

InfiniteLee · 2018-07-23T19:50:51Z

package.json

@@ -41,7 +41,7 @@
    "jsonschema": "^1.2.2",
    "moving-average": "^1.0.0",
    "naf-janus-adapter": "^0.10.1",
-    "networked-aframe": "https://github.com/mozillareality/networked-aframe#mr-social-client/master",
+    "networked-aframe": "https://github.com/mozillareality/networked-aframe#bug/aframe-initialization-race-condition",


can we point back to networked-aframe/master or pull in the latest to the dev fork and keep pointing at that? It got updated with our PR's last week.

InfiniteLee · 2018-07-23T19:52:06Z

src/components/cursor-controller.js

@@ -97,6 +98,11 @@ AFRAME.registerComponent("cursor-controller", {
      if (this.data.drawLine) {
        this.el.setAttribute("line", { start: this.origin.clone(), end: this.data.cursor.object3D.position.clone() });
      }
+
+      // The curser will always be oriented towards the player about its Y axis, so bjects held by the cursor will rotate towards the player.


typos:
// The ~~curser~~ cursor will always be oriented towards the player about its Y axis, so ~~bjects~~ objects held by the cursor will rotate towards the player.

InfiniteLee · 2018-07-23T20:21:30Z

src/components/gltf-model-plus.js

@@ -202,6 +203,7 @@ function cachedLoadGLTF(src, preferredTechnique, onProgress) {
 AFRAME.registerComponent("gltf-model-plus", {
  schema: {
    src: { type: "string" },
+    basePath: { type: "string", default: undefined },


If basePath is not specified, nothing checks for that case and ultimately will wind up with an error being thrown in THREE.GLTFLoader in cachedLoadGLTF(). There should probably be a check to catch that case prior to undefined being passed into THREE.GLTFLoader.

InfiniteLee · 2018-07-23T20:22:14Z

src/components/gltf-model-plus.js

  // Load the gltf model from the cache if it exists.
  if (!GLTFCache[src]) {
    GLTFCache[src] = new Promise((resolve, reject) => {
      const gltfLoader = new THREE.GLTFLoader();
+      gltfLoader.path = basePath;


see comment on basePath below

the intent here is that setting undefined is the same as not setting it. GLTFLoader doesnt require a path to be set.

InfiniteLee · 2018-07-23T20:29:53Z

src/components/image-plus.js

@@ -180,6 +144,7 @@ AFRAME.registerComponent("image-plus", {
      texture.minFilter = THREE.LinearFilter;
      videoEl.addEventListener("loadedmetadata", () => resolve(texture), { once: true });
      videoEl.onerror = reject;
+      texture.audioSource = this.el.sceneEl.audioListener.context.createMediaElementSource(videoEl);


We shouldn't attach arbitrary objects to existing three.js constructs. If you need to associate the audioSource with a texture you should create a map in this component.

InfiniteLee · 2018-07-23T20:32:47Z

src/components/image-plus.js

+    this.el.setObject3D("mesh", this.mesh);
+    this.el.setAttribute("shape", {
+      shape: "box",
+      halfExtents: { x: width / 2, y: height / 2, z: 0.05 }


0.05 should probably be defined in component schema

InfiniteLee · 2018-07-23T20:37:39Z

src/components/image-plus.js

+      halfExtents: { x: width / 2, y: height / 2, z: 0.05 }
+    });
+    if (this.el.components.body && this.el.components.body.body) {
+      this.el.components.body.syncToPhysics(); // not sure if necessary?


This doesn't seem like it should be necessary, but verify?

Since we have already tested it a bunch with this in, going to leave it in for now to avoid missing some edge case. Put in a todo to evaluate though.

InfiniteLee · 2018-07-23T20:58:02Z

src/components/super-spawner.js

-    this.el.sceneEl.appendChild(entity);
+  onGrabEnd(e) {
+    this.heldEntities.delete(e.detail.hand);
+    // This tells super-hands we are handling this releae


// This tells super-hands we are handling this ~~releae~~ release

InfiniteLee · 2018-07-23T21:01:47Z

src/components/super-spawner.js

+    /**
+     * Source of the media asset the spawner will spawn when grabbed. This can be a gltf, video, or image, or a url that the reticiulm media API can resolve to a gltf, video, or image.
+     */
+    src: { default: "https://asset-bundles-prod.reticulum.io/interactables/Ducky/DuckyMesh-438ff8e022.gltf" },


having this defined here as the default seems wrong- this should get moved into the environment component data

Yep, the plan would be to update the enviornments after we push this, then we can remove this default. This is just to allow us to push without breaking the existing environments during out push.

InfiniteLee · 2018-07-23T21:09:35Z

src/hub.html

                >
-                    <a-entity class="delete-button" visible-while-frozen>
+                    <a-entity class="delete-button" visible-while-frozen scale="1.00 1.00 1.00">


isn't scale 1 1 1 by default?

InfiniteLee · 2018-07-23T22:14:28Z

src/utils/auto-box-collider.js

@@ -0,0 +1,20 @@
+export function getBox(entity, boxRoot) {


cache rotation and copy instead of clone

export function getBox() { const rotation = new THEE.Quaternion(); return function (entity, boxRoot) { ... rotation.copy(entity.object3D.rotation); ...

InfiniteLee · 2018-07-23T22:21:00Z

src/components/position-at-box-shape-border.js

@@ -1,3 +1,5 @@
+import { getBox } from "../utils/auto-box-collider.js";
+
 const PI = Math.PI;
 const HALF_PI = PI / 2;
 const THREE_HALF_PI = 3 * PI / 2;


const THREE_HALF_PI = 3 * HALF_PI;

InfiniteLee · 2018-07-23T22:25:56Z

src/components/position-at-box-shape-border.js

+        this.mesh = this.el.getObject3D("mesh");
+        if (this.el.components.shape) {
+          this.shape = this.el.components.shape;
+          this.halfExtents = new THREE.Vector3().copy(this.shape.data.halfExtents);


Cache this vector3

…edia-tools-followup

johnshaughnessy and others added 30 commits July 9, 2018 16:45

Fix offset

d7c5781

Fix google poly resolving and relative gltf asset loading

c6d09bb

Use positional audio for videos

bf8917e

Destroy interactables if they "fall out of the world"

9a80978

Fix issue with postional audio on multiple instances of the same video

e5c1aff

Small cleanup to add media dialog

50c9c9b

Rework interactable spawners to use media-tools

8c4a812

Fix for loading farspark-whitelisted urls

288efa2

CLeanup in super-spawner

d767e66

Use the origin url provided by reticulum instead of parsing it out of

ad635b5

the farspark url

Use expected_content_type from reticulum instead for static mapping

82fb7d0

Change cursor to always orient objects towards the player about the Y

94b4c57

axis

Add the media loading entity

b737496

Merge branch 'media-tools-followup' of https://github.com/mozilla/hubs …

e43de78

…into media-tools-followup

Use requiresNetworkUpdate

8f2ea3f

Remove automatic box shape if an inflated gltf created its own shapes.

57c0844

Unify media into a single component so each client does its own farspark

a8a2a1b

resolution and gets loading indicators

Cleanup of media-loader

64d9d69

Factor out async util

01c3a29

Update yarn.lock

995c05c

Remove unused counter

67cd832

Update aframe-physics-system in lock file

9342c82

Merge branch 'media-tools-followup' of https://github.com/mozilla/hubs …

7abdd17

…into media-tools-followup

Allow inflation

9e3ecdc

Fix issue with objects spawning in the wrong position

ec15a39

Don't resize spawned objects by default

f63603c

Fix for inflating gltf models with no actual components

51ffabb

Fix dragged objects not spawning correctly

8216906

Fix delete button

0b55dcf

Using offset-relative-to is actually required

245fe8b

johnshaughnessy and others added 2 commits July 19, 2018 17:05

Remove code we have for debug

8279813

Add simple giphy attribution

19016da

netpro2k changed the title ~~Media tools followup fixes (WIP)~~ Media tools followup fixes Jul 21, 2018

netpro2k requested a review from InfiniteLee July 21, 2018 00:01

InfiniteLee reviewed Jul 23, 2018

View reviewed changes

johnshaughnessy and others added 6 commits July 23, 2018 17:25

Fix bug where you can't rotate objects after double grab.

0ff0170

Fixes requested in OR

2986f1d

Check el grabbed state

dbc9387

Merge branch 'media-tools-followup' of github.com:mozilla/hubs into m…

b504e87

…edia-tools-followup

Update networked aframe

df0790c

lint

f8eafb9

johnshaughnessy merged commit e8b35b1 into master Jul 24, 2018

johnshaughnessy deleted the media-tools-followup branch July 26, 2018 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media tools followup fixes #463

Media tools followup fixes #463

netpro2k commented Jul 18, 2018 •

edited by johnshaughnessy

Loading

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

netpro2k Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

netpro2k Jul 24, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

netpro2k Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

InfiniteLee Jul 23, 2018

Media tools followup fixes #463

Media tools followup fixes #463

Conversation

netpro2k commented Jul 18, 2018 • edited by johnshaughnessy Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netpro2k commented Jul 18, 2018 •

edited by johnshaughnessy

Loading