Draft toggle grid as a toggle button to indicate grid visibility

[furgo16]

Fixes #13527

Makes the button for the Draft_ToggleGrid command act as a real toggle button:

• Raised: the grid is not visible
• Depressed: the grid is visible

Similar in spirit to the behavior of the Sketcher_Grid command, which is also a toggle button.

Submitting as authored and suggested by @Syres916 at #13527 (comment).

[FEA-eng]

Nice improvement, I can confirm that it works properly. If the PR is ready to merge, can you undraft it?

[MisterMakerNL]

CAn we set the color of this grid already in a preference pack or stylesheet?

[furgo16]

@FEA-eng thank you for testing. Credit (as per commit authorship) goes to Syres916. I intentionally submitted it as draft, as there are a couple of pieces I want to look at to better understand them before requesting a review. However, if anyone wants to test or review it already, that will be welcome too!

@MisterMakerNL I'm not sure I understand the question. Do you mean if the grid itself is themeable? If so, I don't have the answer, this PR deals only with the button to toggle the grid on and off.

[Roy-043]

Thanks for working on this. There are some issues. The code does not handle multiple documents properly.

Scenario:

1. Start FreeCAD.
2. Create a document.
3. Switch to the Draft WB.
4. Grid is visible. Grid button appears checked. OK.
5. Create another document.
6. Grid is visible. Grid button appears checked. OK.
7. Switch off the grid in the 2nd document.
8. Switch back to the 1st document.
9. Grid is visible. Grid button appears unchecked. Problem.
10. Push the Grid button.
11. Grid is invisible. Grid button appears unchecked. OK.
12. Start a command, for example Draft_Circle.
13. Grid is visible. Grid button appears unchecked. Problem.

I am surprised that both Grid buttons are updated separately. They belong to the same action.

[Roy-043]

Another problem:

1. Start FreeCAD.
2. Create a document.
3. Switch to the Draft WB.
4. Grid is visible. Grid button appears checked in the toolbar, but not in the status bar.

Also the entry in the Utilities menu is not updated.

Maybe handling this via the action is better. But I am not sure how that can be accomplished.
action = Gui.Command.get("Draft_ToggleGrid").getAction()[0]

[Roy-043]

This works (tested with the current dev version, not with the code of this PR):

action = Gui.Command.get("Draft_ToggleGrid").getAction()[0]
action.setCheckable(True)
action.setChecked(True)
action.setChecked(False)

OS: Windows 8 build 9600
Word size of FreeCAD: 64-bit
Version: 0.22.0dev.37302 (Git)
Build type: Release
Branch: main
Hash: 0e24e121eb3e5708c380596e0a9fd583befac977
Python 3.11.9, Qt 5.15.13, Coin 4.0.2, Vtk 9.2.6, OCC 7.7.2
Locale: C/Default (C) [ OS: Dutch/Netherlands (nl_NL) ]
Installed mods:

[maxwxyz]

@furgo16 I know this is a new PR but also on this one the heads up for the upcoming feature freeze on 2024-06-03

[furgo16]

@Roy-043 thanks. That looks promising, as it sets the button status on all toolbars at once. However, as a quick test I simply added the following to the current _find_grid_toolbutton:

        action = FreeCADGui.Command.get("Draft_ToggleGrid").getAction()[0]
        action.setCheckable(tbButtonEnabled)
        action.setChecked(tbButtonChecked)

I'm not sure if it's the actual cause, but having that last statement (action.setChecked()) results in UI freeze due to maximum recursion depth exceeded.

Traceback (most recent call last):
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/draftguitools/gui_grid.py", line 91, in Activated
    WorkingPlane.get_working_plane()
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/WorkingPlane.py", line 1752, in get_working_plane
    wp._update_all(_hist_add=False)
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/WorkingPlane.py", line 1687, in _update_all
    self._update_grid()
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/WorkingPlane.py", line 1724, in _update_grid
    FreeCADGui.Snapper.setGrid()
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/draftguitools/gui_snapper.py", line 1585, in setGrid
    self.setTrackers()
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/draftguitools/gui_snapper.py", line 1590, in setTrackers
    v = Draft.get3DView()
  File "$HOME/dev/furgo16/FreeCAD/build/Mod/Draft/draftutils/gui_utils.py", line 67, in get_3d_view
    import FreeCADGui as Gui
RecursionError: maximum recursion depth exceeded
Running the Python command 'Draft_ToggleGrid' failed, try to resume 

[Roy-043]

Try removing:
"Checkable": False,
From gui_grid.py.

[furgo16]

FIXME: I need to find out where this comes from and if it's needed. At the moment, it seems the index parameter is unused.

[furgo16]

Update after a quick test:

1. The code does not handle multiple documents properly (review comment) => can reproduce
2. Toolbar and status bar buttons out of sync (review comment) => cannot reproduce
3. Also the entry in the Utilities menu is not updated (review comment) => cannot reproduce

I'm not sure where to start looking to address the first point. For the rest and general testing, if someone who's interested in this feature wants to give this a go and report back, feedback will be welcome.

[Roy-043]

Switching to the action solution has indeed fixed some issues.

I don't think there is a need to change gui_grid.py. This may have been required before though.

The references to the Draft Tray and the draftToolBar (which both point to the same GUI element) are not relevant. The code does not interact with the Tray.

_update_grid_gui:
The function needs checking, this apart from the multi-document issue:

• gui_utils.get_3d_view() is used as a check twice. The 2nd check is superfluous.
• If there is a 3D view it follows that the GUI is up. The check FreeCAD.GuiUp is therefore strange.

_set_grid_button_state:
action.setCheckable(button_enable) can just become action.setCheckable(True)

It may be worth considering merging the code with WorkingPlane.py. We can then use a single view observer. And there is the intention to make the grid a property of the working plane in the future.

I can help with the multi-document issue if you want. Either with a pointer (look there...) or with code. Just let me know.

[furgo16]

I don't think there is a need to change gui_grid.py. This may have been required before though.

1. I'll double check that.

The references to the Draft Tray and the draftToolBar (which both point to the same GUI element) are not relevant. The code does not interact with the Tray.

2. Understood. I had just left the comment in there, since the observer code on this PR was originally based on that other one.

_update_grid_gui:
The function needs checking, this apart from the multi-document issue:

gui_utils.get_3d_view() is used as a check twice. The 2nd check is superfluous.
If there is a 3D view it follows that the GUI is up. The check FreeCAD.GuiUp is therefore strange.

3. I'll double-check that too.

It may be worth considering merging the code with WorkingPlane.py. We can then use a single view observer. And there is the intention to make the grid a property of the working plane in the future.

4. That makes sense. Given the time, my (limited) current skills and knowledge of the FreeCAD codebase, I'd do this on the next iteration on a separate PR.

I can help with the multi-document issue if you want. Either with a pointer (look there...) or with code. Just let me know.

Thanks. We can start with the "teach a man how to fish" approach with pointers, and I can ask for further help where I get stuck. That said, I'll be cutting down my contribution time in the next few weeks, but I'll do my best to wrap up this PR before the 1.0 feature freeze.

[Roy-043]

OK, the pointer then 😉

FreeCADGui.Snapper.grid is not updated as you switch views. It contains the grid belonging to the view that was active when the last Draft command was executed. To get access to the grid belonging to a view you need to use the FreeCADGui.Snapper.trackers list. The setTrackers function shows how this list is constructed.

FreeCAD/src/Mod/Draft/draftguitools/gui_snapper.py

Line 1588 in 1da8e17

def setTrackers(self, update_grid=True):