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

triage WebGL builds #19

Closed
isiyu opened this issue Apr 19, 2017 · 10 comments
Closed

triage WebGL builds #19

isiyu opened this issue Apr 19, 2017 · 10 comments
Assignees
Labels

Comments

@isiyu
Copy link
Contributor

isiyu commented Apr 19, 2017

WebGL builds of our examples aren't working in latest develop.
@MateoV - do you have bandwidth to investigate this?

@wilhelmberg
Copy link
Contributor

wilhelmberg commented May 30, 2017

Looked at the Slippy example.

First I got: InvalidTokenException: Please configure your access token in the menu!
But StreamingAssets exists in the generated output.

I applied this patch:

diff --git a/sdkproject/Assets/Mapbox/Unity/MapboxAccess.cs b/sdkproject/Assets/Mapbox/Unity/MapboxAccess.cs
index ff6e2a7..611fe9a 100644
--- a/sdkproject/Assets/Mapbox/Unity/MapboxAccess.cs
+++ b/sdkproject/Assets/Mapbox/Unity/MapboxAccess.cs
@@ -93,7 +93,7 @@ namespace Mapbox.Unity

                private void ValidateMapboxAccessFile()
                {
-#if !UNITY_ANDROID
+#if !UNITY_ANDROID && !UNITY_WEBGL
                        if (!Directory.Exists(Application.streamingAssetsPath) || !File.Exists(_accessPath))
                        {
                                throw new InvalidTokenException("Please configure your access token in the menu!");
@@ -107,7 +107,7 @@ namespace Mapbox.Unity
                /// </summary>
                private void LoadAccessToken()
                {
-#if UNITY_EDITOR || !UNITY_ANDROID
+#if UNITY_EDITOR || (!UNITY_ANDROID && !UNITY_WEBGL)
                        AccessToken = File.ReadAllText(_accessPath);
 #else
             AccessToken = LoadMapboxAccess();

and the token seems to load fine now.

However the WebGL page has been loading for several minutes (+8) now.
Chrome's task manager shows (CPU constantly changing between ~26% and 33%):
image
and I'm not even able to close the tab.

Will try a lighter example next.

@wilhelmberg
Copy link
Contributor

Playground/RasterTile example:

  • Same behavior as above.
  • Tried a development build.
    • same behavior as above, except that the tab now uses almost 1.3GB memory
    • last message I see in the console:
    warning: 2 FS.syncfs operations in flight at once, probably just doing extra work
    

@wilhelmberg
Copy link
Contributor

I think the problem is with WWW.isDone in LoadMapboxAccess().

isDone never becomes true and also the backup timeout doesn't seem to kick in, so the app is caught in that loop.

If I remove the isDone check I get an UnityException: WWW is not ready downloading yet.

Even copying Unity's own Application.streamingAssetsPath example I get the same exception:

public class script1 : MonoBehaviour
{

	private string _accessPath;

	void Start()
	{

		_accessPath = Path.Combine(Application.streamingAssetsPath, "MapboxAccess.text");

#if !UNITY_ANDROID && !UNITY_WEBGL
		Debug.Log("called on (almost) all players");
#else
		Debug.Log("called on Android and WebGL");
#endif


#if UNITY_EDITOR || (!UNITY_ANDROID && !UNITY_WEBGL)
		Debug.Log("UNITY_EDITOR || (!UNITY_ANDROID && !UNITY_WEBGL)");
#else
		Debug.Log("ELSE - UNITY_EDITOR || (!UNITY_ANDROID && !UNITY_WEBGL)");
#endif

		Debug.Log("_accessPath:" + _accessPath);
		IEnumerator e = GetToken();
		e.MoveNext();
		e.MoveNext();
	}

	IEnumerator GetToken()
	{
#if UNITY_EDITOR || (!UNITY_ANDROID && !UNITY_WEBGL)
		yield return null;
		Debug.Log("token loaded (via 'File'): " + File.ReadAllText(_accessPath));
#else
		WWW www = new WWW(_accessPath);
		yield return www;
		Debug.Log("token loaded (via 'WWW'): " + www.text);
#endif
	}

}

@wilhelmberg
Copy link
Contributor

I think isDone and how we use it is the root of our problem.
Unity docs about WWW.isDone:

You should not write loops that spin until download is done; use coroutines instead.

While our approach works on Android it does not work with WebGL.


With the above patch in place I just hard coded my token and now WebGL works 🎉

So far tried:

  • RasterTile
  • Slippy

It's great to see memory caching working nicely with those two examples.

@david-rhodes
Copy link

@BergWerkGIS Did you try StartCoroutine(GetToken())?

@david-rhodes
Copy link

Also, now that I see this example, we might be able to greatly simplify this class (logic).

@wilhelmberg
Copy link
Contributor

wilhelmberg commented May 30, 2017

Did you try StartCoroutine(GetToken())?

Yes, tiles start loading before token is availabe -> 401s - Not authenticated.
I based GetToken on the example you linked.

@david-rhodes
Copy link

@BergWerkGIS

After fiddling around with this for a while, I think we have at least three possible paths forward:

  1. Cache requests until the file has been read then submit through the properly configured FileSource
  2. Add an Initialized event to MapboxAccess and expect clients to subscribe to that before making requests
  3. Instruct developers to hardcode their tokens in MapboxAccess, specifically for WebGL builds

The first option is more difficult, because geocoding and directions requests would need separate queueing systems. The second is less friendly, but generally safer.

Do you see other options?

@wilhelmberg
Copy link
Contributor

As the Unity docs (WebGL Networking) explicitly state, we shouldn't use !isDone():

Do not block on WWW or WebRequest downloads

Do not use code which blocks on a WWW or WebRequest download, like this:

while(!www.isDone) {}

Blocking on WWW or WebRequest downloads does not work on Unity WebGL. Because WebGL is single threaded, and because the XMLHttpRequest class in JavaScript is asynchronous, your download never finishes unless you return control to the browser; instead, your content deadlocks. Instead, use a Coroutine and a yield statement to wait for the download to finish.

I tried everything I could think of to make that WWW or UnityWebRequest call within a coroutine and make it sync, but failed.

Another hint from @david-rhodes 🙏 seems more promising: Resources.Load().

I tried Slippy example successfully with

  • Unity Editor
  • Windows native x64
  • WebGL
  • Android

Branch: https://github.com/mapbox/mapbox-unity-sdk/tree/BergWerkGIS-fix-mapboxaccess-for-webgl

Changes

  • moved StreamingAssets/MapboxAccess.text to Resources/MapboxAccess/
  • renamed Resources/MapboxAccess/MapboxAccess.text to MapboxAccess.txt to make Unity recognize it as a TextAsset
  • simplified MapboxAccess.cs

TODO

  • haven't commited the new .meta files as I'm on a Unity Beta version
  • change Editor dialog to save to Resources/MapboxAccess/MapboxAccess.txt

@wilhelmberg
Copy link
Contributor

Done: #109
Let's create dedicated issues if single examples fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants