Bug 389033

Summary: "Show Pixel Grid" checkbox state can desync with multiple documents/views
Product: krita Reporter: Nicholas LaPointe <nicholaslapointe8>
Component: UsabilityAssignee: Scott Petrovic <scottpetrovic>
Status: RESOLVED FIXED    
Severity: normal CC: dimula73, freebox64, halla
Priority: NOR    
Version: git master (please specify the git hash!)   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:

Description Nicholas LaPointe 2018-01-16 01:54:19 UTC
The "checked" state of the "Show Pixel Grid" checkbox inside of the "View" menu can be inaccurate when it is toggled in multiple documents or views.

(Steps assume that the pixel grid is already enabled, which it is by default)
1. Create two documents
2. Disable the pixel grid in one document
3. Switch documents. The pixel grid in this document will still be enabled, but the checkbox is now unchecked.

I'd expect that the state of the checkbox would match the status of the pixel grid on the view (versus the other alternative of the checkbox affecting all views).
Comment 1 mvowada 2018-01-16 10:44:41 UTC
Yes. I can confirm on Ubuntu 14.04 with Krita 4.0.0-beta1.1 and OpenGL enabled.
Comment 2 Halla Rempt 2018-01-17 11:04:29 UTC
Probably caused by 

commit cb1e38118be3e986a252210fa8df61a8fc2d4b18
Author: Scott Petrovic <scottpetrovic@gmail.com>
Date:   Sat Oct 28 09:44:18 2017 -0500

    turned the show pixel grid into an action and moved it to the view menu
Comment 3 Scott Petrovic 2018-01-17 14:25:41 UTC
The pixel grid is on/off based on the kritarc currently. It only currently works globally, not per view.

We will have to code something up so it keeps track of the enabled status by view and remove it from being on/off in the kritarc.
Comment 4 Halla Rempt 2018-01-17 15:18:36 UTC
> The pixel grid is on/off based on the kritarc currently. It only currently
> works globally, not per view.

The weird thing is that there is some per-view code, or so it seems. I think
that's not necessary, actually.
Comment 5 Nicholas LaPointe 2018-01-18 23:11:02 UTC
I could imagine per-view control of it being helpful when a user has multiple views of the same document at different zoom levels, but I don't know how common such a use case would be.
Comment 6 Scott Petrovic 2018-01-19 13:44:54 UTC
Git commit 1a47ae24712377821994c3b0309b9437da129d73 by Scott Petrovic.
Committed on 19/01/2018 at 13:44.
Pushed by scottpetrovic into branch 'master'.

M  +0    -5    libs/ui/KisViewManager.cpp
M  +0    -16   libs/ui/canvas/kis_canvas_controller.cpp
M  +0    -1    libs/ui/canvas/kis_canvas_controller.h
M  +11   -0    libs/ui/kis_paintop_box.cc
M  +3    -0    libs/ui/kis_paintop_box.h
M  +4    -3    libs/ui/opengl/kis_opengl_canvas2.cpp

https://commits.kde.org/krita/1a47ae24712377821994c3b0309b9437da129d73
Comment 7 Scott Petrovic 2018-01-19 13:48:36 UTC
I fixed the bug with it syncing.

For now the pixel grid is going to have to be a global option and not per view. That was how it was coded and the way it is saved/loaded with the kritarc. It is really a settings option that wasn't designed to be in the main menu.

If we start getting requests for people needing it differently per view, that can be a future feature request.
Comment 8 Dmitry Kazakov 2018-01-29 07:57:26 UTC
Hi, Scott!

There is still a bug with sync'ness:

When switching the state of the checkbox in one view, the **state of the checkbox** in another view goes out of sync :(
Comment 9 Dmitry Kazakov 2018-01-29 08:28:57 UTC
Added a review request:
https://phabricator.kde.org/D10171
Comment 10 Dmitry Kazakov 2018-01-30 08:21:51 UTC
Git commit 95b2c02b0bac26cf87788a4ef31c8640635204cf by Dmitry Kazakov.
Committed on 29/01/2018 at 08:27.
Pushed by dkazakov into branch 'master'.

Fix syncing pixel grid state between windows once again

Summary:
This patch extends commit1a47ae24712377821994c3b0309b9437da129d73 and
addresses the following issues:

1) The state of the menu action checkbox was not updated when the other
   window changed the its state
2) There should be **no** access to KisConfig in the OpenGL rendering
   loop (it is slow due to possible disk access).
3) A canvas update call should be issued right after the config changed,
   otherwise the old state of the grid will be visible until one hovers
   the canvas

Test Plan:
1) Open multiple Krita windows and zoom both of them to 1000%
2) Change View->Show Pixel Grid in one of the windows
3) The state on the checkbox and the canvas should automatically
   update in the other window

Reviewers: #krita, scottpetrovic

Differential Revision: https://phabricator.kde.org/D10171

M  +1    -0    libs/ui/KisMainWindow.cpp
M  +16   -0    libs/ui/KisViewManager.cpp
M  +1    -0    libs/ui/KisViewManager.h
M  +9    -0    libs/ui/canvas/kis_canvas_controller.cpp
M  +1    -0    libs/ui/canvas/kis_canvas_controller.h
M  +5    -0    libs/ui/kis_config_notifier.cpp
M  +2    -0    libs/ui/kis_config_notifier.h
M  +0    -11   libs/ui/kis_paintop_box.cc
M  +0    -3    libs/ui/kis_paintop_box.h
M  +15   -6    libs/ui/opengl/kis_opengl_canvas2.cpp
M  +1    -0    libs/ui/opengl/kis_opengl_canvas2.h

https://commits.kde.org/krita/95b2c02b0bac26cf87788a4ef31c8640635204cf