AR-js-org / AR.js

Image tracking, Location Based AR, Marker tracking. All on the Web.
MIT License
5.41k stars 924 forks source link

[feature request] SimulateLatitude, SimulateLongitude work even if gps is turned on #500

Closed marcusx2 closed 1 year ago

marcusx2 commented 1 year ago

I think that if the simulate values are not 0, they should be used regardless if the gps is working or not. After all, why the extra step of having to turn off the GPS, if the simulate value already tells you if it should be used if it's different than 0? This seems unnecessary. There's also the annoyance of having to clear the cache to be asked for gps permission again, or having to use incognito mode.

Is there a reason the GPS must be turned off to use the simulate values even they are already different than 0, and therefore I am saying that I want to use the simulate values regardless if the gps is on or off?

Maybe another property like 'useSimulateValuesEvenIfGpsIsOn'(default false to avoid breaking existing projects) could be created? Let me know what you think.

nickw1 commented 1 year ago

@marcusx2 just getting back to you on this one.

The gps-new-camera component contains this logic:

if (this.data.simulateLatitude === 0 && this.data.simulateLongitude === 0) {
      this.threeLoc.startGps();
}

So you can see that the 'real' GPS is only started if simulateLatitude and simulateLongitude are 0 (or not specified, as 0 is the default value).

Can you let me know exactly what you try which produces this behaviour, so I can reproduce?

Do you dynamically set simulateLatitude and simulateLongitude later? The code does not yet cover this possibility, but is an easy fix.

Thanks.

marcusx2 commented 1 year ago

@marcusx2 just getting back to you on this one.

The gps-new-camera component contains this logic:

if (this.data.simulateLatitude === 0 && this.data.simulateLongitude === 0) {
      this.threeLoc.startGps();
}

So you can see that the 'real' GPS is only started if simulateLatitude and simulateLongitude are 0 (or not specified, as 0 is the default value).

Can you let me know exactly what you try which produces this behaviour, so I can reproduce?

Do you dynamically set simulateLatitude and simulateLongitude later? The code does not yet cover this possibility, but is an easy fix.

Thanks.

Then it's not working as expected. It gets the gps values even if simulateLatitude and simulateLongitude are different than 0. I can only get the simulate values to work in incognito mode for some reason.

Code is very simple

<!DOCTYPE html>
<html>
<head>
<title>AR.js A-Frame Location-based</title>
<script src="https://aframe.io/releases/1.0.4/aframe.min.js"></script>
<script type='text/javascript' src='https://raw.githack.com/AR-js-org/AR.js/master/three.js/build/ar-threex-location-only.js'></script>
<script type='text/javascript' src='https://raw.githack.com/AR-js-org/AR.js/master/aframe/build/aframe-ar.js'></script>
</head>
<body>
<a-scene vr-mode-ui='enabled: false' arjs='sourceType: webcam; videoTexture: false; debugUIEnabled: false' renderer='antialias: true; alpha: true'>
    <a-camera look-controls-enabled='false' arjs-device-orientation-controls='smoothingFactor: 0.1' gps-new-camera='positionMinAccuracy: 100; gpsMinDistance: 5; simulateLatitude: 100; simulateLongitude: 100; simulateAltitude: 0; gpsTimeInterval: 0;' position='0 10 0'></a-camera>
    <a-entity material='color: red' geometry='primitive: box' gps-new-entity-place="latitude: latitude; longitude: longitude" scale="1 1 1" ></a-entity>
</a-scene>
<script>
    const entity = document.querySelector("[gps-new-camera]");
    setTimeout(() => {
        console.log(entity.object3D.position);
    }, 0);
</script>
</body>
</html>

Also, good to know that setting the values dynamically don't work yet. Should I create an issue for this as well so that we can set the simulate values dynamically?

nickw1 commented 1 year ago

Not sure in that case, let me check your code.

Best keep it in this issue as it's related.

nickw1 commented 1 year ago

