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

Removed apparently unused and buggy lines in BlenderBIM's IFC importer #2100

Closed
wants to merge 1 commit into from

Conversation

yorikvanhavre
Copy link
Contributor

These two lines cause an error when importing https://gitlab.com/openingdesign/101_W_33rd_St/-/blob/main/Models%20and%20CAD/IFC/101_W_33rd_St_Details_D.ifc
Removing them seems safe, they seem buggy anyway (IfcOpenShell.geom.settings don't have a 'materials' member) and materials are still correctly created, but I'm creating this PR mostly to ping @Moult to have a better look.

@aothms aothms requested a review from Moult March 22, 2022 12:53
@Moult
Copy link
Contributor

Moult commented Mar 23, 2022

Thanks @yorikvanhavre ! This is actually a duplicate of #2078

Actually removing them isn't the right thing to do - it's actually an implementation of IFC shaders. IFC supports three different types of lighting / reflectance models which is fully compatible with X3D and glTF - it supports FLAT (unlit shadless), PHONG (old-school biased rendering, and PHYSICAL (modern PBR rendering).

It seems as though there were a few typos in my FLAT implementation :)

Here's the result if you're curious. On the left you can see the shader tree. You can see it is actually compatible with glTF in Blender here: https://docs.blender.org/manual/en/dev/addons/import_export/scene_gltf2.html#exporting-a-shadeless-unlit-material

2022-03-23-203945_1547x789_scrot

By the way I was very excited to see the use of IfcAnnotation! I think with a very little bit of work we can actually make annotations roundtrip between Blender and FreeCAD! All that is needed is to tag them with the appropriate IFC object type (e.g. ObjectType=DIMENSION) and associate it with a drawing object in a group, or within a drawing (e.g. IfcDocumentInformation) and then it will come through. For smart labels you just need to implement IfcTextLiteral and assign them to a product IfcRelAssignsToProduct to create a parametric link). I manually changed a couple and here's the result:

2022-03-23-203722_840x746_scrot

@Moult Moult closed this Mar 23, 2022
@yorikvanhavre yorikvanhavre deleted the v0.7.0-fix branch March 23, 2022 10:12
@yorikvanhavre
Copy link
Contributor Author

yorikvanhavre commented Mar 23, 2022

I understand fro the doc ObjectType can be anything, but there are ¨recommended" values to be used... We might want to already define a few on our own...

So what we need to implement in FreeCAD (I think) (sorry I'm hijacking this issue :) )

  • Support Draft Labels
  • Set ObjectType when exporting an IfcAnnotation. In FreeCAD I suppose we would have:
    • Dimensions
    • Linework
    • Texts
    • Areas/surfaces (faces and hatches)
    • Smart labels
    • Undefined (for all the rest I cannot think of)
  • For "usermade" labels, (a line + a text), find a way to export them as one single IfcAnnotation
  • Add IfcRelAssignsToProduct when exporting smart labels and dimensions (if associated to an object)
  • "associate it with a drawing object in a group, or within a drawing (e.g. IfcDocumentInformation)" @Moult care to explain a bit more how that works? What is a drawing object? So far FreeCAD doesn't export anything related to documents/drawings, but might not be too hard...

@yorikvanhavre
Copy link
Contributor Author

Corresponding issue at FreeCAD: yorikvanhavre/BIM_Workbench#92

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.

2 participants