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

'duplicate()' Function Bug #16

Closed
joshnice opened this issue Jul 20, 2020 · 11 comments
Closed

'duplicate()' Function Bug #16

joshnice opened this issue Jul 20, 2020 · 11 comments
Assignees
Labels
🪲 bug Something isn't working
Milestone

Comments

@joshnice
Copy link

Hi, as commented previously on a different post, I am currently in the process of updating from peterqliu's threebox library to this forked version, I think I may of come across a bug with the 'duplicate' function. Below is the JavaScript code from a modified example of logistics (also a google drive shareable link which contains the whole example HTML file), which duplicates a model and tires to call setCoords. However once setCoords is called an error is thrown in the console and the model is not added to the map.

Console errors:
duplicate_bug

Modified file: https://drive.google.com/file/d/1AOfWV_ISFghaDHDRJMsG6T8pIgj9Gtej/view?usp=sharing

Modified JavaScript:
`

          const origin = [-122.4340, 37.7353];

	var map = new mapboxgl.Map({
		container: 'map',
		style: 'mapbox://styles/mapbox/dark-v9',
		center: origin,
		zoom: 14,
		pitch: 60,
		bearing: 90
	});

	map.on('style.load', function () {

		map
			.addLayer({
				id: 'custom_layer',
				type: 'custom',
				renderingMode: '3d',
				onAdd: function (map, mbxContext) {

				window.tb = new Threebox(
					map,
					mbxContext,
					{ defaultLights: true }
				);

				var options = {
					type: 'gltf',
					// converted the truck example to glb but I seem to get the same error with obj files
					obj: 'models/truck.glb',
					scale: 10,
					units: 'meters',
					rotation: { x: 90, y: 180, z: 0 },
				}

				tb.loadObj(options, function (model) {
					truck = model.setCoords(origin);
					tb.add(truck);

					// const new_truck = truck; Works without duplication however only shows last added model
					const new_truck = truck.duplicate();
					tb.add(new_truck);
					// where the error is being thrown
					new_truck.setCoords([-122.4702, 37.6879]);
				})

				},
				render: function (gl, matrix) {
					tb.update();
				}
			});
	});`
@jscastro76
Copy link
Owner

Hi @joshnice
Let me take a look to it... it seems the duplicate method is definitely having an issue... are you using release v.2.0.3?

@joshnice
Copy link
Author

Hi @jscastro76, Yes I am.

@jscastro76 jscastro76 added the 🪲 bug Something isn't working label Jul 20, 2020
@jscastro76
Copy link
Owner

Clearly a bug, I can repro it too... I'm guessing the clone method is not doing what it should be as I had some issues with this method when I included CSS2DObjects that was accepted as new feature by THREE.js CSS2DObjects
I'll work on this asap

@joshnice
Copy link
Author

@JSCastro Ok, thank you very much.

@jscastro76
Copy link
Owner

@JSCastro Ok, thank you very much.

@joshnice good news, I have already identified what it is... the boundingBox created with BoxHelpers... I'll be trying to solve it this afternoon and I will push the change as soon it's ready to test. No worries, it seems simple.

@jscastro76
Copy link
Owner

Hi @joshnice, just an update on this... this is becoming serious and seems the bug is in THREE.js as long as I can repro the bug WITHOUT Threebox code!!
I'm preparing a minimum reproducible example to open the bug. It seems that when the code is minimized, Box3Helper is converted wrongly. I'm trying to gather all the info and prepare the bug repro before opening the bug.

@jscastro76
Copy link
Owner

jscastro76 commented Jul 21, 2020

Hi @joshnice , I have reported this at three.js repo Error "Cannot read property 'isEmpty' of undefined" when cloning a Geometry with a Box3Helper.
In the meantime, I'm implementing a workaround

@jscastro76 jscastro76 self-assigned this Jul 21, 2020
@jscastro76
Copy link
Owner

I found the solution, just adding a copy method to Box3Helper.prototype will solve the issue

	Box3Helper.prototype = Object.assign(Object.create(LineSegments.prototype), {
		constructor: Box3Helper,
		copy: function (source) {
			LineSegments.prototype.copy.call(this, source);
			this.box = source.box.clone();
			return this;
		}
	});

I'll wait for their reply because it seems a clear and easy bug and I provided also de fix. If they don't include it in an upcoming release I will add it manually to our thee.js bundle

jscastro76 added a commit that referenced this issue Jul 21, 2020
@jscastro76
Copy link
Owner

@joshnice I've just pushed the change in the three.js bundle and I rebuilt threebox.js and threebox.min.js

Could you please pull and check if it solves your issue?? I've done a local test where I reproduced your issue and it works

@jscastro76 jscastro76 added this to the v2.0.4 milestone Jul 21, 2020
@joshnice
Copy link
Author

@jscastro76 The fix has solved the bug for me! Thanks for your help.

@jscastro76
Copy link
Owner

Great to hear, let's wait for THREE.js guys to check if they accept it as a bug and apply the fix.

jscastro76 added a commit that referenced this issue Jul 25, 2020
Issues #13, #15, #16, #17, #18, #19, #20, #21, #22
@jscastro76 jscastro76 mentioned this issue Jan 13, 2021
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants