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

wxGUI: Move SbMask widget to main window statusbar #2089

Merged
merged 22 commits into from
Jan 28, 2022

Conversation

lindakarlovska
Copy link
Contributor

@lindakarlovska lindakarlovska commented Jan 13, 2022

It moves statusbar Mask widget to the main statusbar. It is also changing the way Mask is updated - now it is updated using signals and method RefreshMask.

@lindakarlovska lindakarlovska marked this pull request as draft January 13, 2022 15:14
@lindakarlovska lindakarlovska added GUI wxGUI related enhancement New feature or request labels Jan 13, 2022
@lindakarlovska lindakarlovska added this to the 8.2.0 milestone Jan 13, 2022
@@ -351,6 +369,7 @@ def _createConsole(self, parent):
)

self._gconsole.mapCreated.connect(self.OnMapCreated)
self._gconsole.updateMap.connect(self.RefreshMask)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be mapCreated?

@petrasovaa
Copy link
Contributor

You also need to refresh it initially, it doesn't seem to show up when you start GUI and mask exists.
Currently the signals you use don't seem to be sufficient, it doesn't update when I would expect it to do, I would need to look at it more. Don't forget also about the GConsole in forms.py.

@lindakarlovska
Copy link
Contributor Author

You also need to refresh it initially, it doesn't seem to show up when you start GUI and mask exists. Currently the signals you use don't seem to be sufficient, it doesn't update when I would expect it to do, I would need to look at it more. Don't forget also about the GConsole in forms.py.

I agree. I was also thinking about GSConsole in forms.py - I think we should add r.mask to r.colours and other items in a list. But the problem is that the map returned from r.mask is the map from which the mask was created - not the resulting MASK. So we would probably need to update a bit a way r.mask works?..

@lindakarlovska
Copy link
Contributor Author

I have prepared the list of situations when the Mask red button should be refreshed - or better said situations when we need to check whether the MASK file exists. Please @petrasovaa check it and add something if missing:

  1. GRASS launched - the mask is refreshed internally in its constructor
  2. The Mask is created by the r.mask module - in this case, Mask red button should be refreshed as well as Data Catalog
  3. The Mask is removed by removing the MASK layer from the Data Catalog tree - in this case, Mask red button should be refreshed as well as Data Catalog
  4. The Mask is removed by clicking on the red Mask button - in this case, Mask red button should be refreshed as well as Data Catalog
  5. The mapset without MASK has been switched - a new mapset may contain the MASK so the Mask red button should be refreshed
  6. The mapset with MASK has been switched - a new mapset may not contain the MASK so the Mask red button should be refreshed

Probably all, but I am not sure about workspaces...

@petrasovaa
Copy link
Contributor

I have prepared the list of situations when the Mask red button should be refreshed - or better said situations when we need to check whether the MASK file exists. Please @petrasovaa check it and add something if missing:

Any raster/vector module that creates a raster layer can create MASK layer, probably not how most people would do that, but possible.

@petrasovaa
Copy link
Contributor

I suggest these changes, that should cover most cases

diff --git a/gui/wxpython/lmgr/frame.py b/gui/wxpython/lmgr/frame.py
index 3670b38a2..4184abc83 100644
--- a/gui/wxpython/lmgr/frame.py
+++ b/gui/wxpython/lmgr/frame.py
@@ -223,7 +223,6 @@ class GMFrame(wx.Frame):
 
         self._giface.mapCreated.connect(self.OnMapCreated)
         self._giface.updateMap.connect(self._updateCurrentMap)
-        self._giface.updateMap.connect(self.mask.Refresh)
         self._giface.currentMapsetChanged.connect(self.OnMapsetChanged)
 
         # minimal frame size
@@ -428,7 +427,7 @@ class GMFrame(wx.Frame):
         )
 
         self._gconsole.mapCreated.connect(self.OnMapCreated)
-        self._gconsole.mapCreated.connect(self.mask.Refresh)
+        self._gconsole.updateMap.connect(self.mask.Refresh)
         self._gconsole.Bind(
             EVT_IGNORED_CMD_RUN, lambda event: self.RunSpecialCmd(event.cmd)
         )
diff --git a/gui/wxpython/lmgr/statusbar.py b/gui/wxpython/lmgr/statusbar.py
index 62a263447..993581a28 100644
--- a/gui/wxpython/lmgr/statusbar.py
+++ b/gui/wxpython/lmgr/statusbar.py
@@ -38,8 +38,14 @@ class SbMask:
         self.widget.Bind(wx.EVT_BUTTON, self.OnRemoveMask)
         self.widget.SetForegroundColour(wx.Colour(255, 0, 0))
         self.widget.SetToolTip(tip=_("Left mouse click to remove the MASK"))
+        self.giface.updateMap.connect(self.Refresh)
+        self.giface.grassdbChanged.connect(self._dbChanged)
         self.Refresh()
 
+    def _dbChanged(self, map=None, newname=None):
+        if map == "MASK" or newname == "MASK":
+            self.Refresh()
+
     def Show(self):
         """Invokes showing of underlying widget.
 
diff --git a/gui/wxpython/main_window/frame.py b/gui/wxpython/main_window/frame.py
index 44bfdc304..c46bf56c1 100644
--- a/gui/wxpython/main_window/frame.py
+++ b/gui/wxpython/main_window/frame.py
@@ -166,7 +166,6 @@ class GMFrame(wx.Frame):
 
         self._giface.mapCreated.connect(self.OnMapCreated)
         self._giface.updateMap.connect(self._updateCurrentMap)
-        self._giface.updateMap.connect(self.mask.Refresh)
         self._giface.currentMapsetChanged.connect(self.OnMapsetChanged)
 
         # use default window layout ?

@lindakarlovska
Copy link
Contributor Author

I suggest these changes, that should cover most cases

diff --git a/gui/wxpython/lmgr/frame.py b/gui/wxpython/lmgr/frame.py
index 3670b38a2..4184abc83 100644
--- a/gui/wxpython/lmgr/frame.py
+++ b/gui/wxpython/lmgr/frame.py
@@ -223,7 +223,6 @@ class GMFrame(wx.Frame):
 
         self._giface.mapCreated.connect(self.OnMapCreated)
         self._giface.updateMap.connect(self._updateCurrentMap)
-        self._giface.updateMap.connect(self.mask.Refresh)
         self._giface.currentMapsetChanged.connect(self.OnMapsetChanged)
 
         # minimal frame size
@@ -428,7 +427,7 @@ class GMFrame(wx.Frame):
         )
 
         self._gconsole.mapCreated.connect(self.OnMapCreated)
-        self._gconsole.mapCreated.connect(self.mask.Refresh)
+        self._gconsole.updateMap.connect(self.mask.Refresh)
         self._gconsole.Bind(
             EVT_IGNORED_CMD_RUN, lambda event: self.RunSpecialCmd(event.cmd)
         )
diff --git a/gui/wxpython/lmgr/statusbar.py b/gui/wxpython/lmgr/statusbar.py
index 62a263447..993581a28 100644
--- a/gui/wxpython/lmgr/statusbar.py
+++ b/gui/wxpython/lmgr/statusbar.py
@@ -38,8 +38,14 @@ class SbMask:
         self.widget.Bind(wx.EVT_BUTTON, self.OnRemoveMask)
         self.widget.SetForegroundColour(wx.Colour(255, 0, 0))
         self.widget.SetToolTip(tip=_("Left mouse click to remove the MASK"))
+        self.giface.updateMap.connect(self.Refresh)
+        self.giface.grassdbChanged.connect(self._dbChanged)
         self.Refresh()
 
+    def _dbChanged(self, map=None, newname=None):
+        if map == "MASK" or newname == "MASK":
+            self.Refresh()
+
     def Show(self):
         """Invokes showing of underlying widget.
 
diff --git a/gui/wxpython/main_window/frame.py b/gui/wxpython/main_window/frame.py
index 44bfdc304..c46bf56c1 100644
--- a/gui/wxpython/main_window/frame.py
+++ b/gui/wxpython/main_window/frame.py
@@ -166,7 +166,6 @@ class GMFrame(wx.Frame):
 
         self._giface.mapCreated.connect(self.OnMapCreated)
         self._giface.updateMap.connect(self._updateCurrentMap)
-        self._giface.updateMap.connect(self.mask.Refresh)
         self._giface.currentMapsetChanged.connect(self.OnMapsetChanged)
 
         # use default window layout ?

I agree with that, in addition, I suggest adding self.giface.currentMapsetChanged.connect(self.Refresh) to the lgmr/statusbar.py

@lindakarlovska
Copy link
Contributor Author

I suggest these changes, that should cover most cases

diff --git a/gui/wxpython/lmgr/frame.py b/gui/wxpython/lmgr/frame.py
index 3670b38a2..4184abc83 100644
--- a/gui/wxpython/lmgr/frame.py
+++ b/gui/wxpython/lmgr/frame.py
@@ -223,7 +223,6 @@ class GMFrame(wx.Frame):
 
         self._giface.mapCreated.connect(self.OnMapCreated)
         self._giface.updateMap.connect(self._updateCurrentMap)
-        self._giface.updateMap.connect(self.mask.Refresh)
         self._giface.currentMapsetChanged.connect(self.OnMapsetChanged)
 
         # minimal frame size
@@ -428,7 +427,7 @@ class GMFrame(wx.Frame):
         )
 
         self._gconsole.mapCreated.connect(self.OnMapCreated)
-        self._gconsole.mapCreated.connect(self.mask.Refresh)
+        self._gconsole.updateMap.connect(self.mask.Refresh)
         self._gconsole.Bind(
             EVT_IGNORED_CMD_RUN, lambda event: self.RunSpecialCmd(event.cmd)
         )