(setting the values dynamically does work in most cases, by the way, as long as you set an initial simulateLatitude and simulateLongitude in the HTML)

nickw1 commented 1 year ago

I tried your example and it works for me (this was on both Firefox and Chrome), and I accepted the "Allow site to use location?" prompt. The lat and lon I use is not my real location. I made some tweaks to use a valid lat/lon (as a latitude of 100 is invalid), and also replaced setTimeout() with setInterval() so it reports the location continuously:

<!DOCTYPE html>
<html>
<head>
<title>AR.js A-Frame Location-based</title>
<script src="https://aframe.io/releases/1.0.4/aframe.min.js"></script>
<script type='text/javascript' src='https://raw.githack.com/AR-js-org/AR.js/master/three.js/build/ar-threex-location-only.js'></script>
<script type='text/javascript' src='https://raw.githack.com/AR-js-org/AR.js/master/aframe/build/aframe-ar.js'></script>
</head>
<body>
<a-scene vr-mode-ui='enabled: false' arjs='sourceType: webcam; videoTexture: false; debugUIEnabled: false' renderer='antialias: true; alpha: true'>
    <a-camera look-controls-enabled='false' arjs-device-orientation-controls='smoothingFactor: 0.1' gps-new-camera='positionMinAccuracy: 100; gpsMinDistance: 5; simulateLatitude: 51.049; simulateLongitude: -0.723; simulateAltitude: 0; gpsTimeInterval: 0;' position='0 10 0'></a-camera>
    <a-entity material='color: red' geometry='primitive: box' gps-new-entity-place="latitude: 51.05; longitude: -0.72" scale="100 100 100" ></a-entity>
</a-scene>
<script>
    const entity = document.querySelector("[gps-new-camera]");
    setInterval(() => {
        console.log(entity.object3D.position);
    }, 1000);
</script>
</body>
</html>

If I try this, the position is what I would expect in Spherical Mercator coordinates.

Try this, you should get an x of around -80000 and a z of around -6620000 if it works.

marcusx2 commented 1 year ago

It only works on incognito mode here, if I don't allow location services. I tried on chrome for Windows and chrome for Android, both the same thing :(. On second thought, it's probably not about the incognito mode itself, but the fact that on non-incognito location services is already running and I'd have to clear the cache to block it.

nickw1 commented 1 year ago

Strange. Try completely clearing your cache. All I can suggest is that others (@kalwalt ?) try out your example to see if it works for them.

There is one issue that I can see, which I mentioned above, namely that if simulateLatitude and simulateLongitude are not initially set in the HTML, but set later, they will be overridden by the real GPS location, so I will provide a fix for this.

marcusx2 commented 1 year ago

I tested on other browsers as well. This only works if location services is turned off. Maybe it has something to do with newer devices. I'm using an old Samsung S8, and windows 8.1.

nickw1 commented 1 year ago

I will have to draw a blank on this, I am afraid - though if others experience the same problem, please do post here.

nickw1 commented 1 year ago

The only other thing I suggest you do is to check out the source code for AR.js, and then, using console.log() statements, modify the update() method of the gps-new-camera component to work out what is happening on your setup, and then build it. So for example does it read simulateLatitude and simulateLongitude? Does it start the GPS with startGps()?

marcusx2 commented 1 year ago

The only other thing I suggest you do is to check out the source code for AR.js, build it and, using console.log() statements, modify the update() method of the gps-new-camera component to work out what is happening on your setup. So for example does it read simulateLatitude and simulateLongitude? Does it start the GPS with startGps()?

It's fine, this is not a huge deal. I'm done debugging this problem. Maybe other people with the same problem can chime in later with more insight.

Platform-Group commented 1 year ago

@nickw1 I experience this on literally every device I've tried it on, Poco F5, 10th gen iPad, Samsung galaxy. No idea how this is broken if the logic is as simple as:

if (this.data.simulateLatitude === 0 && this.data.simulateLongitude === 0) {
      this.threeLoc.startGps();
}

But it clearly is. I'll try your advice to debug this when I get around to it, but as things currently are I can't enable location services anyway as doing so disables dragging to calibrate true north.

nickw1 commented 1 year ago

@Platform-Group could you provide the code you're using to test this?

As you can see above I am at a loss to explain it but it's conceivable something about your test code allows the problem to be diagnosed.

Thanks.

Platform-Group commented 1 year ago

@nickw1 sure, I've had this issue with the most basic of examples though. I assumed it was normal so I've just been doing a lot of testing with my GPS off.

<!DOCTYPE html>
<html>
  <head>
    <title>AR.js A-Frame Location-based</title>
    <meta
      name="viewport"
      content="width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0, user-scalable=no"
    />
    <script
      type="text/javascript"
      src="https://aframe.io/releases/1.4.2/aframe.min.js"
    ></script>
    <script src="https://unpkg.com/aframe-look-at-component@0.8.0/dist/aframe-look-at-component.min.js"></script>
    <script
      type="text/javascript"
      src="https://www.unpkg.com/@ar-js-org/ar.js@3.4.5/three.js/build/ar-threex-location-only.js"
    ></script>
    <script
      type="text/javascript"
      src="https://www.unpkg.com/@ar-js-org/ar.js@3.4.5/aframe/build/aframe-ar.js"
    ></script>
  </head>
  <body>
    <script>
      // Component registration needs to happen before the scene is constructed
      AFRAME.registerComponent("clickable", {
        init: function () {
          // Note that entities need the following attributes to be clicked:
          // emitevents="true"
          // cursor="rayOrigin: mouse"
          // fuse="false"
          // Of course it will be fuse: false etc for inline
          const clickDisabled = false;
          this.el.addEventListener("click", function (ev, target) {
            // Having lots of issues with clicking firing lots of times, so we put a timeout on this listener
            if (clickDisabled) return;
            clickDisabled = true;
            setTimeout(() => {
              clickDisabled = false;
            }, 100);

            const text = ev.currentTarget.querySelector("a-text");
            const scale = text.getAttribute("scale");
            Object.keys(scale).forEach(
              (key) =>
                // (scale[key] = scale[key] + (scale[key] > 1 ? -1 : 1))
                (scale[key] = scale[key] + 1)
            );
            text.setAttribute("scale", scale);
          });
        },
      });

      AFRAME.registerComponent("fov-change", {
        init: function () {
          // grab the camera
          const cam = document.querySelector("[camera]");
          // this.el should also work for this as long as your fov-change component is on the camera
          // grab the current FOV, this will be 80 as that's the default for aframe
          const fov = cam.getAttribute("camera").fov;
          console.log("Wow, fov is ", fov);
          // update the camera with the new FOV
          // if holding portrait use 61:
          // cam.setAttribute("camera", "fov", 61)

          // For landscape
          cam.setAttribute("camera", "fov", 45);
        },
      });
    </script>
    <a-scene
      vr-mode-ui="enabled: false"
      arjs="sourceType: webcam; videoTexture: true; debugUIEnabled: false"
      renderer="antialias: true; alpha: true"
    >
      <!-- without the far attribute I luckily found (https://aframe.io/docs/1.4.0/primitives/a-camera.html)
    we can't even get distances to render if they're too far, this namely includes Wembley Stadium from the skyscraper -->
      <a-camera
        fov-change
        id="camera"
        gps-new-camera="gpsMinDistance: 5; simulateLatitude: 51.514438902042905; simulateLongitude: -0.08304790201881936; simulateAltitude: 400;"
        far="1000000"
      ></a-camera>

      <!-- Using smoothing seems to disable the drag to orientate camera controls:
      look-controls-enabled='true'
      arjs-device-orientation-controls='smoothingFactor: 0.1' -->

      <a-entity id="rotatable" calibrate-orientation>
        <a-entity
          gps-new-entity-place="latitude: 50.82133005513377; longitude: -0.15091614411679277"
          scale="300 300 300"
          look-at="[gps-new-camera]"
          ,
        >
          <a-entity
            gltf-model="/assets/map_pin/scene.gltf"
            rotation="0 0 0"
          ></a-entity>
          <a-text
            value="The i360"
            position="0 0.6 0"
            align="center"
            scale="0.7 0.7 0.7"
          ></a-text>
        </a-entity>

        <a-entity
          gps-new-entity-place="latitude: 51.53080116780565; longitude: -0.15642379267654144"
          scale="300 300 300"
          look-at="[gps-new-camera]"
        >
          <a-entity
            gltf-model="/assets/map_pin/scene.gltf"
            rotation="0 0 0"
          ></a-entity>
          <a-text
            value="The Regent's Park"
            position="0 0.6 0"
            align="center"
            scale="0.7 0.7 0.7"
          ></a-text>
        </a-entity>

        <a-entity
          gps-new-entity-place="latitude: 51.60420048111148; longitude: -0.06621237317930276"
          scale="300 300 300"
          look-at="[gps-new-camera]"
        >
          <a-entity
            gltf-model="/assets/map_pin/scene.gltf"
            rotation="0 0 0"
          ></a-entity>
          <a-text
            value="Tottenham Hotspur Stadium"
            position="0 0.6 0"
            align="center"
            scale="0.7 0.7 0.7"
          ></a-text>
        </a-entity>

        <a-entity
          gps-new-entity-place="latitude: 51.519516734608516; longitude: -0.12697042050139684"
          scale="300 300 300"
          look-at="[gps-new-camera]"
        >
          <a-entity
            gltf-model="/assets/map_pin/scene.gltf"
            rotation="0 0 0"
          ></a-entity>
          <a-text
            value="The British Museum"
            position="0 0.6 0"
            align="center"
            scale="0.7 0.7 0.7"
          ></a-text>
        </a-entity>

        <a-entity
          gps-new-entity-place="latitude: 51.530869224823206; longitude: -0.12577308439060156"
          scale="300 300 300"
          look-at="[gps-new-camera]"
        >
          <a-entity
            gltf-model="/assets/map_pin/scene.gltf"
            rotation="0 0 0"
          ></a-entity>
          <a-text
            value="St Pancras Station"
            position="0 0.6 0"
            align="center"
            scale="0.7 0.7 0.7"
          ></a-text>
        </a-entity>
<!-- more entities of the same format were here -->
      </a-entity>
    </a-scene>
  </body>
</html>
<script>

  window.onload = () => {
    // Example code for running after the scene is made, this places a triangle to your north
    let testEntityAdded = false;
    const entity = document.createElement("a-triangle");
    entity.setAttribute("scale", {
      x: 20,
      y: 20,
      z: 20,
    });
    entity.setAttribute("material", { color: "red" });
    const text = document.createElement("a-text");
    text.setAttribute("scale", {
      x: 4,
      y: 4,
      z: 4,
    });
    text.setAttribute("position", {
      x: 0,
      y: 1,
      z: 0,
    });
    text.setAttribute("value", "N");
    text.setAttribute("align", "center");
    entity.appendChild(text);
    document
      .querySelector("#camera")
      .addEventListener("gps-camera-update-position", (e) => {
        console.log("gps position updated", e.detail.position);
        entity.setAttribute("gps-new-entity-place", {
          latitude: e.detail.position.latitude + 0.001,
          longitude: e.detail.position.longitude,
        });
        if (!testEntityAdded) {
          // Add a triangle to the north of the initial GPS position
          document.querySelector("a-scene").appendChild(entity);
        }
        testEntityAdded = true;
      });
  };
</script>
<style></style>
Platform-Group commented 1 year ago

So just to be sure I wasn't hallucinating this behaviour, I gave my phone both camera and gyro permissions. But left the location services permission request box on screen. Pointed my camera at an entity from the simulated location, then allowed location services, the entity vanishes and as I look around I'm only left with the entities which are near my real location.

nickw1 commented 1 year ago

OK let me try your example when I have a moment. It certainly "shouldn't" start location if the simulated lat and lon are not zero, but let me give it a go.

Platform-Group commented 1 year ago

I did some debugging @nickw1

the play function is receiving this.data.simulatedLatitude and this.data.simulatedLongitude as zero despite that not being what was set on the a-camera entity. However the update function recieves the correct value.

update: function (oldData) {
    this.threeLoc.setGpsOptions({
      gpsMinAccuracy: this.data.positionMinAccuracy,
      gpsMinDistance: this.data.gpsMinDistance,
      maximumAge: this.data.gpsTimeInterval,
    });
    console.log('update says sim lat is ', this.data.simulateLatitude)
    console.log('update says old data lat is ', oldData.simulateLatitude)
    if (
      (this.data.simulateLatitude !== 0 || this.data.simulateLongitude !== 0) &&
      (this.data.simulateLatitude != oldData.simulateLatitude ||
        this.data.simulateLongitude != oldData.simulateLongitude)
    ) {
      this.threeLoc.stopGps();
      this.threeLoc.fakeGps(
        this.data.simulateLongitude,
        this.data.simulateLatitude
      );
      this.data.simulateLatitude = 0;
      this.data.simulateLongitude = 0;
    }
    if (this.data.simulateAltitude > -Number.MAX_VALUE) {
      this.threeLoc.setElevation(this.data.simulateAltitude + 1.6);
    }
  },

  play: function () {
    console.log('play says sim lat is ', JSON.stringify(this.data.simulateLatitude), ' and is equal to 0? ', this.data.simulateLatitude === 0)
    if (this.data.simulateLatitude === 0 && this.data.simulateLongitude === 0) {
      console.log('starting gps')
      this.threeLoc.startGps();
    } else {
      console.log('not starting gps')
    }
  },

Output:

update says sim lat is 51.514438902042905 gps-new-camera.js:99 update says old data lat is undefined gps-new-camera.js:119 play says sim lat is 0 and is equal to 0? true gps-new-camera.js:121 starting gps

Platform-Group commented 1 year ago

@nickw1 @marcusx2 I've got a fix for you:

update: function (oldData) {
    this.threeLoc.setGpsOptions({
      gpsMinAccuracy: this.data.positionMinAccuracy,
      gpsMinDistance: this.data.gpsMinDistance,
      maximumAge: this.data.gpsTimeInterval,
    });
    if (
      (!this.data.fakeGpsStarted) &&
      (this.data.simulateLatitude !== 0 || this.data.simulateLongitude !== 0) &&
      (this.data.simulateLatitude != oldData.simulateLatitude ||
        this.data.simulateLongitude != oldData.simulateLongitude)
    ) {
      this.threeLoc.stopGps();
      this.threeLoc.fakeGps(
        this.data.simulateLongitude,
        this.data.simulateLatitude
      );
      this.data.fakeGpsStarted = true
    }
    if (this.data.simulateAltitude > -Number.MAX_VALUE) {
      this.threeLoc.setElevation(this.data.simulateAltitude + 1.6);
    }
  },

The issue is that update was setting simulated lat and long to 0, and update runs before play for some reason. So I just added in the fakeGpsStarted boolean to handle that instead of setting simulated lat and long to 0.

nickw1 commented 1 year ago

@Platform-Group many thanks for your fix, I will incorporate it into the code base.

I am puzzled though. play() should indeed be called after update() according to the component lifecycle docs:

https://aframe.io/docs/1.4.0/core/component.html#play

and certainly after init(). Thus the correct values should surely be available.

Furthermore this.data should contain the property data.

It's not clear to me, therefore, why the simulateLatitude and simulateLongitude would be 0 in update() if they are specified in the HTML.

Platform-Group commented 1 year ago

@nickw1

Well because update was called first, that was setting lat and long to 0 and overwriting the values specified in the html. So then play comes around and starts the gps because lat and long is 0 by then

nickw1 commented 1 year ago

@Platform-Group ah ok thanks. Oops... I forgot that update() was resetting simulateLatitude and simulateLongitude to 0 after passing them on to threeLoc ! Should have checked the code, I know ;-) ... but couldn't see any reason why the code would do that, hence overlooked it in my mind.

I'm not sure whether resetting simulateLatitude and simulateLongitude is actually necessary and can't remember why I did it. Let me check the code, it may well be that the simplest solution is to remove those two lines.

Platform-Group commented 1 year ago

@nickw1 from what I saw it was simply to prevent fakeGps from starting on every update call, so that's why I added the new bool in

nickw1 commented 1 year ago

@Platform-Group that makes sense. Do you want to submit a PR? Your fix looks sensible to me so I should be able to incorporate it into the dev branch quickly.

Remember to submit the PR against dev, not master.

Platform-Group commented 1 year ago

@nickw1

I currently have a jumble of a couple of fixes and console.logs I've not cleaned up on my working branch, also the compiled files seem to not be gitignored so those are in there too. Not really sure exactly what you want but you're welcome to grab the code out of my commit.

Platform-Group commented 1 year ago

@nickw1 I've found that my changes don't really work, the constraints as they are seem to actually change the cameras native resolution which is pretty screwed.

I'm now leaving out the ideal width and height values. Which I believe is correct as it isn't asserting a resolution that is completely screwed. But I don't think I can really trust that either as I'm having weird issues with it on my device with multiple possible video feeds. I'll be working on it more though so maybe I'll have something better soon.

Oh yeah it also doesn't seem to work quite right sometimes when the device is vertical vs horizontal.

Also you'll run into browser compatibility issues as apparently the format of that constraints object is different on different browsers depending on which they are and how old they are. I've already found that I can break the camera feed by putting in an ideal resolution that's too high.

nickw1 commented 1 year ago

@Platform-Group I mean a pull request in which you fork the repo, make the changes and then submit a pull request to dev branch. Are you ok with making pull requests? Doing so saves work as we don't then need to make the same changes again to the main AR.js repository - and reduces the chance of accidental bugs.

If not I will make your changes but it's as well learning how to make pull requests if you don't know how to do so already. Thanks!

nickw1 commented 1 year ago

@nickw1 I've found that my changes don't really work, the constraints as they are seem to actually change the cameras native resolution which is pretty screwed.

I'm now leaving out the ideal width and height values. Which I believe is correct as it isn't asserting a resolution that is completely screwed. But I don't think I can really trust that either as I'm having weird issues with it on my device with multiple possible video feeds. I'll be working on it more though so maybe I'll have something better soon.

Oh yeah it also doesn't seem to work quite right sometimes when the device is vertical vs horizontal.

Also you'll run into browser compatibility issues as apparently the format of that constraints object is different on different browsers depending on which they are and how old they are. I've already found that I can break the camera feed by putting in an ideal resolution that's too high.

This relates to issue #498?

Thanks for your work on this, once you've fixed it more reliably again I'd recommend you submit a pull request.

Platform-Group commented 1 year ago

Yeah I know how to make PRs, I just don't think my work so far is a solution or should even be merged. I've actually reverted it locally now in favour of using an iframe to get around broken embed functionality instead.

nickw1 commented 1 year ago

@Platform-Group I have implemented (a slightly different version of) your fix to #500.

See PR #560.

I actually don't think the flag is needed anyway, it makes more sense to me to allow these properties to be set multiple times. But not fixing this now as it might break things for some people.

Platform-Group commented 1 year ago

For some reason for the previous few comments of mine I thought we were talking about the camera resolution/ratio issue. But anyway that flag wasn't about setting properties multiple times, it was to fix the original issue described in this thread which I described the fix for further up.