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

Update to support newer Three.js API and Blender-exported models #6

Merged
merged 1 commit into from Jul 25, 2017

Conversation

@bcongdon
Copy link
Contributor

@bcongdon bcongdon commented Jul 22, 2017

Thanks for making this module! Saved me a ton of headaches. 😁

Seems like it needs a bit of updates to support the newest THREE.js, and I noticed there were a couple bugs that prevented me from loading a Blender-exported X3D file.

I've tested these changes against a couple of my own models, but I'd welcome further scrutiny.

@@ -30,8 +30,7 @@ module.exports = function (THREE) {

var scope = this;

var loader = new THREE.XHRLoader(this.manager);

This comment has been minimized.

@bcongdon

bcongdon Jul 22, 2017
Author Contributor

XHRLoader has been deprecated in favor of FileLoader.

setCrossOrigin has also been removed (mrdoob/three.js@a88a3e6#diff-5705be950df59ac0364d8ffe954313f6)

@@ -288,7 +288,7 @@ function renderX3D(THREE, x3dXml, scene, useImageTexture) {
var groundGeometry = new THREE.SphereGeometry(radius, segments, segments, 0, 2 * Math.PI, 0.5 * Math.PI, 1.5 * Math.PI);
var groundMaterial = new THREE.MeshBasicMaterial({ fog: false, side: THREE.BackSide, vertexColors: THREE.VertexColors });

paintFaces(groundGeometry, radius, data.groundAngle, data.groundColor, false);

This comment has been minimized.

@bcongdon

bcongdon Jul 22, 2017
Author Contributor

This breaks when a Background tag is passed with a ground color without a ground angle. Adding a default to prevent this

@@ -545,19 +545,24 @@ function renderX3D(THREE, x3dXml, scene, useImageTexture) {

var tree = { 'string': 'Scene', children: [] };

parseChildren(x3dXml.documentElement.childNodes[1], tree);

This comment has been minimized.

@bcongdon

bcongdon Jul 22, 2017
Author Contributor

The Scene tag isn't necessarily at index 1. For example, the following is Blender-exported X3D. It's better to validate that we're actually finding the Scene before beginning traversing the Scene

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE X3D PUBLIC "ISO//Web3D//DTD X3D 3.0//EN" "http://www.web3d.org/specifications/x3d-3.0.dtd">
<X3D version="3.0" profile="Immersive" xmlns:xsd="http://www.w3.org/2001/XMLSchema-instance" xsd:noNamespaceSchemaLocation="http://www.web3d.org/specifications/x3d-3.0.xsd">
    <head>
        <meta name="filename" content="tmpsnz74v00.x3d" />
        <meta name="generator" content="Blender 2.78 (sub 5)" />
    </head>
    <Scene>
        ...
    </Scene>
</X3D>
var newChild = {
'nodeType': currentNode.nodeName.toLocaleLowerCase(),
'string': getNodeGroup(currentNode.nodeName) + ' ' + currentNode.nodeName.toLocaleLowerCase(),

This comment has been minimized.

@bcongdon

bcongdon Jul 22, 2017
Author Contributor

I'm not sure if this is a bug in the initial implementation, or if there are just varying version of the X3D spec, but this code was failing to find the contents / tags of USE and DEF attributes.

In my X3D models, I was able to fix this by directly adding the attributes like this.

@jonaskello
Copy link
Member

@jonaskello jonaskello commented Jul 25, 2017

Thanks for the PR! Since we don't have any tests in this repo I'm a bit unsure about merging stuff but you changes seem small and well documented. I also ran the example and it works with your PR so I'll go ahead and merge this.

@jonaskello jonaskello merged commit 49eb31b into dividab:master Jul 25, 2017
@jonaskello
Copy link
Member

@jonaskello jonaskello commented Jul 25, 2017

Published as 0.4.0.

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.