Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New A-Frame location based components #405

Closed
wants to merge 14 commits into from
Closed

Conversation

nickw1
Copy link
Collaborator

@nickw1 nickw1 commented Mar 14, 2022

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?

New. hopefully improved A-Frame components for location-based, plus enhancement of the three.js location-based code.

Can it be referenced to an Issue? If so what is the issue # ?

How can we test it?

I have added two test applications in the examples directory, one for A-Frame and one for three.js; these are in the new-location-based directory in each case.

Summary

The existing location-based components are quite complex code-wise and (IMO) rather difficult to debug. There have been some long standing problems on some devices with location-based such as #275 which have been hard to fix. By simplifying the A-Frame location based components it is hoped that these problems can be resolved more readily.

So two new A-Frame components, gps-new-camera and gps-new-entity-place, have been created, in the new-location-based directory. To avoid duplication of code, these actually wrap the three.js location-based code, which takes care of such things as projecting lat/lon into Spherical Mercator and converting to world coordinates. So to use these components, the three.js location-based code must be included in your project.

Many of the existing properties of gps-camera have been inherited, including simulateLatitude, simulateLongitude, simulateAltitude, gpsMinDistance and positionMInAccuracy. However, minDistance and maxDIstance have been removed as this functionality can be implemented using the near and far properties of the A-Frame camera.

simulateLatitude and simulateLongitude offer improved behaviour compared to the originals, as they did not appear to trigger a gps-camera-update-position event previously. Now they do, which makes it easier to implement code which works with real and simulated GPS.

A new component arjs-device-orientation-controls has also been added. This is a lightweight wrapper around the DeviceOrientationControls class within three.js location-based (which was in turn taken from three.js itself and modified). However note that this does not detect mouse movements so rotation will only work on a mobile device. The reason for this is to separate out orientation sensor movements and mouse movements; in tests I found that code in the A-Frame look-controls component was interfering with the device orientation code, and figured that (for now) it was best to separate them out.

Does this PR introduce a breaking change?

No as the new components have different names for now, gps-new-camera and gps-new-entity-place. However my proposal is, that once they have been tested on a wide range of devices, to replace gps-projected-camera and gps-projected-entity-place with these new components.

So if this happens, a few things will not work:

  • minDistance and maxDistance deprecated; use near and far as detailed above.
  • in keeping with standard A-Frame practice, the gps-camera-update-position event is now emitted by the camera entity that the gps-new-camera is attached to, rather than the window. So any code handling that event will need to be updated to take that into account.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser

Linux desktop: Firefox and Chrome latest.
Mobile: Pixel 3, Android 12, running latest Chrome. Confirmed to display objects in the correct position in-the-field, and objects move as expected when you move round the real world.

Other information

n/a

@nickw1
Copy link
Collaborator Author

nickw1 commented Mar 14, 2022

Sorry, scratch that. Will recreate, the README updates are included, thought these had been merged

@nickw1 nickw1 closed this Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants