Skip to content

Conversation

@kurozael
Copy link

This small fix makes sure that some loaded GLTF models work with other basic mesh types without causing the strange error referenced in this issue: #12380

@mrdoob
Copy link
Owner

mrdoob commented Jun 26, 2018

Just want to double check... GLTFLoader is producing buffergeometries that have geometry.index pointing to a attribute that is undefined?

@mrdoob
Copy link
Owner

mrdoob commented Jun 26, 2018

/cc @donmccurdy @takahirox

@kurozael
Copy link
Author

Hi @mrdoob this could very well be possible, and seems to be what causes the error. A simple undefined check fixes this, and the models load absolutely fine. What's strange, is that this bug does not occur just by loading the GLTF models themselves, I must additionally create a standard Plane mesh before the error occurs.

@mrdoob
Copy link
Owner

mrdoob commented Jun 26, 2018

What's strange, is that this bug does not occur just by loading the GLTF models themselves, I must additionally create a standard Plane mesh before the error occurs.

Oh! I would prefer to understand exactly what is happening before blindly adding a undefined check...

@kurozael
Copy link
Author

kurozael commented Jun 26, 2018

I understand your concern @mrdoob, and I can assure you that my code is not changing anything to undefined. So whilst I am not sure how this is happening when the GLTF model is loaded and another standard mesh is created purely with Three.js, this simple undefined check does fix it without any other symptoms.

Please feel free to investigate the "why", but in the mean time if this fix could be implemented with at least a warning instead of an error that would be great!

If you want to investigate further, whilst I couldn't create a JSFiddle demonstrating the bug, the project files at https://github.com/mrdoob/three.js/files/1372366/test.zip do demonstrate the problem and should be relatively easy for you to debug.

@mrdoob
Copy link
Owner

mrdoob commented Jun 27, 2018

@donmccurdy @takahirox any ideas guys?

@mrdoob mrdoob added this to the r95 milestone Jun 27, 2018
@takahirox
Copy link
Collaborator

Sorry, I've been a bit busy. I'll look into soon.

@donmccurdy
Copy link
Collaborator

I can't see a problem with that model, even after adding a mesh with a PlaneGeometry to the scene. The model contains 86 meshes, all with geometry.index defined. We'll need a JSFiddle showing the issue I think, otherwise I don't see a reason to merge a check looking specifically for one of many invalid things geometry.index could be set to.

@kurozael
Copy link
Author

kurozael commented Jun 27, 2018

Hi @donmccurdy,

Well, this issue has happened to at least 2 of your users... if a simple undefined check fixes it, I don't see the harm in implementing this fix and at the very least just throwing up a warning if it isn't defined.

This issue is definitely occurring for me, I'm unable to get it to occur with JSFiddle, but that doesn't mean it isn't happening. I'm happy to provide a video recording of the error occurring if that helps. It's going to be a lot of trouble for us to continue using Three.js for our project if we have to manually apply this fix every time we update.

Please let me know any other way I can help to demonstrate the problem to you, if that's what it takes to get this check merged in. It's very important to us as we are very excited about the prospect of using Three.js for our upcoming project.

Many thanks!

@mrdoob
Copy link
Owner

mrdoob commented Jun 27, 2018

Well, this issue has happened to at least 2 of your users... if a simple undefined check fixes it, I don't see the harm in implementing this fix and at the very least just throwing up a warning if it isn't defined.

I'm sorry, we can't just add checks just in case. If we did that kind of things the code would be much harder to read and understand.

This issue is definitely occurring for me, I'm unable to get it to occur with JSFiddle, but that doesn't mean it isn't happening.

If you can't reproduce with JSFiddle it may be a bug in your own code.

I'm happy to provide a video recording of the error occurring if that helps.

Anything helps.

@kurozael
Copy link
Author

kurozael commented Jun 28, 2018

I will try to get a video of it tomorrow at work, I don't think it's a bug in my own code because but it might be environmental. For example the original reporter of this bug included his project and it was producing the error for him, similarly the error produces on my machine with the same code I tried putting into JSFiddle. This is on Chrome.

