Differential D19776 Diff 54163 libs/ui/kis_mirror_manager.cpp

Changeset View

Standalone View

View Options

libs/ui/kis_mirror_manager.cpp

Show All 27 Lines
28#include <kactioncollection.h> 28​#include <kactioncollection.h>
29#include <QAction> 29​#include <QAction>
30 30​
31#include "kis_canvas2.h" 31​#include "kis_canvas2.h"
32#include "kis_mirror_axis.h" 32​#include "kis_mirror_axis.h"
33#include <KisMirrorAxisConfig.h> 33​#include <KisMirrorAxisConfig.h>
34#include <KisDocument.h> 34​#include <KisDocument.h>
35#include <kis_signals_blocker.h> 35​#include <kis_signals_blocker.h>
36 36​#include <kis_types.h>
37class KisMirrorManager::Private ​
38{ ​
39public: ​
40 Private() ​
41 : mirrorAxisDecoration(nullptr) ​
42 {} ​
43 ​
44 KisMirrorAxis* mirrorAxisDecoration; ​
45}; ​
46 37​
47KisMirrorManager::KisMirrorManager(KisViewManager* view) : QObject(view) 38​KisMirrorManager::KisMirrorManager(KisViewManager* view) : QObject(view)
48 , d(new Private()) ​
49 , m_imageView(0) 39​ , m_imageView(0)
50{ 40​{
51} 41​}
52 42​
53KisMirrorManager::~KisMirrorManager() 43​KisMirrorManager::~KisMirrorManager()
54{ 44​{
55} 45​}
56 46​
Show All 10 Lines
67} 57​}
68 58​
69void KisMirrorManager::setView(QPointer<KisView> imageView) 59​void KisMirrorManager::setView(QPointer<KisView> imageView)
70{ 60​{
71 if (m_imageView) { 61​ if (m_imageView) {
72 m_mirrorCanvas->disconnect(); 62​ m_mirrorCanvas->disconnect();
73 m_imageView->document()->disconnect(); 63​ m_imageView->document()->disconnect();
74 64​
75 d->mirrorAxisDecoration->disconnect(); 65​ KisMirrorAxisSP canvasDecoration = this->decoration();
76 d->mirrorAxisDecoration = nullptr; 66​ if (canvasDecoration) {
67​ canvasDecoration->disconnect();
68​ }
77 } 69​ }
78 70​
79 m_imageView = imageView; 71​ m_imageView = imageView;
80 72​
81 if (m_imageView) { 73​ if (m_imageView) {
82 connect(m_mirrorCanvas, SIGNAL(toggled(bool)), dynamic_cast<KisCanvasController*>(m_imageView->canvasController()), SLOT(mirrorCanvas(bool))); 74​ connect(m_mirrorCanvas, SIGNAL(toggled(bool)), dynamic_cast<KisCanvasController*>(m_imageView->canvasController()), SLOT(mirrorCanvas(bool)));
83 connect(m_imageView->document(), SIGNAL(sigMirrorAxisConfigChanged()), this, SLOT(slotDocumentConfigChanged()), Qt::UniqueConnection); 75​ connect(m_imageView->document(), SIGNAL(sigMirrorAxisConfigChanged()), this, SLOT(slotDocumentConfigChanged()), Qt::UniqueConnection);
84 76​
85 KisMirrorAxis* decoration; 77​ KisMirrorAxisSP canvasDecoration = this->decoration();
86 if (m_imageView->canvasBase() && m_imageView->canvasBase()->decoration("mirror_axis")) { 78​ if (!canvasDecoration) {
87 decoration = dynamic_cast<KisMirrorAxis*>(m_imageView->canvasBase()->decoration("mirror_axis").data()); 79​ KisMirrorAxis* decoration = new KisMirrorAxis(m_imageView->viewManager()->canvasResourceProvider(), m_imageView);
88 } else { ​
89 decoration = new KisMirrorAxis(m_imageView->viewManager()->canvasResourceProvider(), m_imageView); ​
90 connect(decoration, SIGNAL(sigConfigChanged()), this, SLOT(slotMirrorAxisConfigChanged()), Qt::UniqueConnection); 80​ connect(decoration, SIGNAL(sigConfigChanged()), this, SLOT(slotMirrorAxisConfigChanged()), Qt::UniqueConnection);
91 m_imageView->canvasBase()->addDecoration(decoration); 81​ m_imageView->canvasBase()->addDecoration(decoration);
82​ } else {
83​ connect(canvasDecoration.data(), SIGNAL(sigConfigChanged()), this, SLOT(slotMirrorAxisConfigChanged()), Qt::UniqueConnection);
92 } 84​ }
93 85​
94 d->mirrorAxisDecoration = decoration; ​
95 setDecorationConfig(); 86​ setDecorationConfig();
96 } 87​ }
97 88​
98 updateAction(); 89​ updateAction();
99} 90​}
100 91​
101void KisMirrorManager::updateAction() 92​void KisMirrorManager::updateAction()
102{ 93​{
Show All 11 Lines
114{ 105​{
115 setDecorationConfig(); 106​ setDecorationConfig();
116} 107​}
117 108​
118void KisMirrorManager::slotMirrorAxisConfigChanged() 109​void KisMirrorManager::slotMirrorAxisConfigChanged()
119{ 110​{
120 if (m_imageView && m_imageView->document()) { 111​ if (m_imageView && m_imageView->document()) {
121 KisSignalsBlocker blocker(m_imageView->document()); 112​ KisSignalsBlocker blocker(m_imageView->document());
122 m_imageView->document()->setMirrorAxisConfig(d->mirrorAxisDecoration->mirrorAxisConfig()); 113​
114​ KisMirrorAxisSP canvasDecoration = this->decoration();
115​ if (canvasDecoration) {
116​ m_imageView->document()->setMirrorAxisConfig(canvasDecoration->mirrorAxisConfig());
117​ }
118​ }
119​}
120​
121​KisMirrorAxisSP KisMirrorManager::decoration() const
122​{
123​ if (m_imageView) {
124​ return qobject_cast<KisMirrorAxis*>(m_imageView->canvasBase()->decoration("mirror_axis").data());
125​ } else {
126​ return 0;
123 } 127​ }
124} 128​}
125 129​
126void KisMirrorManager::setDecorationConfig() 130​void KisMirrorManager::setDecorationConfig()
127{ 131​{
128 if (m_imageView && m_imageView->document()) { 132​ if (m_imageView && m_imageView->document()) {
129 KisMirrorAxisConfig config = m_imageView->document()->mirrorAxisConfig(); 133​ KisMirrorAxisConfig config = m_imageView->document()->mirrorAxisConfig();
130 d->mirrorAxisDecoration->setMirrorAxisConfig(config); 134​
135​ KisMirrorAxisSP canvasDecoration = this->decoration();
dkazakovUnsubmitted
Not Done

As a general rule, it is usually preferable to read the pointers from lazily calculated functions into a local variable, and only after that access them. Like that:

KisMirrorAxisSP decoration = this->decoration();
if (decoration) {
    decoration->->setMirrorAxisConfig(config);
}

Such rule guards from weird mistakes, when someone (sometimes the author himself) inserts some code between the two calls, which affects the result of the second call and basically, invalidates the pointer. Such problems may also appear as a result of automatic merges or something like that.

This comment is not a blocker of the patch. Just my opinion on the topic :)

dkazakov: As a general rule, it is usually preferable to read the pointers from lazily calculated…
136​ if (canvasDecoration) {
137​ canvasDecoration->setMirrorAxisConfig(config);
138​ }
131 } 139​ }
132} 140​}