opensourceBIM / BIMserver

The open source BIMserver platform
GNU Affero General Public License v3.0
1.55k stars 610 forks source link

Implement interface IfcSurfaceStyleShading #689

Closed kurb70 closed 5 years ago

kurb70 commented 6 years ago

I tried to import an (quite simple) IFC4 file – created by ARCHICAD 21 – into BIMserver v1.5.95.

After selecting the file and hitting Check in new version I am getting the message Unimplemented interface org.bimserver.models.ifc4.IfcSurfaceStyleShading.

Do I miss some restrictions here?

This was the file I used: AC21_aut_simple_arch_IFC4_RV_model.ifc.gz

rubendel commented 6 years ago

Thanks for the IFC file. It seems to be invalid though. The error is in all the IfcIndexedColourMap entities. In this file the second field is always the value 1. The IFC4 final schema says it should be an IfcSurfaceStyleShading reference, hence the error message.

The error message could be clearer and should include a line number, I'll make sure that happens. Please contact the developer of your CAD/BIM software to fix this issue.

Excerpt from file:

#398= IFCINDEXEDCOLOURMAP(#316,1.,#394,(1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,2,2,2,2,2,2,2,2,1,1,1,1,1,1,1,1));
rubendel commented 6 years ago

The new error message would now be:

Error on line 93: IfcSurfaceStyleShading expected, but got "1."
kurb70 commented 6 years ago

FYI I informed GRAPHISOFT about this issue.

kurb70 commented 6 years ago

I passed the problem on to Graphisoft. A programmer replies that the file is valid.

@rubendel If I understood your comment correctly, it's about the attribute opacity. In my opinion, the specification allows the value.

http://www.buildingsmart-tech.org/mvd/IFC4Add2TC1/RV/1.1/html/schema/ifcpresentationappearanceresource/lexical/ifcindexedcolourmap.htm

I tried to open the file in two different standard IFC viewers. One of them was SOLIBRI. No error is reported.

kurb70 commented 6 years ago

In a former IFC spec (IFC4Add1 RV) I found that the value Opacity shall not be set.

rubendel commented 6 years ago

It's about the second field of "IfcIndexedColourMap" which is called "Overrides". BIMserver uses the IFC4 final specification [1].

I see that a following specification (IFC4/Add2) has replaced this field by "Opacity" [2].

The header of this file indicates that the schema "IFC4" is used, which indicates IFC4 Final (in my opinion) [3]. However, I see that buildingSMART decided to use the exact same schemaname for the subsequent Addendum1/Addendum2 releases, which I don't think is smart at all [4].

So in short, your file is indeed valid if you check it against the IFC4/Add2 schema. We could switch to that schema in BIMserver, but that would potentially make all IFC4 files created earlier invalid. I really can't believe the schemaname is the same for different schemas...

[1] http://www.buildingsmart-tech.org/ifc/IFC4/final/html/schema/ifcpresentationappearanceresource/lexical/ifcindexedcolourmap.htm [2] http://www.buildingsmart-tech.org/ifc/IFC4/Add2/html/schema/ifcpresentationappearanceresource/lexical/ifcindexedcolourmap.htm [3] http://www.buildingsmart-tech.org/ifc/IFC4/final/IFC4.exp [4] http://www.buildingsmart-tech.org/ifc/IFC4/Add2/IFC4_ADD2.exp

hlg commented 6 years ago

I have been wondering before about the schema name not including the addendum / technical corrigendum information. But I assumed this could be justified if the changes in addendums and corrigendums would ensure backward compatibility for STEP files (e.g. rename attributes, change from mandatory to optional). This example breaks that faith.

Looking again at the change logs, I see two columns indicating compatibility issues for STEP and XML. In the presence of STEP file compatibility issues, of course it should be clear that the schemaname must change.

rubendel commented 6 years ago

@hlg Thanks for that link. My plan is to switch the BIMserver IFC4 implementation to Add2. For the few incompatible changes between IFC4 "final"/Add1 and Add2 a few hacks will be implemented.

Hardest problem now would be getting 3 IFC4 files of the 3 different versions that use the entities that have incompatible changes to them...

hlg commented 6 years ago

You can try these. No warranty.

ifc4final.ifc.txt ifc4add1.ifc.txt ifc4add2.ifc.txt

rubendel commented 6 years ago

Thanks for the models!

So far, a few hacks have been implemented in both the streaming and non-streaming step deserializer:

The last file does seem to have invalid RelatingPriorities/RelatedPriorities. It's supposed to be < 100, but BIMserver does not currently check these type of constraints.

While fixing these issues I also found out that the https://github.com/opensourceBIM/BuildingSMARTLibrary needed an update for Add2 as well, a new release was made and will be referenced from BIMserver 1.5.101 and onwards.

Also the ifc4.js file in both BIMserver and the BIMserver JavaScript API had to be updated to Add2, which is also done now.

hlg commented 6 years ago

I think the range of those priorities should be inclusive [0..100], the Express rule says "0 <= temp <= 100".

rubendel commented 6 years ago

Ah yes you are right, I thought I had seen numbers in the thousands, but looked at the wrong number.

hlg commented 6 years ago

IfcRelConnectsPathElements.RelatingPriorities and IfcRelConnectsPathElements.RelatedPriorities are only be multiplied if they are REALs and <=1, right? That is, only if the checkin conforms to an older IFC4 version. I just checked ISO 10303 - REALS have mandatory decimal points, so that in case of value 1 the IFC4 version is decidable too without looking further than the current attribute value. My sample files where wrong in this regard. Sorry, I am attaching new ones.

Since IfcSurfaceStyleShading.Transparency is optional, it can be left undefined if not provided in the STEP file, no need to set it to 0 IMHO. Downstream applications should assume 0 in that case. Note that the attribute was pulled up from IfcSurfaceStyleRendering, where it might have been set even before IFC4 final. I included an instance in the new files.

Another attribute to be handled is IfcIndexedColourMap.Opacity AKA Overrides in IFC4final. It is optional, hence can be left undefined if not provided. However, the parser should handle references to IfcSurfaceStyleShading gracefully. In case this is of type IfcSurfaceStyleRendering, the opacity could be calculated from the respective IfcSurfaceStyleRendering.Transparency, but I assume this would not be worth the effort, since IFC4final files using this specific construction should hardly exist.

ifc4final.ifc.txt ifc4add1.ifc.txt ifc4add2.ifc.txt

rubendel commented 5 years ago

Thanks again for the files @hlg. I think I got it right now. The first 2 items have been implemented as suggested, and the last one I left out as these files hopefully don't exist in the wild.