It removes the ability to provide an external canvas. The canvas size… #1474

dmarcos · 2016-05-18T05:53:44Z

… and positioning can be controled styling the a-scene when adding the embedded attribute

ngokevin · 2016-05-26T19:10:38Z

src/core/scene/a-scene.js

@@ -65,11 +65,30 @@ module.exports = registerElement('a-scene', {
          this.setupRenderer();
          this.resize();
        });
+        if (!this.hasAttribute('embedded')) {


Componentize to keep scene uncluttered?

ngokevin · 2016-05-26T19:38:15Z

Should we add a test example showing multiple scenes on a page?

dmarcos · 2016-05-31T21:24:54Z

I don't think multiple scenes per page works quite yet

ngokevin · 2016-06-03T00:07:18Z

src/components/scene/embedded.js

@@ -0,0 +1,13 @@
+var register = require('../../core/component').registerComponent;


registerComponent to be consistent

dmarcos · 2016-07-21T22:10:39Z

The offset in the y axis should still apply

ngokevin · 2016-07-21T22:11:42Z

Yeah, I'm just talking about the Y-axis. Try the geometry gallery scene and enter fullscreen (non-VR).

dmarcos · 2016-07-22T04:28:58Z

I believed I covered all the cases: Desktop with and without headset, mobile, scenes with and without default camera. I have to do one more round of tests with a headset.

ngokevin · 2016-07-22T04:47:46Z

src/systems/camera.js

      defaultCameraEl.setAttribute(DEFAULT_CAMERA_ATTR, '');
      defaultCameraEl.setAttribute('camera', {'active': true});
      defaultCameraEl.setAttribute('wasd-controls', '');
      defaultCameraEl.setAttribute('look-controls', '');
      sceneEl.appendChild(defaultCameraEl);
+      // Adding defautlt offset to the camera


ngokevin · 2016-07-22T04:51:13Z

Consider configurable camera property nonVRHeight or similar name such that user-defined cameras can benefit from the non-VR offset. Default camera can set this to 1.8 by default.
Consider, now or later, to differentiate between VR and flat fullscreen. The save/restore camera pose when entering in and out seems useful for only VR, not flat fullscreen.

dmarcos · 2016-07-22T23:22:47Z

Ready to go. The offset in the y is now applied to all cameras and configured via the userHeight property in camera. The offset is removed in the presence of a headset. When exiting VR the previous (before entering VR mode) position of the camera is restored when a headset is connected.

… and positioning can be controled styling the a-scene when adding the embedded attribute

ngokevin · 2016-07-22T23:27:11Z

very nice

ngokevin · 2016-07-22T23:27:34Z

r+ when you ready to merge

ngokevin · 2016-07-22T23:28:30Z

I would probably add userHeight: 1.8 to <a-camera> as a default.

dmarcos · 2016-07-22T23:29:17Z

@ngokevin can you test a bit to see if everything make sense?

ngokevin · 2016-07-22T23:35:13Z

I tested around a few examples on my laptop, everything makes sense there:

Flat:
- Default camera, 1.8m offset.
- Enter fullscreen, offset still applies.
- Exit fullscreen, pose is not overwritten.

I haven't tested in VR. But I assume you have this:

VR:
- Enter VR, offset is removed.
- Exit VR, pose is restored to before entering VR.

Just the one note that if I did <a-camera></a-camera>, I would sort of expect to have similar defaults as the default camera.

ngokevin · 2016-07-25T22:50:46Z

We can decide on the user-defined <a-camera> default later.

ngokevin · 2016-07-25T22:51:21Z

Nice. Love resolving the camera height inside and outside VR.

FrederickDesimpel · 2016-08-02T17:51:56Z

If we're nitpicking eyeheight would rather be 1.6 m ...

CHANGELOG.md says "Default camera is now positioned at 0, 1.6, 0 rather than 0, 1.8, -4. In VR mode, the 1.6m height offset as defined by camera.userHeight is removed. (aframevr#1474, aframevr#1718)" but in v0.3 doc, still v0.2 value of Default camera position. My English is so poor~Sorry

CHANGELOG.md says "Default camera is now positioned at 0, 1.6, 0 rather than 0, 1.8, -4. In VR mode, the 1.6m height offset as defined by camera.userHeight is removed. (#1474, #1718)" but in v0.3 doc, still v0.2 value of Default camera position. My English is so poor~Sorry

dmarcos added the work in progress label May 18, 2016

dmarcos force-pushed the noExternalCanvas branch from 0890ded to cdea26b Compare May 25, 2016 01:36

dmarcos added needs review and removed work in progress labels May 25, 2016

ngokevin reviewed May 26, 2016
View reviewed changes

ngokevin added needs changes and removed needs review labels May 31, 2016

dmarcos force-pushed the noExternalCanvas branch 8 times, most recently from 5c87917 to dacfc03 Compare June 2, 2016 02:01

dmarcos added needs review and removed needs changes labels Jun 2, 2016

ngokevin reviewed Jun 3, 2016
View reviewed changes

ngokevin added needs changes and removed needs review labels Jun 3, 2016

dmarcos force-pushed the noExternalCanvas branch 4 times, most recently from 35ee462 to 80b03ed Compare June 7, 2016 22:39

dmarcos force-pushed the noExternalCanvas branch 2 times, most recently from 302771c to a868b9e Compare July 22, 2016 04:26

ngokevin reviewed Jul 22, 2016
View reviewed changes

ngokevin added needs changes and removed needs review labels Jul 22, 2016

dmarcos force-pushed the noExternalCanvas branch from a868b9e to 846bbe1 Compare July 22, 2016 23:19

dmarcos added needs review and removed needs changes labels Jul 22, 2016

It removes the ability to provide an external canvas. The canvas size…

f420a45

… and positioning can be controled styling the a-scene when adding the embedded attribute

dmarcos force-pushed the noExternalCanvas branch from 846bbe1 to f420a45 Compare July 22, 2016 23:27

ngokevin merged commit 7d9ddcb into aframevr:master Jul 25, 2016

ngokevin mentioned this pull request Aug 3, 2016

Allow A-Frame to enter fullscreen mono mode if headset not detected #1060

Closed

JChehe mentioned this pull request Aug 18, 2016

Update a-camera.md #1821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

It removes the ability to provide an external canvas. The canvas size… #1474

It removes the ability to provide an external canvas. The canvas size… #1474

dmarcos commented May 18, 2016

ngokevin May 26, 2016

ngokevin commented May 26, 2016

dmarcos commented May 31, 2016

ngokevin Jun 3, 2016

dmarcos commented Jul 21, 2016

ngokevin commented Jul 21, 2016

dmarcos commented Jul 22, 2016

ngokevin Jul 22, 2016

ngokevin commented Jul 22, 2016

dmarcos commented Jul 22, 2016

ngokevin commented Jul 22, 2016

ngokevin commented Jul 22, 2016

ngokevin commented Jul 22, 2016

dmarcos commented Jul 22, 2016

ngokevin commented Jul 22, 2016

ngokevin commented Jul 25, 2016

ngokevin commented Jul 25, 2016

FrederickDesimpel commented Aug 2, 2016

		@@ -0,0 +1,13 @@
		var register = require('../../core/component').registerComponent;

It removes the ability to provide an external canvas. The canvas size… #1474

It removes the ability to provide an external canvas. The canvas size… #1474

Conversation

dmarcos commented May 18, 2016

ngokevin May 26, 2016

Choose a reason for hiding this comment

ngokevin commented May 26, 2016

dmarcos commented May 31, 2016

ngokevin Jun 3, 2016

Choose a reason for hiding this comment

dmarcos commented Jul 21, 2016

ngokevin commented Jul 21, 2016

dmarcos commented Jul 22, 2016

ngokevin Jul 22, 2016

Choose a reason for hiding this comment

ngokevin commented Jul 22, 2016

dmarcos commented Jul 22, 2016

ngokevin commented Jul 22, 2016

ngokevin commented Jul 22, 2016

ngokevin commented Jul 22, 2016

dmarcos commented Jul 22, 2016

ngokevin commented Jul 22, 2016

ngokevin commented Jul 25, 2016

ngokevin commented Jul 25, 2016

FrederickDesimpel commented Aug 2, 2016