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

[ArchCurtainWall] Add OverrideEdges & ArchSketch Support #13950

Conversation

paullee0
Copy link
Contributor

  • Add Overridges property to let user to select particular edge(s) in a Sketch / ArchSketch to use create the shape of the Arch Curtain Wall (instead of using all edges by default).

ENHANCEMENT by External 'ArchSketch' Add-on:

  • GUI 'Edit Curtain Wall' Tool is provided in external Add-on ('SketchArch') to let users to select the edges interactively.
  • The selection of edges is 'Toponaming-Tolerant' if ArchSketch is used in Base (and SketchArch Add-on is installed).
  • Warning : Not 'Toponaming-Tolerant' if just Sketch is used.
  • Property is ignored if Base ArchSketch provided the selected edges.

Forum Discussion:

- Add Overridges property to let user to select particular edge(s) in a Sketch / ArchSketch to use create the shape of the Arch Curtain Wall (instead of using all edges by default).

ENHANCEMENT by External 'ArchSketch' Add-on:

- GUI 'Edit Curtain Wall' Tool is provided in external Add-on ('SketchArch') to let users to select the edges interactively.
- The selection of edges is 'Toponaming-Tolerant' if ArchSketch is used in Base (and SketchArch Add-on is installed).
- Warning : Not 'Toponaming-Tolerant' if just Sketch is used.
- Property is ignored if Base ArchSketch provided the selected edges.

Forum Discussion:
- https://forum.freecad.org/viewtopic.php?p=756554#p756554
  [ ArchSketch ] - Curtain Wall, Slab, ArchWall etc. on Same ArchSketch
@github-actions github-actions bot added the WB BIM Related to the BIM/Arch Workbench label May 11, 2024
if obj.Base.Shape.Faces:
faces = obj.Base.Shape.Faces
elif obj.Height.Value and obj.VerticalDirection.Length:
ext = FreeCAD.Vector(obj.VerticalDirection)
ext.normalize()
ext = ext.multiply(obj.Height.Value)
faces = [edge.extrude(ext) for edge in obj.Base.Shape.Edges]
# ArchSketch feature :
if hasattr(obj.Base, 'Proxy'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will leave out all curtain walls whose Base object has a Proxy, but no getCurtainWallBaseShapeEdgesInfo

I thin you need either an else: here, or set a default value for curtainWallBaseShapeEdges

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking :) See if I understand you and below clarify the algorithm :-

  • Line 307 : set curtainWallBaseShapeEdges = None

  • Line 317 : though there is no elif/else after this, the test is in Line 320, 322, 336

  • Line 320 : it test if there is information in 'curtainWallBaseShapeEdges'

  • Line 322 : (elif) if above is not, default would following CurtainWall.OverrideEdges, if the Base is a Sketch
    (partially work, i.e. without 'Toponaming-Tolerant' support)

  • Line 336 : (else) this catch all, and it revert back to Base's edge - i.e. obj.Base.Shape.Edges

Hope this works, would review if otherwise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right. I didn't look well enough :) Sorry about the noise.

I'll leave this to merge after #13783 okay?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, feel free to proceed as you plan.

Incidentally, I like the name Arch more than BIM :) And an add-on allow it be updated in a more free timetable not tied to the release cycle of FC itself.

Anyway, will have a look at your big merge to understand your big plan :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, I was myself not sure about which path to take for a long time... But at the end, there is duplication work happening (same things in both Arch and BIM), and also it's time the BIM stuff gets the eyes of everybody (too few contributors when it's an addon on my own github), and also everybody thinks it's a good idea to have a built-in BIM workbench, same as we just did with CAM...

@FEA-eng
Copy link
Contributor

FEA-eng commented May 20, 2024

Now that Arch became BIM, isn't it necessary to account for that in this PR? I guess that's what causes the conflicts.

@yorikvanhavre
Copy link
Member

All the object classes are left in the original Arch files. What has changed is that all the command classes have been moved into the bimcommands subfolder. This PR contains only changes to the Curtain Wall object, so it should be applicable without problems. Only issue I guess, is that the "Arch" folder has been renamed to "BIM" and github doesn't find its way anymore...

@paullee could you try this? I think it should work:

  1. Save the diff of your modified ArchCurtainWall file: (git diff <commit-id-before-this-one> -p > curtainwall.patch)
  2. Undo your commit (git reset HEAD~)
  3. Rebase your branch (git rebase main)
  4. Edit curtainwall.patch and change Mod/Arch with Mod/BIM, and apply it (git apply curtainwall.patch)
  5. Redo the commit and force-push to this branch (git push -f ArchCurtainWall_02_Add_OverrideEdges_and_ArchSketch_Support)

@paullee0
Copy link
Contributor Author

All the object classes are left in the original Arch files. What has changed is that all the command classes have been moved into the bimcommands subfolder. This PR contains only changes to the Curtain Wall object, so it should be applicable without problems. Only issue I guess, is that the "Arch" folder has been renamed to "BIM" and github doesn't find its way anymore...

@paullee could you try this? I think it should work:

1. Save the diff of your modified ArchCurtainWall file: (`git diff <commit-id-before-this-one> -p > curtainwall.patch`)

2. Undo your commit (`git reset HEAD~`)

3. Rebase your branch (`git rebase main`)

4. Edit curtainwall.patch and change Mod/Arch with Mod/BIM, and apply it (`git apply curtainwall.patch`)

5. Redo the commit and force-push to this branch (`git push -f ArchCurtainWall_02_Add_OverrideEdges_and_ArchSketch_Support`)

Thanks very much for the detailed procedure, that's very helpful. I only know very fundamental git command to commit etc. and messed up things very badly a couples of time if did some more than that :D

I would see if I manage to follow the steps or 'manually cherry-pick' would be much faster for me : LOL

@paullee0
Copy link
Contributor Author

I 'finally' re-do, after puzzling a while at Step 1 only :D

See :
#14201

Thanks

@yorikvanhavre
Copy link
Member

Closing this one as superseded by #14201

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WB BIM Related to the BIM/Arch Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants