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

Double-sided materials #4

Closed
MadsHolten opened this issue Dec 23, 2022 · 4 comments
Closed

Double-sided materials #4

MadsHolten opened this issue Dec 23, 2022 · 4 comments

Comments

@MadsHolten
Copy link
Contributor

DoubleSided materials are not rendered correctly.

Issue

Below you see an example of the glTF taken directly from the zip loaded in an online viewer and here there is no problem.
image

The following fragments zip can be used for testing:
Duplex.zip

I tried making the following changes to fragment-ifc-importer/geometry.ts with no luck

private sortGeometriesByMaterials(
    geometryData: WEBIFC.PlacedGeometry,
    geometry: THREE.BufferGeometry
  ) {
    const materialID = this.saveMaterials(geometryData);
    if (!this._geometriesByMaterial[materialID]) {
      this._geometriesByMaterial[materialID] = [geometry];
    } else {
      this._geometriesByMaterial[materialID].push(geometry);
    }
    geometry.computeVertexNormals(); <----ADDED
  }

private saveNewMaterial(colorID: string, color: WEBIFC.Color) {
    console.log("SaveNewMat")
    this._materials[colorID] = new THREE.MeshLambertMaterial({
      color: new THREE.Color(color.x, color.y, color.z),
      transparent: color.w !== 1,
      opacity: color.w,
      side: THREE.DoubleSide <----ADDED
    });
}
@MadsHolten
Copy link
Contributor Author

Probably the problem is with bim-fragment. I will try solving it here: https://github.com/IFCjs/fragment/blob/main/library/src/fragment-loader.ts

@MadsHolten
Copy link
Contributor Author

I managed to fix it myself. In order to fully fix the problem one will have to 1) Implement this PR in fragments, 2) Publish a new version of fragments on npm, 3) Implement this PR and 4) Bump the version of fragments used here.

@agviegas
Copy link
Contributor

Hey, sorry for the late reply, I missed this. I think that with the current implementation, you can set all the materials of a fragment set to be double sided by just traversing them and setting them as such. Don't you think this is better, as double sided materials have a higher computational cost? That way, the scene will be more efficient by default, and only people that need double sides will be able to set it up.

@agviegas
Copy link
Contributor

agviegas commented Nov 1, 2023

Closing this for now, but open to discussing this further!

@agviegas agviegas closed this as completed Nov 1, 2023
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

No branches or pull requests

2 participants