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

Unexpected Products in latest myget Geometry causing crashes #26

Closed
andyward opened this issue Jul 29, 2015 · 10 comments
Closed

Unexpected Products in latest myget Geometry causing crashes #26

andyward opened this issue Jul 29, 2015 · 10 comments
Assignees
Labels

Comments

@andyward
Copy link
Member

@martin1cerny , just a heads up, this may be one for you or it may be one for @CBenghi

It looks like there's been a change in how we're handling openings in the Geometry Engine (possibly this change: xBimTeam/XbimGeometry@e63ad95) - which is causing the xbim-viewer[...].js to throw an exception during parsing of the wexbim file.

Repro

Using Xbim.Geometry 3.1.3 from Myget/develop, and the latest XbimWebUI (pre-release from myget)

Take any Model with an opening(e.g. OneWallTwoWindows.ifc) and you'll get error on the console at this line:

https://github.com/xBimTeam/XbimWebUI/blob/master/Xbim.WeXplorer/Viewer/xbim-model-geometry.debug.js#L175

If you use a wexbim generated from the latest Nuget Xbim.Geometry it's fine.

I'll see if I can track it back to a particular change but it may be a few days before I can look at it. I put a spot fix in to skip the problem items and you see something like this:

image

Left is latest Geometry (Myget 3.1.3). Right is the old Geometry (Nuget 3.0.21). Both are the same source model file and XbimWebUI code. The only difference is the version of Xbim.Geometry used to generate the webxbim. Might be worth speaking to Claudio to see what the intention is with the above change. The newer wexbim is larger so I'm assuming it contains extra records (intentionally, or unintentionally).

Loading the new (and old) xbim files into XbimXplorer renders OK

Sounds like either:

  1. WebUI needs to handle the new entries
    or
  2. The Xbim3DModelContext Serialiser needs to strip them out during wexbim serialisation
@andyward andyward added the bug label Jul 29, 2015
@CBenghi
Copy link
Member

CBenghi commented Jul 30, 2015

Hi,
I'll look into it this afternoon.
Thanks for the heads up.
Quite strange the file is larger... I've only changed the value of an enum
in xbim to handle voids differently.
Claudio

On 29 Jul 2015, at 23:24, Andy Ward notifications@github.com wrote:

@martin1cerny https://github.com/martin1cerny , just a heads up, this may
be one for you or it may be one for @CBenghi https://github.com/CBenghi

It looks like there's been a change in how we're handling openings in the
Geometry Engine (possibly this change:
e63ad958e9edfa158834fe50775c7b8bd2bacf64) - which is causing the
xbim-viewer[...].js to throw an exception during parsing of the wexbim file.

Repro

Using Xbim.Geometry 3.1.3 from Myget/develop, and the latest XbimWebUI
(pre-release from myget)

Take any Model with an opening(e.g. OneWallTwoWindows.ifc) and you'll get
error on the console at this line:

https://github.com/xBimTeam/XbimWebUI/blob/master/Xbim.WeXplorer/Viewer/xbim-model-geometry.debug.js#L175

If you use a wexbim generated from the latest Nuget Xbim.Geometry it's
fine.

I'll see if I can track it back to a particular change but it may be a few
days before I can look at it. I put a spot fix in to skip the problem items
and you see something like this:

[image: image]
https://cloud.githubusercontent.com/assets/923699/8971341/5c06b2fe-3647-11e5-9210-82d8c5fd9ac8.png

Left is latest Geometry (Myget 3.1.3). Right is the old Geometry (Nuget
3.0.21). Both are the same source model file and XbimWebUI code. The only
difference is the version of Xbim.Geometry used to generate the webxbim.
Might be worth speaking to Claudio to see what the intention is with the
above change. The newer wexbim is larger so I'm assuming it contains extra
records (intentionally, or unintentionally).

Loading the new (and old) xbim files into XbimXplorer renders OK

Sounds like either:

  1. WebUI needs to handle the new entries or
  2. The Xbim3DModelContext Serialiser needs to strip them out during
    wexbim serialisation


Reply to this email directly or view it on GitHub
#26.

@CBenghi CBenghi self-assigned this Aug 3, 2015
@andyward
Copy link
Member Author

@CBenghi Did you get any further with this? Still seeing the issue with latest develop code.

@martin1cerny
Copy link
Member

@andyward I'm sorry I didn't catch on this before. Thanks for the heads up. But it seems it is a bug in XbimGeometry or XbimIO rather than in WeXplorer. It just indicates that the wexbim file is wrong which is apparently true. I also wonder if @CBenghi got any further.

P.S.: It is interesting that most issues raised on GitHub in XbimWebUI project are related to different parts of xBIM :-)

@andyward
Copy link
Member Author

I did a bit a investigation into this. There are actually two issues.

  1. The WexBIM contains shape definitions that reference products not listed in the products list. Notably IfcOpeningElement geometry is coming though, but the elements are not.
  2. The WexBIM file contains duplicate geometry where there has been cutting. E.g. A wall will have geometry for before and after any opening.

So after resolving the first issue you get:
image
not
image

Using OneWallTwoWindows.ifc as a test case I added some diagnostics using Geometry 3.0.23 and the latest Develop.

Geometry 3.023:
image

Latest Geometry (develop)
image

We have the same 3 products (1 Wall and 2 Windows), but three extra shapes (# 5-7). Shape 6 is the uncut wall, while 5&7 are the openings with no product reference.

Hope that helps you @CBenghi

Andy

@CBenghi
Copy link
Member

CBenghi commented Aug 28, 2015

Hi Andy,
I think I have a solution to post, but I'm currently travelling overseas,
I'll post it as soon as I get a sound connection for you to test.
I'm not really strong on the web viewer, to be sure it's all right.
Also I think the wexbim format should be enriched. At the moment it cannot
differentiate between model shape types.
Claudio

CBenghi added a commit to xBimTeam/XbimGeometry that referenced this issue Aug 29, 2015
@CBenghi
Copy link
Member

CBenghi commented Aug 29, 2015

Hi Andy,
I've posted a couple of changes to Geometry that should fix this problem, but I don't know enough of the webviewer to understand how it ever worked before them...
Can you please double check?
Thanks,
Claudio

@dazmahal
Copy link

Hi Claudio
Has the Geometry fix been published to Nuget / MyGet
If so I can test this fix

Thanks
Darren

On 29 August 2015 at 14:31, Claudio Benghi notifications@github.com wrote:

Hi Andy,
I've posted a couple of changes to Geometry that should fix this problem,
but I don't know enough of the webviewer to understand how it ever worked
before them...
Can you please double check?
Thanks,
Claudio


Reply to this email directly or view it on GitHub
#26 (comment).

@CBenghi
Copy link
Member

CBenghi commented Aug 29, 2015

Hi Darren,
yes it's been uploaded under the develop branch.
You will find the nuget package in the myget develop repo.
https://www.myget.org/F/xbim-develop/api/v2
Best,
Claudio

@andyward
Copy link
Member Author

@CBenghi Nice work! Looks to have fixed the issue 👍
image

Note for anyone else - I used 3.1.10-V00006

I guess we need to discuss if we ever want these geometries in a wexbim at some point. My gut feel is to wait until we find a need since we want to keep the wexbim payload a light as possible.

@andyward
Copy link
Member Author

Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants