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

[Issue] Sketcher Toolbar Configuration not Saved #9208

Closed
2 tasks done
howie-j opened this issue Apr 6, 2023 · 13 comments · Fixed by #9694
Closed
2 tasks done

[Issue] Sketcher Toolbar Configuration not Saved #9208

howie-j opened this issue Apr 6, 2023 · 13 comments · Fixed by #9694
Labels
Bug This issue or PR is related to a bug UI/UX WB Sketcher Related to the Sketcher Workbench

Comments

@howie-j
Copy link
Contributor

howie-j commented Apr 6, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Forums discussion

No response

Version

0.21 (Development)

Full version info

[code]
OS: Fedora Linux 37 (Workstation Edition) (GNOME/gnome)
Word size of FreeCAD: 64-bit
Version: 0.21.0.32764 (Git)
Build type: Release
Branch: master
Hash: 8ea5d9bac10698a9ae31735ad45a63688a6be58b
Python 3.11.2, Qt 5.15.8, Coin 4.0.0, Vtk 9.1.0, OCC 7.6.3
Locale: English/United States (en_US)
[/code]

Subproject(s) affected?

Sketcher

Issue description

Sketcher edit mode toolbars position and visibility is not saved to user.cfg after modification.

Steps to reproduce:

  • Switch to sketcher
  • New sketch, open edit mode
  • Rearrange toolbars
  • Hide toolbars
  • Close FreeCAD

When FreeCAD is restarted all sketcher toolbars are restored to default visibility and position.

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@howie-j
Copy link
Contributor Author

howie-j commented Apr 6, 2023

Reading from a previously configured user.cfg works, but writing the current setup does not.

Added a user.cfg file for testing:
user_cfg.zip

Reading toolbar positions works, but changes are not saved.

@Syres916
Copy link
Contributor

Syres916 commented Apr 9, 2023

From some of my testing it appears to be https://github.com/FreeCAD/FreeCAD/blob/master/src/Gui/ToolBarManager.cpp#L347 section that is causing the regression you are seeing and it's the Sketcher Edit Mode that possibly appears to require an additional condition to not be subject to this Ignore function. @wwmayer am I way off the mark here?

@luzpaz luzpaz added UI/UX WB Sketcher Related to the Sketcher Workbench Bug This issue or PR is related to a bug labels Apr 10, 2023
@adrianinsaval
Copy link
Member

@PaddleStroke may be related to the auto hiding?

@PaddleStroke
Copy link
Contributor

Yes it's related. I think it's a regression from the last fix from as pointed out by Syres916.

@abdullahtahiriyo
Copy link
Contributor

@Syres916

The position of the toolbars does get saved on my side. The visibility of the toolbars does not (if I hide one toolbar, it will appear again if I close FC and reopen it) . Do you have the same behaviour?

@abdullahtahiriyo
Copy link
Contributor

@wwmayer

I am looking at this one.

If I hide the constraints toolbar in sketcher edit mode, when I leave edit mode, Sketcher/Gui/Workbench.cpp, Workbench::leaveEditMode() gets called. There is a comment that the sate must be saved first. There is a call to Gui::ToolBarManager::getInstance()->saveState();. Eventually the constraint bar gets iterated and the ignoreSave lambda gets executed:

// If the toggle action is invisible then it's controlled by the application.

While the toolbar is not visible, the toggleaction isVisible, the comment indicates this is not being controlled by the application, so "do not ignore".

Because for the constraint bar the style is ToolBarItem::HideStyle::FORCE_HIDE, saving gets ignored.

I do not fully grasp how the mechanism is intended to work.

Is sketcher supposed to handle the saving of these toolbars manually? How?

@wwmayer
Copy link
Contributor

wwmayer commented May 28, 2023

The position of the toolbars does get saved on my side. The visibility of the toolbars does not (if I hide one toolbar, it will appear again if I close FC and reopen it) .

Same behaviour here. You don't have to restart FC but switching to another wb and back or leaving edit mode and re-entering shows the same issue.

Because for the constraint bar the style is ToolBarItem::HideStyle::FORCE_HIDE, saving gets ignored.

It's on purpose not to save visibility of toolbars that are managed by the application because otherwise you will end up in a situation where all relevant toolbars are missing. The changes were made with PR #9150 that fixed issue #9135.

But all these kind of problems started with c9aadee

@abdullahtahiriyo
Copy link
Contributor

Thanks Werner.

I was missing the last commit where all originated.

I will then look at it from a different perspective.

abdullahtahiriyo added a commit to abdullahtahiriyo/FreeCAD_sf_master that referenced this issue May 29, 2023
…t preserved

=================================================================================

fixes FreeCAD#9208

Essentially:
- The regular mechanism to save toolbar state when changing from one WB to another
is not designed to support changes within one WB (e.g. from edit mode and back).
- At creation time, toolbars can be initialised with default visible or default hidden
state. Additionally, there is third configuration "Unavailable", which refers to a
toolbar that is hidden, and the control to enable it is also hidden by default.
- The ToolBarManager is extended to enable to set the State of one or more toolbars.
- The State refers to changes to be effected on one or more toolbars by client code:
* ForceHidden allows to hide a toolbar and also hide its control (a toolbar not available
in a mode).
* ForceAvailable allows to make a toolbar available by making its control visible, the
toolbar itself is visible or not depending on user settings.
* RestoreDefault allows to bring the control visibility to the default of the toolbar, the
toolbar itself is visible or not depending on user settings.
* SaveState allows to store the current visibility state of a toolbar. It enables client
code to save the state when appropriate. It provides the only option for default "Unavailable"
toolbars, which are fully managed by client code. It provides additional flexibility to save
other toolbar visibility on request.

For the Sketcher this means:
- That edit mode toolbars are not shown outside edit mode.
- That edit mode toolbars and non-edit mode toolbars can be configured independently.
- that edit mode toolbars' state is saved when leaving edit mode if and only if, the
workbench that is selected when leaving edit mode is the Sketcher WB.
- it won't save the state if the user manually selected another WB and then left edit
mode (why? see limitation above).

Limitation:
- When switching to another WB while in edit mode, the other WB is activated before the
current WB (sketcher WB) is deactivated. This means that at sketcher level, the sketcher
has no chance to save states or do other tidy up actions before the tools of the other WB
are activated.
- This, however, is understood as not relevant enough as to warrant changing the mechanisms
in place.
@abdullahtahiriyo
Copy link
Contributor

@Syres916
@howie-j

Feel free to give it a run to the PR above and let me know if it does not meet expectations.

@howie-j
Copy link
Contributor Author

howie-j commented May 29, 2023

Thanks for your work @abdullahtahiriyo, just tested pr #9694. It fixes the problem as described, and it seems to save toolbar states even if you close freecad in sketch edit mode (sketcher leaves edit mode when you get the promt to save file).

However i noticed one issue: on a clean install (no user.cfg exists), the default toolbar visibility is not written to user.cfg, meaning you have no toolbars as default:
no_toolbars

abdullahtahiriyo added a commit to abdullahtahiriyo/FreeCAD_sf_master that referenced this issue May 30, 2023
================================================================

FreeCAD#9208 (comment)

Issue:
- Unavailable toolbars default to not visible (as they are unavailable)
- However, when made available, they remain not visible. There may be
  cases when this is indeed the wanted behaviour, but they are not the
  ones we are facing.
- In the latter, the user expects the toolbars to show (and the
  configuration file to be saved with all toolbars showing.

Solution:
- Change the default behaviour of Unavailable toolbars when forcing
  them to be available, so that they are visible when made available.
- If then the case arises that we have toolbars where we would prefer
  then to be left not visible when making them available (so they are
  listed in the toolbar context menu to make them visible, but not
  visible per default), then we would need to create another toolbar
  policy class "UnavailableHidden".
- I chose not to add it directly (although it is straightforward) in
  fear that we will never need it, and it would make the code more
  complex to understand and thus maintain.
@abdullahtahiriyo
Copy link
Contributor

@howie-j
@Syres916

Thank you. Yes, indeed the edit-mode toolbars defaulted to not visible when entering sketch edit mode.

I have pushed a fix.

Please give it a run and let me know if there are other outstanding issues.

@abdullahtahiriyo
Copy link
Contributor

@adrianinsaval

I saw your comment in the PR.

It will save state every time edit mode is left with the "Sketcher WB" workbench being the active WB. No matter how. Closing FC included. Of course, if you kill the process (or FreeCAD segfaults), it does not quite qualify as "leaving edit mode" and then it will not be saved. But that is probably beyond user expectations.

Give it a run if you have time and let me know if it misbehaves.

@Syres916
Copy link
Contributor

Thanks for the efforts @abdullahtahiriyo , I've tried the PR using my normal workflow of Sketcher, Part, Draft along with some macros and it appears to be fixed for me. I'll keep working with it for the next few days but I for one am happy for the PR to be merged.

abdullahtahiriyo added a commit to abdullahtahiriyo/FreeCAD_sf_master that referenced this issue May 30, 2023
… preserved

=================================================================================

fixes FreeCAD#9208

Essentially:
- The regular mechanism to save toolbar state when changing from one WB to another
is not designed to support changes within one WB (e.g. from edit mode and back).
- At creation time, toolbars can be initialised with default visible or default hidden
state. Additionally, there is third configuration "Unavailable", which refers to a
toolbar that is hidden, and the control to enable it is also hidden by default.
- The ToolBarManager is extended to enable to set the State of one or more toolbars.
- The State refers to changes to be effected on one or more toolbars by client code:
* ForceHidden allows to hide a toolbar and also hide its control (a toolbar not available
in a mode).
* ForceAvailable allows to make a toolbar available by making its control visible, the
toolbar itself is visible or not depending on user settings.
* RestoreDefault allows to bring the control visibility to the default of the toolbar, the
toolbar itself is visible or not depending on user settings.
* SaveState allows to store the current visibility state of a toolbar. It enables client
code to save the state when appropriate. It provides the only option for default "Unavailable"
toolbars, which are fully managed by client code. It provides additional flexibility to save
other toolbar visibility on request.

For the Sketcher this means:
- That edit mode toolbars are not shown outside edit mode.
- That edit mode toolbars and non-edit mode toolbars can be configured independently.
- that edit mode toolbars' state is saved when leaving edit mode if and only if, the
workbench that is selected when leaving edit mode is the Sketcher WB.
- it won't save the state if the user manually selected another WB and then left edit
mode (why? see limitation above).

Limitation:
- When switching to another WB while in edit mode, the other WB is activated before the
current WB (sketcher WB) is deactivated. This means that at sketcher level, the sketcher
has no chance to save states or do other tidy up actions before the tools of the other WB
are activated.
- This, however, is understood as not relevant enough as to warrant changing the mechanisms
in place.
abdullahtahiriyo added a commit to abdullahtahiriyo/FreeCAD_sf_master that referenced this issue May 30, 2023
================================================================

FreeCAD#9208 (comment)

Issue:
- Unavailable toolbars default to not visible (as they are unavailable)
- However, when made available, they remain not visible. There may be
  cases when this is indeed the wanted behaviour, but they are not the
  ones we are facing.
- In the latter, the user expects the toolbars to show (and the
  configuration file to be saved with all toolbars showing.

Solution:
- Change the default behaviour of Unavailable toolbars when forcing
  them to be available, so that they are visible when made available.
- If then the case arises that we have toolbars where we would prefer
  then to be left not visible when making them available (so they are
  listed in the toolbar context menu to make them visible, but not
  visible per default), then we would need to create another toolbar
  policy class "UnavailableHidden".
- I chose not to add it directly (although it is straightforward) in
  fear that we will never need it, and it would make the code more
  complex to understand and thus maintain.
abdullahtahiriyo added a commit that referenced this issue May 30, 2023
… preserved

=================================================================================

fixes #9208

Essentially:
- The regular mechanism to save toolbar state when changing from one WB to another
is not designed to support changes within one WB (e.g. from edit mode and back).
- At creation time, toolbars can be initialised with default visible or default hidden
state. Additionally, there is third configuration "Unavailable", which refers to a
toolbar that is hidden, and the control to enable it is also hidden by default.
- The ToolBarManager is extended to enable to set the State of one or more toolbars.
- The State refers to changes to be effected on one or more toolbars by client code:
* ForceHidden allows to hide a toolbar and also hide its control (a toolbar not available
in a mode).
* ForceAvailable allows to make a toolbar available by making its control visible, the
toolbar itself is visible or not depending on user settings.
* RestoreDefault allows to bring the control visibility to the default of the toolbar, the
toolbar itself is visible or not depending on user settings.
* SaveState allows to store the current visibility state of a toolbar. It enables client
code to save the state when appropriate. It provides the only option for default "Unavailable"
toolbars, which are fully managed by client code. It provides additional flexibility to save
other toolbar visibility on request.

For the Sketcher this means:
- That edit mode toolbars are not shown outside edit mode.
- That edit mode toolbars and non-edit mode toolbars can be configured independently.
- that edit mode toolbars' state is saved when leaving edit mode if and only if, the
workbench that is selected when leaving edit mode is the Sketcher WB.
- it won't save the state if the user manually selected another WB and then left edit
mode (why? see limitation above).

Limitation:
- When switching to another WB while in edit mode, the other WB is activated before the
current WB (sketcher WB) is deactivated. This means that at sketcher level, the sketcher
has no chance to save states or do other tidy up actions before the tools of the other WB
are activated.
- This, however, is understood as not relevant enough as to warrant changing the mechanisms
in place.
abdullahtahiriyo added a commit that referenced this issue May 30, 2023
================================================================

#9208 (comment)

Issue:
- Unavailable toolbars default to not visible (as they are unavailable)
- However, when made available, they remain not visible. There may be
  cases when this is indeed the wanted behaviour, but they are not the
  ones we are facing.
- In the latter, the user expects the toolbars to show (and the
  configuration file to be saved with all toolbars showing.

Solution:
- Change the default behaviour of Unavailable toolbars when forcing
  them to be available, so that they are visible when made available.
- If then the case arises that we have toolbars where we would prefer
  then to be left not visible when making them available (so they are
  listed in the toolbar context menu to make them visible, but not
  visible per default), then we would need to create another toolbar
  policy class "UnavailableHidden".
- I chose not to add it directly (although it is straightforward) in
  fear that we will never need it, and it would make the code more
  complex to understand and thus maintain.
abdullahtahiriyo added a commit to abdullahtahiriyo/FreeCAD_sf_master that referenced this issue Jun 1, 2023
… preserved

=================================================================================

fixes FreeCAD#9208

Essentially:
- The regular mechanism to save toolbar state when changing from one WB to another
is not designed to support changes within one WB (e.g. from edit mode and back).
- At creation time, toolbars can be initialised with default visible or default hidden
state. Additionally, there is third configuration "Unavailable", which refers to a
toolbar that is hidden, and the control to enable it is also hidden by default.
- The ToolBarManager is extended to enable to set the State of one or more toolbars.
- The State refers to changes to be effected on one or more toolbars by client code:
* ForceHidden allows to hide a toolbar and also hide its control (a toolbar not available
in a mode).
* ForceAvailable allows to make a toolbar available by making its control visible, the
toolbar itself is visible or not depending on user settings.
* RestoreDefault allows to bring the control visibility to the default of the toolbar, the
toolbar itself is visible or not depending on user settings.
* SaveState allows to store the current visibility state of a toolbar. It enables client
code to save the state when appropriate. It provides the only option for default "Unavailable"
toolbars, which are fully managed by client code. It provides additional flexibility to save
other toolbar visibility on request.

For the Sketcher this means:
- That edit mode toolbars are not shown outside edit mode.
- That edit mode toolbars and non-edit mode toolbars can be configured independently.
- that edit mode toolbars' state is saved when leaving edit mode if and only if, the
workbench that is selected when leaving edit mode is the Sketcher WB.
- it won't save the state if the user manually selected another WB and then left edit
mode (why? see limitation above).

Limitation:
- When switching to another WB while in edit mode, the other WB is activated before the
current WB (sketcher WB) is deactivated. This means that at sketcher level, the sketcher
has no chance to save states or do other tidy up actions before the tools of the other WB
are activated.
- This, however, is understood as not relevant enough as to warrant changing the mechanisms
in place.
abdullahtahiriyo added a commit to abdullahtahiriyo/FreeCAD_sf_master that referenced this issue Jun 1, 2023
================================================================

FreeCAD#9208 (comment)

Issue:
- Unavailable toolbars default to not visible (as they are unavailable)
- However, when made available, they remain not visible. There may be
  cases when this is indeed the wanted behaviour, but they are not the
  ones we are facing.
- In the latter, the user expects the toolbars to show (and the
  configuration file to be saved with all toolbars showing.

Solution:
- Change the default behaviour of Unavailable toolbars when forcing
  them to be available, so that they are visible when made available.
- If then the case arises that we have toolbars where we would prefer
  then to be left not visible when making them available (so they are
  listed in the toolbar context menu to make them visible, but not
  visible per default), then we would need to create another toolbar
  policy class "UnavailableHidden".
- I chose not to add it directly (although it is straightforward) in
  fear that we will never need it, and it would make the code more
  complex to understand and thus maintain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This issue or PR is related to a bug UI/UX WB Sketcher Related to the Sketcher Workbench
7 participants