Just to be clear, everything works fine with GLTF loading, until I add this code...

        let brandingTexture = textureLoader.load("./table/switch.png");
        let geometry = new THREE.PlaneGeometry(300, 300);
        let material = new THREE.MeshBasicMaterial({
            map: brandingTexture,
            side: THREE.DoubleSide,
            transparent: true
        });
        let plane = new THREE.Mesh(geometry, material);

        this.threeLayer.scene3D.add(plane);

The GLTF model is also added to "this.threeLayer.scene3D". For some reason having both causes that error to occur, I can have one or the other, but both at the same time doesn't work.

So I don't see how my code could be causing the bug.

I can also tell you from my logging that I iterate through every single BufferGeometry that gets loaded and they all have index set to a BufferAttribute whether or not the Plane is created. So somehow, even though index is always set to a valid attribute, the error is still occuring. This is because of this line:

attribute = attributes.get( index );

The attributes (WebGLAttributes) get method is returning undefined for the index, even though the index is a valid BufferAttribute. This implies to me that for some reason WebGLAttributes.get is not returning the right value sometimes. But how this is dependent on whether or not a Plane is created, absolutely baffles me.

It would seem to me that you guys are far more likely to be able to investigate this with good measure than I am, seeing as how this is your code base :)

Thanks again!

Edit: Where do the attributes even get added to the WebGLAttributes.buffers WeakMap for the renderer?

@mrdoob
Copy link
Owner

mrdoob commented Jul 2, 2018

@kurozael any chance you can create a jsfiddle?

@kurozael
Copy link
Author

kurozael commented Jul 2, 2018

Hi @mrdoob,

I created a JSFiddle, however the issue does not manifest itself there. What I cannot explain, and maybe you guys can help, is why even when .index is set to valid BufferAttribute values, sometimes the WebGLAttributes.get method can return undefined when passed in a value that is valid.

Leads me to believe that in some circumstances, the WebGLAttributes are not properly updated or in sync. It would be good practice to throw in a check to ensure that the returned value is not undefined, at the very least, to prevent this bug from occuring.

I appreciate that you are unable to reproduce it, but my team is unable to use Three.js to complete our project while this bug persists and it is a serious blocker for us. We can use a patched version, but this will present problems for us down the line.

Edit: I've updated my local to use the newly release r94 and the bug no longer occurs. Very strange.

@mrdoob
Copy link
Owner

mrdoob commented Jul 2, 2018

Edit: I've updated my local to use the newly release r94 and the bug no longer occurs. Very strange.

I see... Should this be closed then?

@kurozael
Copy link
Author

kurozael commented Jul 2, 2018

Sure @mrdoob, if the issue comes up again in a future version we can re-evaluate it. Thanks!

@kurozael kurozael closed this Jul 2, 2018
@DerKorb
Copy link

DerKorb commented Oct 18, 2018

I am having the same problem with R97 (attributes.get returning undefined).
Additionally I have behavior where one mesh uses the geometry of another.
I have three basic meshes:

    const vis = new THREE.Object3D();
    vis.add(new THREE.Mesh(
      new THREE.BoxGeometry(50, 30, 30),
      new THREE.MeshBasicMaterial({ color: "red", wireframe: true }),
    ));
    const vis2 = new THREE.Object3D();
    vis2.add(new THREE.Mesh(
      new THREE.SphereGeometry(14),
      new THREE.MeshBasicMaterial({ color: "blue", wireframe: true }),
    ));
    this.node.add(vis);
    this.node.add(vis2);
    const vis2 = new THREE.Object3D();
    vis2.userData = {a: 1};
    vis2.add(new THREE.Mesh(
       new THREE.BoxGeometry(140, 20, 20),
       new THREE.MeshBasicMaterial({ color: "yellow", wireframe: true }),
    ));
    object2.add(vis2);
    return object2;

I made a video of the behaviour
The blue sphere is rendered correctly. The yellow boxes are never rendered with their own geometry, depending on camera position (I am using OrbitControls) they are either not visible or rendered with the red meshes geometry. I also noticed once that changing visible to false then true on an object that it changed its geometry.
I very thoroughly checked my full scene tree, every object got the right geometry there.
It looks to me like attributes.get may in some situations return the wrong attribute.
Unfortunately this is part of quite a huge React app and I was not able to isolate the problem into a small fiddle.

@DerKorb
Copy link

DerKorb commented Oct 18, 2018

I made a second video to show more camera movement / culling? related behaviour.

@DerKorb
Copy link

DerKorb commented Oct 18, 2018

Also I just noticed getting this error:
[.WebGL-0x56206ada00d0]GL ERROR :GL_INVALID_OPERATION : glDrawElements: attempt to access out of range vertices in attribute 0

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 18, 2018

@DerKorb Please try to isolate the issue in a live example for debugging. Besides, I think it's better if you open a new issue which is focused on the view-dependent rendering problem.

Your code seems a bit confusing to me. You declare vis2 two times although it's a constant. This should actually produce a syntax error (Identifier 'vis2' has already been declared).

@DerKorb
Copy link

DerKorb commented Oct 18, 2018

You are right, i just pasted the second vis object into the same block. In my original code I had it in two different places.

I found the reason for all the problems. I am using webpack to load some of my code from a custom npm module. Now when I imported THREE once in the module code and once in my app, webpack did create two instances of THREE. Without deep knowledge of THREE I assume both instances did have their own WebGL Context/Handle/whatever. So when I handed over geometries that had been allocated in one context to the renderer from the second context, attributes.get would somehow either get undefined or a reference to some random other geometry created in the same context as the renderer.

@malcolmwhite
Copy link

@DerKorb Thanks for sharing your fix -- just saved me some time on similar behavior!

@rohan-deshpande
Copy link
Contributor

rohan-deshpande commented Mar 17, 2019

@DerKorb thanks so much for this - seriously. In short - don't ever use two instances of three in the same app. Bad things can happen. Here was my use case

  1. My module depends on three and I am bundling with webpack
  2. In my examples, which are just html files with script tags, I load my bundled module, which uses three internally, but doesn't export it. This is done via a script tag
  3. I include three via a script tag also, because I need to setup three stuff like a scene, renderer etc.,
  4. Now, when using a texture loader and doing things asynchronously, I get the following stack trace
Uncaught TypeError: Cannot read property 'type' of undefined
    at WebGLIndexedBufferRenderer.setIndex (three.js:15321)
    at WebGLRenderer.renderBufferDirect (three.js:22689)
    at renderObject (three.js:23343)
    at renderObjects (three.js:23313)
    at WebGLRenderer.render (three.js:23120)

I did a small experiment to export three from my module, then use it in my example like const { THREE } = myModule

And everything works. I would be very curious to know what is happening here, any ideas @mrdoob? Some kind of WebGL context confusion?

@mrdoob
Copy link
Owner

mrdoob commented Mar 17, 2019

And everything works. I would be very curious to know what is happening here, any ideas @mrdoob? Some kind of WebGL context confusion?

No idea... Having a hard time to follow what it is you're trying to do. Any chance you can create a jsfiddle that shows the issue?

@DerKorb
Copy link

DerKorb commented Mar 17, 2019

As far as I see it:

  • webpack is causing the error by creating two seperate instances of the threejs source
  • both instances then initiate a WebGL context on their own, as they do not know from each other
  • your code exchanges objects from one instance to the other instance
  • the webGL core functions wont work right, as you give them an index only valid in the second context
  • wierd stuff happens

@mrdoob
Copy link
Owner

mrdoob commented Mar 17, 2019

😵

@rohan-deshpande
Copy link
Contributor

rohan-deshpande commented Mar 18, 2019

@DerKorb interesting. My use case is similar but perhaps slightly different, it's been challenging to set up an example see #14367 (comment) for examples. I still don't quite understand what is happening though. Let me try to explain a bit and give some light code examples.

  1. I am creating a particle system engine as a module that depends on three, here is some sample code
import { AdditiveBlending } from 'three';

export const DEFAULT_MATERIAL_PROPERTIES = {
  color: 0xff0000,
  blending: AdditiveBlending,
  fog: true,
};
  1. I bundle the module using webpack
/*global __dirname, require, module*/

const path = require('path');

let libraryName = 'ParticleSystemEngine';

let plugins = [],
  outputFile;

outputFile = 'particle-system-engine.js';

const config = {
  entry: __dirname + '/src/index.js',
  devtool: 'source-map',
  output: {
    path: __dirname + '/build',
    filename: outputFile,
    library: libraryName,
    libraryTarget: 'umd',
    umdNamedDefine: true,
  },
  module: {
    rules: [
      {
        test: /(\.jsx|\.js)$/,
        loader: 'babel-loader',
        exclude: /(node_modules)/,
      },
      {
        test: /(\.jsx|\.js)$/,
        loader: 'eslint-loader',
        exclude: /node_modules/,
      },
    ],
  },
  resolve: {
    modules: [path.resolve('./src'), path.resolve('./node_modules')],
    extensions: ['.json', '.js'],
  },
  plugins: plugins,
};

module.exports = config;
  1. I am creating examples on how to use my engine with straight HTML, so the script tags look like
    <script src="../js/three.min.js"></script>
    <script src="../js/three-proton.js"></script>

I do this because I need to create a Scene to pass to my engine, it renders particles into that scene, but it also uses three internally. I'm aware that in this use case, there may be differences in the versions of three being used by the HTML and by the module, but that is not the case for me, both are locked to r98 for now.

  1. Every single one of my examples works, except for one. In this example, I load my particle textures asynchronously, everything is wrapped in promises that only resolve when all textures are loaded. This is the example which causes the above error.

  2. I have resolved this by changing my module's index.js to

import * as THREE from 'three';

import { ParticleSystemEngine } from './core';

export * from './behaviour';
export * from './debug';
export * from './ease';
export * from './emitter';
export * from './initializer';
export * from './math';
export * from './renderer';
export * from './utils';
export * from './zone';
export { ParticleSystemEngine, Particle, Pool } from './core';
export default ParticleSystemEngine;
export { THREE }; // note this export

Then in my example, instead of using a script tag to load three I do the following (pseudo code)

<script src="../js/particle-system-engine.js"></script> 
<script>
  let particleSystem;
  const { THREE } = ParticleSystemEngine; // this works because it was exported from my module
  const scene = new THREE.Scene();
  
  ParticleSystemEngine.fromJSONAsync(jsonContainingTexturesToLoad)
    .then(engine => {
      particleSystem = engine; 
    })
    .catch(console.error);
</script>

This resolves the error. But I am confused as to why. I do not set up a WebGLRenderer in my module. The only external dependency that my engine requires as far as rendering goes is an Object3D to push children into. There should only, in theory, be one three accessing the WebGL context. This is what I thought anyway, I guess I am wrong.

Sorry for the essay! I think webpack is definitely at fault here, but I also am wondering about the internals of three and how this kind of error could occur.

@donmccurdy
Copy link
Collaborator

For another simple case I've seen fail:

if ( object instanceof Mesh ) {

  // ...

}

If there are two copies of three.js in the page, there are two copies of Mesh, Texture, and other classes. This will cause instanceof checks to fail pretty badly if the wrong thing gets passed in or cached somewhere.

@rohan-deshpande
Copy link
Contributor

rohan-deshpande commented Mar 19, 2019

@mrdoob Here's some codesandboxes

  • Working
    • I use THREE internally in my module as a dependency
    • I export THREE from my module entry and use that in app.js instead of loading it via a script tag
  • Broken
    • I use THREE internally in my module as a dependency
    • I load THREE for use in app.js via a script tag
  • Still Broken
    • Only difference here is removing all import * as THREE imports from my module, I thought this might somehow be causing a problem.

I tried switching out my module bundler from webpack to parcel and it didn't resolve the issue.

Also, would #12380 be a better place to track this? At the moment my only resolution is to drop script tag support for my module, which I am uncomfortable with, but can make do for now. This will mean that consumers will simply have to be in a transpile targeting environment in order to use my code.

@rohan-deshpande
Copy link
Contributor

rohan-deshpande commented Mar 21, 2019

Okay I had a hunch about this and was able to fix it. Posting the solution here for anyone else who comes across this error.

I created an internal compatibility module that conditionally requires three. All my internal app code imports from this module and does not use import * as THREE from 'three' Here's the module:

const isBrowser = typeof window !== 'undefined' ? true : false;
const THREE = isBrowser && window.THREE ? window.THREE : require('three');

export const {
  BoxGeometry,
  Euler,
  Geometry,
  Mesh,
  MeshBasicMaterial,
  MeshLambertMaterial,
  OctahedronGeometry,
  SphereGeometry,
  Sprite,
  SpriteMaterial,
  Texture,
  TextureLoader,
  Vector3,
  AdditiveBlending,
  CustomBlending,
  MultiplyBlending,
  NoBlending,
  NormalBlending,
  SubtractiveBlending,
} = THREE;

My tests pass in a node environment and my issue is now fixed if you use script tags.

@oparisy
Copy link
Contributor

oparisy commented Mar 24, 2019

Thanks to everyone sharing their experience here! I had the same issue, which boiled to a forgotten <script> import of three. The module and non-module version should definitely not be mixed.

@danvim
Copy link

danvim commented May 4, 2019

I just debugged my app for a whole night.

Reason

As @donmccurdy and @rohan-deshpande have said, the whole thing fails when there are multiple threes on the scene.

So I'm using a library https://github.com/vue-gl/vue-gl which in the package.lock has declared its three@^0.103.0 as a dependency. I then went ahead and npm i three on my own to load my GLTF models, to inject into vue-gl. Then the exact problem appeared, only when meshes from different THREEs exist together does the error occur.

Solution

I replaced all my import * as THREE from 'three'; with import * as THREE from 'vue-gl/node_modules/three'; (I call this a hack.)

P.S. This reminded me of a similar problem I had with jquery, because my bootstrap package also carried a jquery. I forgot how I managed to integrate that with my other modules, but it was a mess because I didn't know why one jquery scope lacked bootstrap APIs and the other lacked something else. How are packages like that supposed to be designed to have a single instance? Honestly curious.

@AlexeyKrotkov
Copy link

AlexeyKrotkov commented Jun 16, 2019

I found another solution for my react app (create-react-app without eject) with typescript.
Maybe it's not so cool, but it can be point for finding out.

  1. I create module 'three-exp' that exports THREE and binds to window (to export loaders in old style)
import * as THREE from 'three';
window.THREE = THREE;
const exportTHREE = window.THREE;
export default exportTHREE;
  1. We also have to export all the required modules and do all things after import.
const threeModulesToConnect = [import('three/examples/js/loaders/GLTFLoader')];
Promise.all(threeModulesToConnect).then(() => {
    ReactDOM.render(<App />, document.getElementById('root'));
});
  1. After we can import THREE from our module
    import THREE from 'three-exp';
    or use window.THREE
  2. We also could add declatations maybe like that
declare module 'modules/three-exp' {
    import * as THREE from 'three';
    const GLTFLoader: any;
    export = {...THREE, GLTFLoader};
}

odys-z added a commit to odys-z/x-visual that referenced this pull request May 20, 2020
@TacticalVilius
Copy link

Experienced the same issue here when using aframe. We have a React app that:

  • imports aframe via <script> tag. aframe bundles three with it
  • also used three throughout the code, so we had it as a dependency in package.json

This caused the exact same issue as described by others in this thread. We fixed it similarly to @rohan-deshpande's solution and use only aframe's three throughout the app (it is made available in window.THREE).

Annoyingly we can't completely remove the extra three from our project because it is a dependency of @types/aframe and we do want those types. The best would be if it was safe to import three into a project in all cases, but this solution works for now.

Many thanks to everyone in this thread for figuring this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.