-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add face reference modes (for angle and offset) to AddWall command #467
Conversation
Updates that's make the code work properly with 3D angle
Fixes at SheetMetalCmd.py
But the origin planes cannot be selected in the taskpanel yet
Thank you for your great work! |
You're welcome! |
Tried running the code from this PR and got missing icons and a bunch of errors like this:
I am on Linux, with Python 3.13.1 and Freecad 1.0. Switching to Love the look of new icons, they make each mode much easier to understand. Good job! |
Played with the feature a little bit, here are my comments from usability perspective: Perfect touching, which the face mode makes very easy, often cause unfold to partially fail (touching parts not included in unfold). This somewhat limits the usefulness of the new feature. You end up using offset(plane)+offset mode to introduce gap in which case you might as well skip face reference and eyball it. Unfolding improvements are out of scope for this issue. It felt like whether it fails depended on which face was on top so maybe it's fixable. But I wouldn't be surprised if OCCT complained about such geometry even with unfolding fix. After selecting bend angle face, preview doesn't immediately refresh. I have to change back and forth one of the other settings like relative angle or wallpos to make it refresh. I found the the double face selection workflow a bit clunky. Even though I understand the reasoning behind it, half the time after I selected face for bend angle and saw that "wall pos" is set to "material outside" or "material inside" inside but actual position is nowhere close it made stop for a few seconds until I remember that I need to set mode to "offset" select face again, and then select "material inside" in second dropdown. Not sure how to be better solve this. I somewhat like the idea of wall position face choice being separate from angle choice. I guess it might depend on the situation where you use this feature. If you are referencing some global datum plane, I guess it makes sense to use only angle, but for the types of bends I tried to make relative to other faces of shape most of the time I also wanted to use it for wall pos. Does it even make sense to use to use 2 different faces here instead of single face for both or single face only for angle? I guess the double selection might be more more useful if second reference feature could be a vertice not just face.
Is there any way to change the reference face without toggling face relative angle/offset off and back on? Face relative angle worked even when selecting multiple edges for symmetrical bends. I was able to make hexagonal box with overlapping flaps using only 2 bend operations and no additional Sheetmetal or PD operations. No idea how, but it worked. Does it do the face relative calculations only for the first edge in list? Since it's not obvious which bend will be used as reference, it was easier to set it up for single edge first and then add rest of them afterwards. |
@karliss Thanks for the catch!
@shaise I think is needed to fix these errors before merging. @karliss I glad you like the icons! I think is very good to have someone testing on another OS. SheetMetal WB need it. Thanks! 👍 _
This happens even if the user has set a value for the "angle" manually. The code works like a user who inserts a value in the "angle" property. _
If you change the angle mannualy, happens the same: the model only updates on the second change of the angle. If you change the wall length it updates too. I try fix it on the past, but I couldn't. I'll try again when I fix that thing about the icons path. _
If the user select a face for "angle" reference, it will works only for angle. The wall position is relative to the selected edge on the sheet metal part, not the angle face reference. The wall position need to be defined even if the angle is manually inserted value. _
I agree about the offset reference, maybe will be nice if the user can select another type of geometry as reference to it (like vertex). Just a little trick: as asked in the said issue, was inserted too a new "shortcut" type in the code. If the user select the face reference (together with the edges selections) before click on the AddWall command it will use this face for angle and offset references. _
Well, I think something like that was discussed on the refereced issue #307 , but the conclusion for that to make the current UI in this PR. I think is better to keep this taskpanel for this moment. Maybe in the future a new UI can be discussed and change it again. _
I was thinking about that. I think it can be done if instead of a LineEdit field for the face references properties we use a button. But then I would get into another long debate about UI and consistency across FreeCAD (consistent user experience and similar interfaces for similar tasks). And I have enough about this on this issue. I will let this for my next issue. 😅 _
Yes, the calculations for "angle" and "offset" use the first edge in the list. _
I tried this here on a elevator part project, and unfold works well. I will attach a file where I used this. But I agree we will need some sort of corner gap tool on the future. Can you see it unfold well on Linux? elevatorPart.zip *PS.: I used the new unfolder code. _
|
Thanks for test! 👍 Got it! This is why there's a "offset position" for face reference. Sometimes is better to has some clearance between the reference face and the wall (even if so small that it can be considered negligible of manufactoring perspective). And the good thing here is: this clearance is parametric (even if some length is changed the clearance size will be the same). By the way, the distance of the offset for "offset position" is measured perpendicular to the face reference. I haven't looked much at the new unfolder code either. I'm looking to help with other things on SheetMetalWB. But of course, I'm very excited about the new possibilities of the new code and its speed! |
* Fix the directory path for better compatibility with other OS; * Fix model angle update when changed in taskpanel; * Fix angle and offset form fields when using face reference modes.
Done!
|
+1 !! |
QtCreator 15.0.1 (community)
Thanks for the tip! I will look at that and use the icons through the UI files.
It's set checked when is the unfold is on - like the face reference mode buttons. Thanks! I added that fix in the code (but not in github yet - I will change the UI file before put all code into github).
I test the file here and no problem when open the bend feature. But get another error when opening the file:
|
Other fixes include: * Add a line to make sure all properties ared added - very needed for backward compatibility; * Simplification of some lines of taskpanel updatesForm function; * Remove some unused code lines.
Done!
Waiting for the answer about the unfold option. buttonsUnfoldFlip01.mp4 |
Thanks @sheetmetalman ! |
Done! |
In the below images I use the buttons as PushButtons in all themes. Notice the buttons.
|
@shaise The icons got empty in FreeCAD on all buttons? Can you send a image (with the buttons as ToolButtons)? |
Ah, and regarding the usage of the same button for select-geometry / remove-geometry-selection, I'm OK with that. But for better clarity, you can set the Normal-off Icon to the same icon you have now, and for the Normal-On add a small red X on the icon. This way the icon will toggle the image and the user can understand that pressing again deletes the selection. |
@shaise Thanks for tip about the icons size! 👍 Exactly what I needed! The buttons are fixed! 🥳 About the face selection mode for offset: it only appears if the wall position is set to "offset", otherwise the offset SpinBox (to manually enter the offset value) and the face selection mode button do not appear. The icon for the "on" and "off" button mode is also done. I did it a little different. Also fixed the HorizontalLine that did not appear in OpenTheme nor in the default FreeCADTheme Dark or Light. Take a look: task.panel.06.mp4 |
Fix an error saying that a QLineEdit was deleted after selected a face reference. This error did not affect the feature functionality. Fixed for a clean working.
Right directory!
Hey @sheetmetalman , this looks great! I have squashed the merge to a single change. |
I glad you like it! 🤝 |
Closes issue #307 .
Including new icons and additions in the taskpanel of the said command.