Lightbeam in VR using A-Frame #230

princiya · 2017-10-14T08:19:53Z

Issue #202

Launch Lightbeam web-extension and click the VR button [below gif] to see Lightbeam in VR.

@jonathanKingston @cvan @caseyyee let me know what you think.

princiya · 2017-10-14T10:38:42Z

This comment is outdated. We don't require the two repos approach anymore.

Initially I followed the following approach:
Click the VR button and this launches vr-index.html page from the Lightbeam web-extension.

The above method didn't work for the following reasons:

vr-index.html was launched as moz-extension://xxxyyyzzz/vr-index.html
But <a-camera> component didn't work as it requires a server to serve assets etc. from A-Frame.

The existing solution that I have in this PR requires two repos, please let me know of any other inputs to make it possible from within the web-extension without using an additional repo. Thanks.

jonathanKingston · 2017-10-15T16:12:02Z

But component didn't work as it requires a server to serve assets etc. from A-Frame.

We should just fix that I think. I don't know enough why that restriction is in place at the moment. However we can't merge this like it is.

This is looking really great as a demo though.

princiya · 2017-10-15T16:41:27Z

@jonathanKingston i think the restriction is due to serving content across cross origin issues and that's why there is a hard requirement to serve A-Frame via a server. @cvan @caseyyee shed some light?

jonathanKingston · 2017-10-15T16:56:42Z

@princiya why does it need to be from a server at all though? Can't the files be hosted via the add-on?
We can't ship anything with lightbeam that requires an install really.

cvan · 2017-10-15T18:47:44Z

@princiya why does it need to be from a server at all though? Can't the files be hosted via the add-on?

yeah, that's approach I'd recommend (if it works). there's a section in the A-Frame FAQ docs that briefly addresses this.

the CORS requirement isn't the case for every type of media. like images should load fine, for example, but if you're trying to load a local file (that's not over a http:/https: protocol), fetching a JSON file using XHR (instead of fetch) will fail.

I can take a look at it - should be pretty quick to fix (but could be a non-obvious change).

cvan

nice work! I have some questions here. but hopefully your answers will help me understand this a bit better. thanks for doing this 👍

cvan · 2017-10-15T18:50:31Z

src/js/lightbeamVR.js

+      if (site.thirdParties) {
+        const thirdPartyLinks = site.thirdParties.map((thirdParty) => {
+          return {
+            source: website,


I could be off here, and I likely don't understand the structure of the data just yet. but would it be helpful to key this off site.hostname?

cvan · 2017-10-15T18:50:54Z

src/js/lightbeamVR.js

+  async getData() {
+    const data = await storeChild.getAll();
+    const transformedData = this.transformData(data);
+    const blob = new Blob([JSON.stringify(transformedData ,' ' , 2)],


nit: " ," -> ", "

cvan · 2017-10-15T18:51:56Z

src/js/lightbeamVR.js

+      url : url,
+      filename : 'data.json',
+      conflictAction : 'overwrite',
+      saveAs: true


should probably change " : " to ": " above

cvan · 2017-10-15T18:53:20Z

src/js/lightbeamVR.js

+    const fullUrl = 'http://localhost:3000/';
+    async function isOpen() {
+      const tabs = await browser.tabs.query({});
+      const lightbeamTabs = tabs.filter((tab) => {


could use Array#find:

tabs.find(tab => tab.url === fullUrl);

cvan · 2017-10-15T18:54:05Z

src/js/lightbeamVR.js

+    const lightbeamTab = await isOpen();
+    if (!lightbeamTab) {
+      // only open a new LightbeamVR instance if one isn't already open.
+      browser.tabs.create({ url: fullUrl});


nit: the space after the {

cvan · 2017-10-15T18:55:57Z

src/js/lightbeamVR.js

+
+  loadLightbeamVR() {
+    const vrView = document.getElementById('vr-view-button');
+    vrView.addEventListener('click', async () => {


I would also listen for the vrdisplaypresentchange event which fires on window: https://developer.mozilla.org/en-US/docs/Web/Events/vrdisplaypresentchange

cvan · 2017-10-15T19:07:46Z

src/js/lightbeamVR.js

+    const blob = new Blob([JSON.stringify(transformedData ,' ' , 2)],
+      {type : 'application/json'});
+    const url = window.URL.createObjectURL(blob);
+    const downloading = browser.downloads.download({


I know it'd likely be much more convoluted, but this seems like a pretty good use case for a Service Worker, using the Cache API.

some basic Service Workers of how to do that:

https://serviceworke.rs/json-cache_service-worker_doc.html

https://googlechrome.github.io/samples/service-worker/mock-responses/service-worker.js

https://googlechrome.github.io/samples/service-worker/post-message/index.html

https://serviceworke.rs/virtual-server_index_doc.html

you'd just postMessage some data to the worker, stringify it, and have it be returned upon 'fetch' events whenever requesting /data.json.

I could be misunderstanding the flow here, but I think that would enable storing, serving, and updating the JSON file just fine.

let me know if I ought to file a separate issue to elaborate on this. don't want to suggest too much additional work here 💻

Lets make this a separate issue, we have this downloading elsewhere in Lightbeam too anyway even if we remove the need here. The data will change a lot. Is there likely to be an advantage on caching this if it might change per minute?
We could cache the JSON some other form of storage too perhaps if Service Workers don't work correctly in web extensions (I have never tried and I know there are some bugs).

cvan · 2017-10-15T19:08:11Z

src/index.html

@@ -14,6 +14,7 @@
    <script src="ext-libs/dialog-polyfill.js"></script>
    <script src="js/viz.js" type="text/javascript"></script>
    <script src="js/lightbeam.js" type="text/javascript"></script>
+    <script src="js/lightbeamVR.js" type="text/javascript"></script>


nit: can remove the type="text/javascript" from the past three (for consistency)

cvan · 2017-10-15T19:10:19Z

But component didn't work as it requires a server to serve assets etc. from A-Frame.

this seems like an A-Frame bug. which part didn't work exactly? were there any console errors?

I assume if I just pull the repo, I can reproduce that?

jonathanKingston · 2017-10-15T19:18:55Z

I would work on fixing the issue with the server being required when hosting from the web extension and remove the need for a JSON file, instead updating the nodes as we do for the main diagram.

Perhaps we should just call this "3d view" and leave the goggles icon for people that have a headset.

If the extension can host the code itself then we can just make it a different page that the browser action filters for, so the code to check for "localhost:3000" in the isOpen filter can be removed too.

princiya · 2017-10-16T05:17:02Z

@cvan thanks for the nits 👍 will update the code based on your feedback.

this seems like an A-Frame bug. which part didn't work exactly? were there any console errors?

a-camera it was!

princiya · 2017-10-16T05:18:57Z

@jonathanKingston agree on your feedback about fixing this PR to solve the issue of having a server from web-extension.

Any inputs on how to go about this?

jonathanKingston · 2017-10-16T13:18:01Z

@princiya I think you should be able to just host the files yourself in the addon like we do for dexie, we might need to compile them (I don't know enough about a-frame to know if that is the case) either way just ignore their example and load the scripts into an extension page like we do for d3 for the main lightbeam page.

princiya · 2017-10-21T19:08:15Z

src/js/aframe/aframeForcegraph.js

+	      });
+	    } */
+			// Auto add color to uncolored nodes
+			const { nodes, links } = await aframeHelper.getData();


@jonathanKingston as a quick hack, I decided to pass nodes, links into this aframe-component.

Because we weren't using a local server, the above qwest code didn't work for json file. The qwest library uses XMLHTTPRequest by default.

I shall think about submitting a patch for this aframe-component itself :)

cvan

looking good! thanks for taking this on 👍

cvan · 2017-10-21T20:21:31Z

src/js/aframe/aframeHelper.js

+      if (site.thirdParties) {
+        const thirdPartyLinks = site.thirdParties.map((thirdParty) => {
+          return {
+            source: website,


is it possible that there will be duplicate link items? do we want to make the list contain only unique values - or is this fine?

cvan · 2017-10-21T20:23:46Z

src/js/aframe/aframeHelper.js

+  }
+};
+
+aframeHelper.init();


without reading through aframeForcegraph.js, how is this used externally from this file? this module ought to export something, I’d think?

cvan · 2017-10-21T20:25:11Z

src/indexVR.html

+    <script src="js/storeChild.js"></script>
+    <script src="js/aframe/aframeHelper.js"></script>
+    <script src="https://aframe.io/releases/0.5.0/aframe.min.js"></script>
+    <script src="js/aframe/aframeForcegraph.js"></script>


this file looks like it’s autogenerated from Webpack? did you write this or is it a vendor, third-party lib? in other words, how could one modify the source and regenerate it?

@cvan Here is the main source.

I was trying to load forcegraph components by passing nodes and links and it kept complaining about wrong JSON syntax.

<a-entity forcegraph="nodes:[{id: 1, name: 'first'}, {id: 2, name: 'second'}]; links: [{source: 1, target: 2}]"</a-entity>

The jsonUrl property for forcegraph component worked while running on a local server, but as you pointed out earlier, when I was trying to load indexVR.html via moz-extension:// it failed.

<a-entity forcegraph="jsonUrl: data/data.json"></a-entity>

The forcegraph component uses qwest here which uses XMLHttpRequest and doesn't seem to work without a local server.

Keeping MozFest in priority, the next task was to quickly look into forcegraph's source code and I decided to use this hack.

This is off-course temporary, and I will consider submitting a patch to this forcegraph component itself to make it work without requiring a local server for qwest (to fetch the JSON file).

I attempted my hack directly on the Webpack generated file because I am running short on time (MozFest is this week) and I didn't have to worry about the forcegraph component failing to build etc.

Considering this is a 1-2 week hack solution, can we leave it this way or are we violating any license rules?
cc @jonathanKingston

cvan · 2017-10-21T20:25:33Z

src/js/aframe/aframeHelper.js

@@ -0,0 +1,42 @@
+const aframeHelper = {


I’d name this something more descriptive

is it possible to write this as an A-Frame component?

cvan · 2017-10-21T20:26:31Z

src/js/aframe/aframeHelper.js

+
+  async getData() {
+    const data = await storeChild.getAll();
+    const { nodes, links } = this.transformData(data);


could you just return this here (instead of creating these intermediate constant variables)?

cvan · 2017-10-21T20:29:10Z

src/js/lightbeamVR.js

+      const lightbeamTabs = tabs.filter((tab) => {
+        return (tab.url === fullUrl);
+      });
+      return lightbeamTabs[0] || false;


I would make sure that this returns a Boolean. so: !!lightbeamTabs[0]

cvan · 2017-10-21T20:29:36Z

src/js/lightbeamVR.js

+      // only open a new LightbeamVR instance if one isn't already open.
+      browser.tabs.create({ url: fullUrl });
+    } else if (!lightbeamTab.active) {
+       // re-focus LightbeamVR if it is already open but lost focus


nit: remove trailing white space (one space) and add a period

@cvan surely, a hard-core nit picker ;) beats @jonathanKingston just kidding!

thanks for reviewing and I shall fix these.

cvan · 2017-10-21T20:30:09Z

src/js/lightbeamVR.js

+    } else if (!lightbeamTab.active) {
+       // re-focus LightbeamVR if it is already open but lost focus
+      browser.tabs.reload(lightbeamTab.id);
+      browser.tabs.update(lightbeamTab.id, {active: true});


are these methods async? if so, should use Promise callback

cvan · 2017-10-21T20:30:19Z

src/manifest.json

@@ -40,5 +40,7 @@
      "js/capture.js",
      "js/background.js"
    ]
-  }
+  },
+


can remove this newline

cvan · 2017-10-21T20:30:56Z

src/manifest.json

-  }
+  },
+
+  "content_security_policy": "script-src 'self' 'unsafe-eval' https://aframe.io/releases/0.5.0/aframe.min.js; object-src 'self';"


unrelated: can you bump the A-Frame version to 0.7.1 (can be in separate PR; I can file an issue)

for some reasons, in the latest version I get the following error:
THREE.Raycaster: Unsupported camera type. from three.js.

oof, is that with 0.7.0 or 0.7.1?

both 0.7.0 and 0.7.1

I really would prefer to install the module locally than remotely load it, is this not possible through npm?

possible, but I get the same error for newer versions. but will load the code via npm 👍

princiya · 2017-10-22T05:45:44Z

@cvan cc @jonathanKingston

this file looks like it’s autogenerated from Webpack? did you write this or is it a vendor, third-party lib? in other words, how could one modify the source and regenerate it?

@cvan Here is the main source.

I was trying to load forcegraph components by passing nodes and links and it kept complaining about wrong JSON syntax.

<a-entity forcegraph="nodes:[{id: 1, name: 'first'}, {id: 2, name: 'second'}]; links: [{source: 1, target: 2}]"</a-entity>

The jsonUrl property for forcegraph component worked while running on a local server, but as you pointed out earlier, when I was trying to load indexVR.html via moz-extension:// it failed.

<a-entity forcegraph="jsonUrl: data/data.json"></a-entity>

The forcegraph component uses qwest here which uses XMLHttpRequest and doesn't seem to work without a local server.
Keeping MozFest in priority, the next task was to quickly look into forcegraph's source code and I decided to use this hack.
This is off-course temporary, and I will consider submitting a patch to this forcegraph component itself to make it work without requiring a local server for qwest (to fetch the JSON file).
I attempted my hack directly on the Webpack generated file because I am running short on time (MozFest is this week) and I didn't have to worry about the forcegraph component failing to build etc.
Considering this is a 1-2 week hack solution, can we leave it this way or are we violating any license rules?

jonathanKingston · 2017-10-23T06:50:49Z

The forcegraph component uses qwest here which uses XMLHttpRequest and doesn't seem to work without a local server.

Can you pass in a fully qualified URL by using: browser.extesion.getUrl("data.json") or similar?
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/extension/getURL

I was trying to load forcegraph components by passing nodes and links and it kept complaining about wrong JSON syntax.

I assume you used JSON.stringify here? I could believe it would complain about single quotes for example.

jonathanKingston · 2017-10-23T06:54:59Z

src/js/lightbeamVR.js

+    });
+  },
+
+  async runLightbeam() {


I would prefer not to have this code at all. The background script that currently checks for "index.html" could also check for indexVR.
I would load the url in the same page rather than create another tab.

princiya · 2017-10-23T07:27:01Z

@jonathanKingston

Can you pass in a fully qualified URL by using: browser.extesion.getUrl("data.json") or similar?

This is the patch (and a couple of other things) I was talking about submitting to the forcegraph component. https://github.com/vasturiano/aframe-forcegraph-component

Because of time constraints, I decided to put the tiny hack solution into the webpack generated file from the above component.

At this point, the forcegraph component is limited to run only through a local server.

cvan · 2018-03-07T21:12:59Z

friendly ping: @princiya would you or someone on your team happen to have some spare cycles to resurrect some of the work on this? let me know if you need a hand. I or someone on my team might be able to. thanks - and, again, great work here!

princiya · 2018-03-08T19:11:09Z

@cvan certainly I would love to see this PR merged. I shall try to clean-up (and recall :)) things this weekend and will ping you. Thank you for writing :)

jonathanKingston · 2018-03-08T19:21:15Z

@cvan to be clear @princiya is a contributor at the moment and I'm not actively working on Lightbeam at the moment. I can review PRs and help out where possible though. Maybe @princiya should clean the code up and we could pass some of the visualisation work off to your team. I know the 2D could really merit from having someone with real visualisation perf knowledge look at it.

cvan · 2018-03-09T01:54:19Z

@jonathanKingston: @princiya: completely understand! you have already gone way above and beyond. I do not want to set any expectations (you have already exceeded them 😄), and I want to make sure it is okay with you both that we build on top of and extend the great work you have done already.

we have some spare cycles and can work on the art assets. @caseyyee in particular can help with any perf analysis.

@princiya: no pressure, really. do not feel obligated to work on this over the weekend. I will check back next week, and let me just know the roadmap, processes, and potential other folks I ought to rope in in conversations if I we want to add the VR visualisations mode properly into the extension. thanks!

princiya · 2018-03-12T20:45:25Z

@cvan It would be great if you could extend Lightbeam for A-Frame. I am happy to talk about the work done so far, roadmap etc. I have NDA volunteer access to Mozilla's slack and I am in the #lightbeam channel. Also, princiya dot marina at gmail dot com is my email address. Thank you.

princiya added 2 commits October 14, 2017 13:35

aframe commits

1ad021d

file renamed

0d20cb3

reload logic fixed. fixes mozilla#202

ffd2295

jonathanKingston added the do not merge label Oct 15, 2017

cvan suggested changes Oct 15, 2017

View reviewed changes

aframe supported

3d72935

princiya commented Oct 21, 2017

View reviewed changes

nit fixes

7658d97

princiya removed the do not merge label Oct 21, 2017

cvan suggested changes Oct 21, 2017

View reviewed changes

nit fixes

05d9ec4

jonathanKingston reviewed Oct 23, 2017

View reviewed changes

Lightbeam in VR using A-Frame #230

Are you sure you want to change the base?

Lightbeam in VR using A-Frame #230

Conversation

princiya commented Oct 14, 2017 • edited Loading

princiya commented Oct 14, 2017 • edited Loading

jonathanKingston commented Oct 15, 2017

princiya commented Oct 15, 2017

jonathanKingston commented Oct 15, 2017

cvan commented Oct 15, 2017

cvan left a 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

cvan commented Oct 15, 2017

jonathanKingston commented Oct 15, 2017

princiya commented Oct 16, 2017 • edited Loading

princiya commented Oct 16, 2017

jonathanKingston commented Oct 16, 2017

Choose a reason for hiding this comment

cvan left a 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

princiya Oct 22, 2017 • edited 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

princiya commented Oct 22, 2017

jonathanKingston commented Oct 23, 2017

Choose a reason for hiding this comment

princiya commented Oct 23, 2017 • edited Loading

cvan commented Mar 7, 2018

princiya commented Mar 8, 2018

jonathanKingston commented Mar 8, 2018

cvan commented Mar 9, 2018

princiya commented Mar 12, 2018

princiya commented Oct 14, 2017 •

edited

Loading

princiya commented Oct 14, 2017 •

edited

Loading

princiya commented Oct 16, 2017 •

edited

Loading

princiya Oct 22, 2017 •

edited

Loading

princiya commented Oct 23, 2017 •

edited

Loading