diff --git a/gui/wxpython/lmgr/statusbar.py b/gui/wxpython/lmgr/statusbar.py
index 62a263447..993581a28 100644
--- a/gui/wxpython/lmgr/statusbar.py
+++ b/gui/wxpython/lmgr/statusbar.py
@@ -38,8 +38,14 @@ class SbMask:
         self.widget.Bind(wx.EVT_BUTTON, self.OnRemoveMask)
         self.widget.SetForegroundColour(wx.Colour(255, 0, 0))
         self.widget.SetToolTip(tip=_("Left mouse click to remove the MASK"))
+        self.giface.updateMap.connect(self.Refresh)
+        self.giface.grassdbChanged.connect(self._dbChanged)
         self.Refresh()
 
+    def _dbChanged(self, map=None, newname=None):
+        if map == "MASK" or newname == "MASK":
+            self.Refresh()
+
     def Show(self):
         """Invokes showing of underlying widget.
 
diff --git a/gui/wxpython/main_window/frame.py b/gui/wxpython/main_window/frame.py
index 44bfdc304..c46bf56c1 100644
--- a/gui/wxpython/main_window/frame.py
+++ b/gui/wxpython/main_window/frame.py
@@ -166,7 +166,6 @@ class GMFrame(wx.Frame):
 
         self._giface.mapCreated.connect(self.OnMapCreated)
         self._giface.updateMap.connect(self._updateCurrentMap)
-        self._giface.updateMap.connect(self.mask.Refresh)
         self._giface.currentMapsetChanged.connect(self.OnMapsetChanged)
 
         # use default window layout ?

I agree with that, in addition, I suggest adding self.giface.currentMapsetChanged.connect(self.Refresh) to the lgmr/statusbar.py

There is also the thing with the Data Catalog grassdb update. It is not ideal but I would suggest adding a few rows to gconsole.py. And also after removing the MASK using the button, we should also emit grassdbChanged signal... , please look at 3f74719

@lindakarlovska lindakarlovska marked this pull request as ready for review January 20, 2022 12:00
@lindakarlovska
Copy link
Contributor Author

I have just finished the last part regarding status bar repositioning, I think we are almost at the end. :-)

Comment on lines 273 to 296
def _createStatusbar(self):
"""Create main window statusbar"""
self.statusbar = wx.StatusBar(self, id=wx.ID_ANY)
self.statusbar.SetMinHeight(24)
self.statusbar.SetFieldsCount(2)
self.statusbar.SetStatusWidths([-1, 100])
self.mask = SbMask(self.statusbar, self._giface)
self._repositionStatusbar()

def _repositionStatusbar(self):
"""Reposition widgets in main window statusbar"""
rect1 = self.statusbar.GetFieldRect(1)
rect1.x += 1
rect1.y += 1
self.mask.GetWidget().SetRect(rect1)

def OnSize(self, event):
"""Adjust main window statusbar on changing size"""
self._repositionStatusbar()

def SetStatusText(self, *args):
"""Overide wx.StatusBar method"""
self.statusbar.SetStatusText(*args)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this into lmgr/statusbar.py, similarly to the SbManager, frame.py is already big.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. :-) I made the class SbMain which encapsulates the creation as well as statusbar reposition..

gui/wxpython/lmgr/statusbar.py Outdated Show resolved Hide resolved
gui/wxpython/lmgr/statusbar.py Outdated Show resolved Hide resolved
dlg.Destroy()
return
RunCommand("r.mask", flags="r")
self.giface.updateMap.emit(render=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to emit updateMap whenever the status changes, maybe in Refresh(). Otherwise map display won't refresh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure. I think it is sufficient to emit updateMap just in gconsole.py. In this case, it refreshes when using the module - so when creating and removing mask through r.mask.
After currentMapsetChanged I think we do not need to update a map and after grassdbChanged a map is updated automatically. Which other possible scenarios are in your mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

try also running r.mask from command console

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

I rewrote some of the signals, let me know if you agree and please test, before that rerendering of map display didn't work.

gui/wxpython/lmgr/frame.py Outdated Show resolved Hide resolved
@lindakarlovska
Copy link
Contributor Author

I rewrote some of the signals, let me know if you agree and please test, before that rerendering of map display didn't work.

Nice, it makes greater sense like that. I have tested it by running r.mask from the command console and I've also tested all other cases and checked the map rendering, data catalog updating, mask button updating. All seem to be working fine.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

Almost there!

gui/wxpython/lmgr/statusbar.py Outdated Show resolved Hide resolved
gui/wxpython/lmgr/statusbar.py Outdated Show resolved Hide resolved
gui/wxpython/lmgr/statusbar.py Outdated Show resolved Hide resolved
lindakarlovska and others added 4 commits January 26, 2022 07:35
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
@petrasovaa petrasovaa merged commit 251bce4 into OSGeo:main Jan 28, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request GUI wxGUI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants