mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.72k stars 35.38k forks source link

ObjectLoader: Add support for helpers #15714

Open feus4177 opened 5 years ago

feus4177 commented 5 years ago
Description of the problem

If you serialize a Box3Helper object it drops the reference to box, which means that parsing it always returns the default Box3Helper. In the snippet below, it should appear as if there is only one box, but instead two boxes of different sizes show up.

Relevant snippet:

    var box = new THREE.Box3(
      new THREE.Vector3( -5, -5, -5 ),
      new THREE.Vector3( 5, 5, 5 ),
    );

    var helper = new THREE.Box3Helper( box, 0xffff00 );
    scene.add(helper);

    var string = JSON.stringify(helper);
    var loader = new THREE.ObjectLoader();
    var parsed = loader.parse(JSON.parse(string));
    scene.add(parsed);

https://codepen.io/anon/pen/rPzLPO

Three.js version
Browser
OS
Hardware Requirements (graphics card, VR Device, ...)
Mugen87 commented 5 years ago

var string = JSON.stringify(helper);

This is not the way objects are serialized. You have to do it like so:

var string = JSON.stringify( helper.toJSON() );

But even with this code it won't work since ObjectLoader does not support loading helpers. If you inspect the type of your deserialized object, you will see it's LineSegments.

feus4177 commented 5 years ago

Doesn't JSON.stringify call toJSON on objects internally? Either way, it looks like JSON.stringify(helper) and JSON.stringify(helper.toJSON()) output the same string.

I would expect the deserialized object to be of type LineSegments as that is what I'm trying to create. If you look at helper.type, you'll notice it is also LineSegments.

If helpers are not supposed to be serialized you can go ahead and close this issue. Since, it was giving me close to the right output I thought perhaps it was supposed to be supported. The documentation around serialization/parsing is a little sparse so I just wasn't sure.

Mugen87 commented 5 years ago

Doesn't JSON.stringify call toJSON on objects internally?

Ups, you're right. Totally missed the automatic invocation of toJSON(). Thanks for pointing this out! 👍

If helpers are not supposed to be serialized you can go ahead and close this issue.

There were lately some other issues in context of helpers. Maybe the support in ObjectLoader is reconsider at some point.

If you look at helper.type, you'll notice it is also LineSegments.

I'm not sure but I think that is not intended. It should be Box3Helper which is actually set at the top of the respective ctor.

Mugen87 commented 5 years ago

Let's turn this issue into a more concrete feature request: Add support for helpers in ObjectLoader.

@feus4177 Is this okay for you? 😉

feus4177 commented 5 years ago

Yeah, that's fine and I modified the title accordingly.

In terms of helper.type, I noticed that in the Box3Helper it calls the LineSegments constructor here. I'm guessing that call overwrites helper.type. I'm not sure if this is the desired behavior though.