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

Sketcher: task tool settings widget #5389

Closed
wants to merge 9 commits into from

Conversation

PaddleStroke
Copy link
Contributor

This feature adds a widget to Task in sketcher.
This enables user to type dimensions of geometries and create constraints accordingly.
The goal is to speed up sketching.

Feature discussed here : https://forum.freecadweb.org/viewtopic.php?f=9&t=65279
Video showing the feature : https://youtu.be/K8oTQNpI8CQ

One thing to discuss : What key to use for polyline mode switch. 'M' can't be used anymore because it's typed in the spinbox. So I changed to shift. But shift may be in use by others?

Please let me know if you have any question/feedback.

Also add seek third to oblong to select radius point.
Circle, arcs, lines, point, polygone implemented.
Polyline need further work.
Also ESC keypress doesn't work properly when task window has focus. This needs to be corrected
(conflict solved)
Several bugs of tool settings fixed. Includes Oblong, arc and 3points arc.
Keypressed still can't be forwarded to sketchhandler...
Support for polyline improved and mode key replaced by shift.
Solved bugs on spinbox pre-selection of text.
Also update the geometry directly after value entered (no need to move mouse).
@@ -0,0 +1,111 @@
/***************************************************************************
* Copyright (c) 2009 Jürgen Riegel <juergen.riegel@web.de> *
Copy link
Contributor

Choose a reason for hiding this comment

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

Your name. Jurgen has left the project. He does not need more files in his name.

Copy link
Contributor

Choose a reason for hiding this comment

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

He does not need more files in his name.

I don't disagree with the point but I think the tone could be perceived as a little irreverent.

Copy link
Contributor

Choose a reason for hiding this comment

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

That was not the intention. He is just not the author of this file. He probably authored the template this was based on. But the main contributor is Paddle so he should be mentioned. Also the year should probably be updated.

Gui::SelectionSingleton::MessageType Reason);
SketcherToolWidget* widget;

//public Q_SLOTS:
Copy link
Contributor

Choose a reason for hiding this comment

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

No point in having commented out code here.

Copy link
Contributor

@davidosterberg davidosterberg left a comment

Choose a reason for hiding this comment

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

Looks good. Some minor comments. The question of the shift key for the polyline tool is a bit tricky.

@luzpaz luzpaz changed the title SKETCHER : task tool settings widget Sketcher: task tool settings widget Jan 17, 2022
@PaddleStroke
Copy link
Contributor Author

Thanks.
I made the modifications mentioned and solved the over-constraint bug with perpendicular mode of polyline that can be seen in the video.

The last question is for polyline mode shortcut.
For shift : is there other shortcuts using it beside shift + RMB for rotation? If it's the only issue then we can try to catch the +RMB in the polyline tool and cancel the mode change of the tool if + RMB is done.
Beside we can use another key than shift. What about the F3-F12 keys? Or Screenlock or pause (though most laptop won't have those). Other key ideas?

@PaddleStroke
Copy link
Contributor Author

It seems the shortcuts are currently being refactored. So it might be better to keep shift for now until the global shortcut refactoring is done.
Beside the only conflict is with rotating with shift + rmb which is currently not working properly in sketcher edit mode. So it's kind of niche issue and might be solved anyway by the refactoring.

@freecadci
Copy link

pipeline status for feature branch PR_5389. Pipeline 454040201 was triggered at cbc579a. All CI branches and pipelines.

@PaddleStroke PaddleStroke marked this pull request as draft February 4, 2022 14:28
@adrianinsaval
Copy link
Member

any particular reason for closing this one? :(

@PaddleStroke
Copy link
Contributor Author

It's because I build constrain contextually on top of ToolSettings. And I made further improvement of toolsettings on that ConstrainContextuall branch.
So this branch was kind of obsolete. All should be merged in the other branch.

@adrianinsaval
Copy link
Member

considering past statements from the maintainers, I think they would prefer to merge it separately as this feature is independent of the other but better to wait for what they have to say. I think abdullah is who usually reviews sketcher stuff but he's not around for now.

@luzpaz
Copy link
Contributor

luzpaz commented Feb 4, 2022

@abdullahtahiriyo has had IRL (work etc...) stuff going on. But he pops in here and there. Hopefully he can weigh-in on some of @PaddleStroke's work soon

@PaddleStroke
Copy link
Contributor Author

considering past statements from the maintainers, I think they would prefer to merge it separately as this feature is independent of the other but better to wait for what they have to say. I think abdullah is who usually reviews sketcher stuff but he's not around for now.

Yes I know but I had to make lot of modification and fixes to toolsettings while developping constrain contextually. And I couldn't checkout of branches everytime I needed to make a small change (because it takes 1 hour to rebuild everytime). So I ended up making the changes to ToolSettings along the development of Constrain contextually. Which in the end was a lot.
And as Constrain Contextually builds on top of ToolSettings, it had to integrate that branch in the first place.

@adrianinsaval
Copy link
Member

are you mixing stuff within the same commit? if you have separate commits for stuff only related to this widget it would be possible to cherry pick only those. You are developing on windows right? Is there really no equivalent to ccache on windows? That would be a shame, with ccache compilation should require just a few minutes if the difference isn't big.

@PaddleStroke
Copy link
Contributor Author

PaddleStroke commented Feb 4, 2022 via email

@abdullahtahiriyo
Copy link
Contributor

Ok, I will handle it with the other commit. It gets some time before learning how to use git effectively.

@luzpaz luzpaz added WB Sketcher Related to the Sketcher Workbench and removed ✏️ Sketcher labels Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WB Sketcher Related to the Sketcher Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants