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

Refresh of map after map style change #35

Closed
ShaneTex opened this issue Aug 9, 2020 · 14 comments
Closed

Refresh of map after map style change #35

ShaneTex opened this issue Aug 9, 2020 · 14 comments
Assignees
Labels
🪲 bug Something isn't working 🔍 needs investigation 🥇 good first issue Good for newcomers
Milestone

Comments

@ShaneTex
Copy link

ShaneTex commented Aug 9, 2020

If I change the map.setStyle using standard mapbox tools, I loose all objects that have been added to the map. Is there a way to refresh via tb.update(); on a reload of the page after a setStyle. The only way I could get it to work was add a simple feature within style.load and then do the tb.update() within the render property. Then I get all my previous custom objects back on the map.

Is there a way to use the tb.update() outside of render on the style.load by it self.

@jscastro76
Copy link
Owner

Hi @ShaneTex, do you have a repro of this issue? I am working on v.2.0.5. adding an example on change style and it works perfectly

@ShaneTex
Copy link
Author

@jscastro76 This is what I have: If I select one of my change style buttons up in the upper right corner, and I use the normal on style.load, I get 2 versions of the same models, if I add a check to see if it was loaded, then the original don't render.

Here is one that doesn't work:
http://www.septrio.com/threebox/examples/refreshfails.html#19.03/33.1941965/-96.6131104/-140.8/53

Here is one that works by adding a small 3d graphic and then render, but I don't thing it is good practice:
http://www.septrio.com/threebox/examples/refreshfails.html#19.17/33.1938477/-96.6131324/-160.8/60

@jscastro76
Copy link
Owner

Hi @ShaneTex ,
I took a look to your code but I wasn't able to identify what is wrong.
But I'm about to push v.2.0.5. in the next coming days, and I have built an example on how to change style with the Eiffel tower. Indeed, as the Eiffel tower is cached the first time, you'll see it before the map is reloaded with the new style.

Just copy and paste this full HTML+js on your threebox /examples cloned folder. This code is using Threebox v.2.0.4.

<!doctype html>
<html>
<head>
	<title>Change style map for Eiffel Tower</title>
	<script src="https://api.mapbox.com/mapbox-gl-js/v1.11.1/mapbox-gl.js"></script>
	<link href="https://api.mapbox.com/mapbox-gl-js/v1.11.1/mapbox-gl.css" rel="stylesheet" />
	<script src="../dist/threebox.js" type="text/javascript"></script>
	<link href="./css/threebox.css" rel="stylesheet" />

	<script src="config.js"></script>

	<style>
		body {
			margin: 0;
			padding: 0;
		}

		html, body, #map {
			height: 100%;
		}

		#menu {
			position: absolute;
			background: #fff;
			padding: 10px;
			font-family: 'Open Sans', sans-serif;
		}
	</style>
</head>
<body>
	<div id='map' class='map'></div>
	<div id="menu">
		<input id="streets-v11" type="radio" name="rtoggle" value="streets" checked="checked" />
		<label for="streets-v11">streets</label>
		<input id="light-v10" type="radio" name="rtoggle" value="light" />
		<label for="light-v10">light</label>
		<input id="dark-v10" type="radio" name="rtoggle" value="dark" />
		<label for="dark-v10">dark</label>
		<input id="outdoors-v11" type="radio" name="rtoggle" value="outdoors" />
		<label for="outdoors-v11">outdoors</label>
		<input id="satellite-v9" type="radio" name="rtoggle" value="outdoors" />
		<label for="satellite-v9">satellite</label>
	</div>
	<script>
		// This example downloads a tower model from an external OBJ/MTL file, adds it to the map, and drives it around via paths fetched from the Mapbox Directions API

		if (!config) console.error("Config not set! Make a copy of 'config_template.js', add in your access token, and save the file as 'config.js'.");

		mapboxgl.accessToken = config.accessToken;

		let mapConfig = {
			PAR: { origin: [2.294514, 48.857475, 0], center: [2.294625, 48.861085, 0], zoom: 15.95, pitch: 60, bearing: 0, scale: 0.01029, timezone: 'Europe/Paris' },
			names: {
				customLayer: "custom-layer"
			}
		}

		let map = window.map = new mapboxgl.Map({
			container: 'map',
			zoom: mapConfig.PAR.zoom,
			pitch: mapConfig.PAR.pitch,
			bearing: mapConfig.PAR.bearing,
			center: mapConfig.PAR.center,
			style: 'mapbox://styles/mapbox/streets-v11',
			hash: true
		});

		window.tb = new Threebox(
			map,
			map.getCanvas().getContext('webgl'),
			{
				defaultLights: true
			}
		);

		let layerId = "streets-v11";
		function switchLayer(layer) {
			if (layerId != layer.target.id) {
				layerId = layer.target.id;
				map.setStyle('mapbox://styles/mapbox/' + layerId);
			}
		}
		let styleList = document.getElementById('menu');
		let inputs = styleList.getElementsByTagName('input');
		for (let i = 0; i < inputs.length; i++) {
			inputs[i].onclick = switchLayer;
		}

		map.on('style.load', () => {
			map.addLayer({
				id: mapConfig.names.customLayer,
				type: 'custom',
				renderingMode: '3d',
				onAdd: function (map, mbxContext) {
					options = {
						obj: './models/eiffel.glb',
						type: 'gltf',
						scale: mapConfig.PAR.scale,
						units: 'meters',
						//adjustment: { x: -0.5, y: -0.5, z: 0 } // model center is displaced
					}

					tb.loadObj(options, function (model) {
						model.setCoords(mapConfig.PAR.origin);
						model.setRotation({ x: 0, y: 0, z: 45.7 });
						model.addTooltip("Eiffel Tower", true);
						model.castShadow = true;
						tb.add(model);
					})

				},

				render: function (gl, matrix) {
					tb.update();
				}
			});

		});

	</script>
