ifc-git icon indicating copy to clipboard operation
ifc-git copied to clipboard

Porting to BlenderBIM

Open brunopostle opened this issue 2 years ago • 16 comments

The intent is that something like this add-on should be merged into BlenderBIM:

  • [x] The code is not structured like a BlenderBIM module, so there is some refactoring needed.

  • [x] There are a couple of dodgy bits of code, mainly relating to the ifcgit_repo global.

  • [x] GitPython is easy enough to bundle into the BlenderBIM package as it is all python, however it does require the git executable in the PATH, I think initially this should be an additional step to install separately. Or the add-on could switch from GitPython to dulwich which has a python git implementation,

  • [ ] ifcmerge is a single file perl script with no library dependencies, so it should just work on Linux and OSX, there is a windows ifcmerge.exe, but wherever is comes from, it also needs to be installed in the PATH. A solution would be an ifcmerge rewrite in python by somebody who understands python regular expressions, so it can be called directly not as an executable.

brunopostle avatar Mar 23 '23 23:03 brunopostle

I can try to help with the refactoring to BlenderBIM module. I'll sketch an initial proposal to see if I'm on the right way.

brunoperdigao avatar Apr 08 '23 21:04 brunoperdigao

@brunoperdigao I'm part way through doing this in this 'split' branch: https://github.com/brunopostle/ifc-git/tree/split

The plan is to break it up into separate files that are equivalent to the init, core, tool, operator, data, ui & prop file separation in blenderbim modules. This should make it easier to move later.

So far I have been following the hello world! tutorial:

  • [x] split the files, they are still in the ifc-git repo. I had to temporaily name operator.py as operators.py as there is a base python module called operator.
  • [x] move all the functional code out of operator and into core
  • [x] structure data properly with everything in a data hash and a load() method that actually does something
  • [x] move functional code out of core and into tool
  • [x] move some logic out of ui
  • [x] change all the tool functions into classmethods
  • [x] move all the Property definitions out of init and into prop
  • [x] move everything to a fork of blenderbim
  • [x] write a GitPython check for the git executable (which doesn't ship with GitPython) and disable the module if necessary
  • [x] handle merge failure when there is no ifcmerge executable
  • [ ] other stuff
  • [ ] write tests
  • [x] do a pull request

Any advice, better ideas, or help getting these done is appreciated.

brunopostle avatar Apr 08 '23 23:04 brunopostle

Great, I'll take a look and see what tasks I can be helpful with

brunoperdigao avatar Apr 08 '23 23:04 brunoperdigao

I can start working in these tasks if that's ok for you:

  • [x] move functional code out of core and into tool
  • [x] move some logic out of ui
  • [x] change all the tool functions into classmethods
  • [x] move all the Property definitions out of init and into prop

brunoperdigao avatar Apr 09 '23 02:04 brunoperdigao

@brunoperdigao yes go ahead!

brunopostle avatar Apr 09 '23 07:04 brunopostle

@brunopostle i was thinking about two things that maybe are an improvement:

  • create two folders: one called "core" and another called "tool", like how blenderbim is structured. In this way, it is possible to create also the "tool.py" file in core folder, the ifc-git tool file in tool folder and ifc-git core file in core folder
  • in theory, core file shouldn't has import, only tool file should has import...

I'm just trying to help btw so i'm writing what i learned...

maxfb87 avatar Apr 10 '23 15:04 maxfb87

@maxfb87 yes this would be ok and would match the blenderbim structure a bit more closely

brunopostle avatar Apr 10 '23 17:04 brunopostle

@brunopostle Could you elaborate on "move some logic out of ui". I'm not sure what would be the best approach here.

Edit: I read a bit more of the comments on the BlenderBIM "demo" module, so I guess this task is related to "structure data properly...", right?

brunoperdigao avatar Apr 20 '23 00:04 brunoperdigao

@brunoperdigao this is my first blenderbim module, so I'm figuring it out as we go.

From the demo example I think that 'ui' shouldn't access 'tool'. Things like the repo and the lookup tables should be accessed via 'data' and all refreshed at the same time (at the moment the refresh is haphazard).

I'm on a train, so not the best diagram, this is how I think the inheritance is supposed to work:

IMG_20230420_065158~2

Tagging @moult to confirm I have this right

brunopostle avatar Apr 20 '23 08:04 brunopostle

Looks good!

Moult avatar Apr 20 '23 08:04 Moult

@Moult How does the 'refresh()' from 'data' work? I feel I'm missing something because the data is not being refreshed. I'm not working in the correct folder structure yet, maybe it has something to do with it?

brunoperdigao avatar Apr 20 '23 23:04 brunoperdigao

The refresh function does whatever it needs to purge the data available for the UI for that module. Most of the time setting a "is_loaded = False" is sufficient since that triggers a reload.

blenderbim.bim.handler.refresh_ui_data() calls the refresh function of all modules.

Moult avatar Apr 20 '23 23:04 Moult

I guess this refresh() function doesn't get called until we integrate this into blenderbim, so until then we could get away with running it in the existing refresh button/operator

brunopostle avatar Apr 21 '23 06:04 brunopostle

@brunoperdigao I made a few more fixes. There are still some glitches but I think this is mainly to do with the refresh mechanism not getting called.

Do you think it is ready to move into a fork of blenderbim? Would you like to do this?

brunopostle avatar Apr 22 '23 08:04 brunopostle

Yes, I'd love to. I'm not sure how to proceed github-wise, though. I have a BlenderBIM fork, should I just copy the code to there and create a PR?

brunoperdigao avatar Apr 22 '23 13:04 brunoperdigao

@brunoperdigao yes use your GitHub fork of ifcopenshell, but create a branch and add the files to that, don't use your main branch.

I think there will be a bit of work to get it to run, maybe with the refresh stuff. I'd also like to try and write some tests, because I have no idea how these work in blenderbim, but this shouldn't stop you creating a pull request for @Moult to review once the basic functionality seems ok.

I have a room to paint today and tomorrow :( but can look at anything you have done this evening.

brunopostle avatar Apr 22 '23 14:04 brunopostle

Closing as merge with BlenderBIM is done

brunopostle avatar Jul 24 '24 13:07 brunopostle