mcneel / rhino3dm

Libraries based on OpenNURBS with a RhinoCommon style
MIT License
573 stars 135 forks source link

three.js loader produces black geometry in certain files after rhino3dm v8 upgrade #590

Closed tb-viktor closed 4 months ago

tb-viktor commented 4 months ago

This relates to an issue from the three repository since there was no further activity there: https://github.com/mrdoob/three.js/issues/27752

Description

Hello,

We've encountered a regression with the Rhino3dmLoader after upgrading to the latest version of three. We've upgraded from version 0.156.1 to 0.161.0 and with that loaded rhino3dm@8.0.1 which is required by the loader. After doing so, certain geometries seem to break the loader and result in the loaded geometry appearing black. After loading the file that causes this, the following files that are loaded also appear black for that loader instance.

I've put together a quick codesandbox with the file in question, I am not sure whether it is the file size (around 50 MB) or something in the file that causes this: https://codesandbox.io/p/sandbox/three-rhino-latest-sxz2fy

I've uploaded the file separately here: https://filebin.net/a258vmwnae1nj10a - it has an expiration, so in case the link goes dead I will reupload.

Any ideas what is causing this?

Reproduction steps

  1. Load the provided file using Rhino3dmLoader in the latest three version, with rhino3dm@8.0.1.
  2. The geometry will appear black.
  3. Bonus: Using the same loader instance load another file, this file (that might have been rendering fine previously) will also appear black).

Code

Quick React example:

import { useEffect, useRef } from "react";
import * as THREE from "three";

import { OrbitControls } from "three/examples/jsm/controls/OrbitControls.js";
import { Rhino3dmLoader } from "three/examples/jsm/loaders/3DMLoader";

const width = 640;
const height = 480;

export default function App() {
  const containerRef = useRef(null);
  const statusRef = useRef(null);

  useEffect(() => {
    if (containerRef.current && statusRef.current) {
      let camera, scene, renderer;
      let controls;

      init();
      animate();

      function init() {
        THREE.Object3D.DEFAULT_UP.set(0, 0, 1);

        renderer = new THREE.WebGLRenderer({ antialias: true });
        renderer.setPixelRatio(window.devicePixelRatio);
        renderer.setSize(width, height);
        statusRef.current.innerHTML = "Loading...";
        containerRef.current.innerHTML = "";
        containerRef.current.appendChild(renderer.domElement);

        camera = new THREE.PerspectiveCamera(60, width / height, 1, 1000);
        camera.position.set(26, -40, 5);

        scene = new THREE.Scene();
        scene.background = new THREE.Color(0xedf1ff);

        const directionalLight = new THREE.DirectionalLight(0xffffff, 3);
        directionalLight.position.set(0, 0, 2);
        scene.add(directionalLight);

        const ambientLight = new THREE.AmbientLight(0xffffff, 0.8);
        scene.add(ambientLight);

        const loader = new Rhino3dmLoader();
        loader.setLibraryPath("https://cdn.jsdelivr.net/npm/rhino3dm@8.0.1/");
        loader.load(
          "https://filebin.net/02okngzrvnv6pis8/circular_open.3dm",
          function (object) {
            statusRef.current.innerHTML = "Loaded";
            scene.add(object);
          }
        );

        controls = new OrbitControls(camera, renderer.domElement);
      }

      function animate() {
        controls.update();
        renderer.render(scene, camera);

        requestAnimationFrame(animate);
      }
    }
  }, []);

  return (
    <>
      <div ref={containerRef} />
      <div ref={statusRef} />
    </>
  );
}

Before (three@0.156.1 & rhino3dm v7): https://codesandbox.io/p/sandbox/three-rhino-v7-v99r8c image

After (three r157+ & rhino3dm v8): https://codesandbox.io/p/sandbox/three-rhino-latest-sxz2fy image

Note: This also happens with rhino3dm@8.4.0.

fraguada commented 4 months ago

Thanks for the heads up. I had missed the notification from the threejs repo. I'll look into it. Very much appreciate your detailed report.

fraguada commented 4 months ago

@tb-viktor I see what is going on. Your file has some meshes with vertex colors, some meshes without. None of the objects in your file have any assigned materials. I need to give objects materials in threejs, and since some of these objects have vertex colors, I switch on vertex colors for the "default" material I assign for objects without material assignments. This means then that any object without a material assignment will get assigned a default material with vertex colors. When the mesh has no vertex colors, it will render them black.

What I will do is to detect when the default material is used in situations with vertex colors. In this case I'll make a second default material with vertex colors switched on for meshes with vertex colors, and another default for meshes without vertex colors.

This will take some days to propagate into the threejs repo so my suggestion in the meantime would be to assign a material in Rhino to the objects without vertex colors. This way they won't get assigned the default material with vertex colors switched on.

I'll comment back here when I get the fix in.

tb-viktor commented 4 months ago

@fraguada Thanks for your quick response. Looking forward to the fix 👍

The geometry we render in our product is user-generated, so we don't have much control over it nor know what to expect. Due to this it's important that we render all possibilities in a decent way.

tb-viktor commented 4 months ago

@fraguada question - the second part of this problem is once we load a file that causes the loader to display the geometries black, the next file we load are also turns black, even if it loaded fine before with a clean loader instance. Will this problem also be addressed with the fix?

fraguada commented 4 months ago

Yes. This is again due to the default material being set to try and render vertex colors.

fraguada commented 4 months ago
image

Already better, but that one area still black!

fraguada commented 4 months ago

Ok. A PR has been pushed to the threejs repo. Let's hope it passes!

fraguada commented 4 months ago

It passed and this fix will go out in r162 tomorrow

tb-viktor commented 4 months ago

Perfect, thank you for your work. I will let you know if there are any further problems 👍

fraguada commented 4 months ago

@tb-viktor please test in threejs 162 and let me know if you find any issues.

tb-viktor commented 4 months ago

@tb-viktor please test in threejs 162 and let me know if you find any issues.

So far everything works well, will let you know if we catch anything more 👍