</body>
</html>
Streets style Satellite style
image image

@jscastro76
Copy link
Owner

@ShaneTex the mistake is in your code with the variable styleLoaded, you set it to true after the first style.load so it doesn't load once you click on the top right control for changing the style. I just changed the value in runtime, and you page now shows the satellite style with your models
image

@jscastro76 jscastro76 added the ⛔ wontfix This will not be worked on label Aug 10, 2020
@ShaneTex
Copy link
Author

@jscastro76 Yes, I added that styleLoaded because it was duplicating all the models, if you look at those green drone coins, you see now that they are duplicated, or double what they were to start with.

@ShaneTex
Copy link
Author

@jscastro76 Ok, I just added this to your example:

enableSelectingFeatures: true, //change this to false to disable fill-extrusion features selection
enableSelectingObjects: true, //change this to false to disable 3D objects selection
enableDraggingObjects: true, //change this to false to disable 3D objects drag & move once selected
enableRotatingObjects: false, //change this to false to disable 3D objects rotation once selected
enableTooltips: true, // change this to false to disable default tooltips on fill-extrusion and 3D models
outlinePass: false

Then after I switch style, then drag the eiffel tower, I have 2 now.

@jscastro76 jscastro76 added 🪲 bug Something isn't working 🔍 needs investigation and removed ⛔ wontfix This will not be worked on labels Aug 10, 2020
@jscastro76
Copy link
Owner

jscastro76 commented Aug 10, 2020

Let me check that out!
The layer is definitely removed on map.setStyle, otherwise we will be receiving an error because of the creation of a second layer with the same name. But the models stay at tb.world as children... so they are unlinked from their original layer (that’s pretty curious)
I think we need to clear tb.world before or after setStyle.
I’m going to implement a tb.clear but also a tb.setStyle to override the map one ensuring all the objects of the layer are removed when the layer is.

@jscastro76 jscastro76 reopened this Aug 10, 2020
@ShaneTex
Copy link
Author

sounds good

@jscastro76 jscastro76 added this to the v2.0.5. milestone Aug 10, 2020
@jscastro76 jscastro76 self-assigned this Aug 10, 2020
@jscastro76 jscastro76 added the 🥇 good first issue Good for newcomers label Aug 10, 2020
@jscastro76
Copy link
Owner

jscastro76 commented Aug 11, 2020

@ShaneTex, good news, I got it!

As said, once setStyle is called all the layers are removed, included the custom one, but the 3D objects still at tb.world.

So there are not too many changes needed, and I have already implemented this bug for v.2.0.5 within a new method tb.setStyle. I'll be delivering the new version by the end of this week. In the meantime, if you cannot wait, you can copy this method:

function clear () {
    tb.world.traverse(function (obj) {
        tb.remove(obj);
    });
}

You have to call it after the setStyle, and remove the variable styleLoaded and its check to true in style.load, it should work.

@ShaneTex
Copy link
Author

@jscastro76 Ok, I try that and it tells me this: mckinneyevents.html:633 Uncaught TypeError: Cannot read property 'traverse' of undefined, i tried chaning this to tb. as well and get the same errir.

@jscastro76
Copy link
Owner

@jscastro76 Ok, I try that and it tells me this: mckinneyevents.html:633 Uncaught TypeError: Cannot read property 'traverse' of undefined, i tried chaning this to tb. as well and get the same errir.

Sorry, there was a mistake in my answer it’s tb.world.traverse not this.world.traverse. I copied directly the method from my threebox code

@jscastro76
Copy link
Owner

Here is the full test page, comment or uncomment line with the clear() call to see the effect. If you have declared Threebox at window level, it should work.

<!doctype html>
<html>
<head>
	<title>Change style map for Eiffel Tower</title>
	<script src="https://api.mapbox.com/mapbox-gl-js/v1.11.1/mapbox-gl.js"></script>
	<link href="https://api.mapbox.com/mapbox-gl-js/v1.11.1/mapbox-gl.css" rel="stylesheet" />
	<script src="../dist/threebox.js" type="text/javascript"></script>
	<link href="./css/threebox.css" rel="stylesheet" />

	<script src="config.js"></script>

	<style>
		body {
			margin: 0;
			padding: 0;
		}

		html, body, #map {
			height: 100%;
		}

		#menu {
			position: absolute;
			background: #fff;
			padding: 10px;
			font-family: 'Open Sans', sans-serif;
		}
	</style>
</head>
<body>
	<div id='map' class='map'></div>
	<div id="menu">
		<input id="streets-v11" type="radio" name="rtoggle" value="streets" checked="checked" />
		<label for="streets-v11">streets</label>
		<input id="light-v10" type="radio" name="rtoggle" value="light" />
		<label for="light-v10">light</label>
		<input id="dark-v10" type="radio" name="rtoggle" value="dark" />
		<label for="dark-v10">dark</label>
		<input id="outdoors-v11" type="radio" name="rtoggle" value="outdoors" />
		<label for="outdoors-v11">outdoors</label>
		<input id="satellite-v9" type="radio" name="rtoggle" value="outdoors" />
		<label for="satellite-v9">satellite</label>
	</div>
	<script>
		// This example downloads a tower model from an external OBJ/MTL file, adds it to the map, and drives it around via paths fetched from the Mapbox Directions API

		if (!config) console.error("Config not set! Make a copy of 'config_template.js', add in your access token, and save the file as 'config.js'.");

		mapboxgl.accessToken = config.accessToken;

		let mapConfig = {
			PAR: { origin: [2.294514, 48.857475, 0], center: [2.294625, 48.861085, 0], zoom: 15.95, pitch: 60, bearing: 0, scale: 0.01029, timezone: 'Europe/Paris' },
			names: {
				customLayer: "custom-layer"
			}
		}

		let map = window.map = new mapboxgl.Map({
			container: 'map',
			zoom: mapConfig.PAR.zoom,
			pitch: mapConfig.PAR.pitch,
			bearing: mapConfig.PAR.bearing,
			center: mapConfig.PAR.center,
			style: 'mapbox://styles/mapbox/streets-v11',
			hash: true
		});

		window.tb = new Threebox(
			map,
			map.getCanvas().getContext('webgl'),
			{
				defaultLights: true,
				enableSelectingObjects: true,
				enableDraggingObjects: true
			}
		);

		let layerId = "streets-v11";
		function switchLayer(layer) {
			if (layerId != layer.target.id) {
				layerId = layer.target.id;
				map.setStyle('mapbox://styles/mapbox/' + layerId);
				clear(); // comment this so see the effect
			}
		}

		function clear () {
			tb.world.traverse(function (obj) {
				tb.remove(obj);
			});
		}

		let styleList = document.getElementById('menu');
		let inputs = styleList.getElementsByTagName('input');
		for (let i = 0; i < inputs.length; i++) {
			inputs[i].onclick = switchLayer;
		}

		map.on('style.load', () => {
			map.addLayer({
				id: mapConfig.names.customLayer,
				type: 'custom',
				renderingMode: '3d',
				onAdd: function (map, mbxContext) {
					options = {
						obj: './models/eiffel.glb',
						type: 'gltf',
						scale: mapConfig.PAR.scale,
						units: 'meters',
						//adjustment: { x: -0.5, y: -0.5, z: 0 } // model center is displaced
					}

					tb.loadObj(options, function (model) {
						model.setCoords(mapConfig.PAR.origin);
						model.setRotation({ x: 0, y: 0, z: 45.7 });
						model.addTooltip("Eiffel Tower", true);
						model.castShadow = true;
						tb.add(model);
					})

				},

				render: function (gl, matrix) {
					tb.update();
				}
			});

		});

	</script>
</body>
</html>

@ShaneTex
Copy link
Author

ShaneTex commented Aug 11, 2020

@jscastro76 It is interesting, I made those changes, and it still does that or I get the same error.
If it is fixed in the new release, no worries, I can wait.

@jscastro76
Copy link
Owner

Ok, let me try to finish the rest of the release changes and I'll try to push the commit asap

jscastro76 added a commit that referenced this issue Aug 14, 2020
Enhacements: #28, #30, #31, #33, #34, #36, #37
Bugs: #32, #35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working 🔍 needs investigation 🥇 good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants