[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: Junior job - zooming tool doesn't zoom out
From: Hans Bakker <hansmbakker () gmail ! com>
Date: 2009-09-23 20:29:52
Message-ID: 761dc76c0909231329t7b4d0371m17c4f1c1fcb95226 () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
This is a resend of the patch so that Cyrille can commit it - unfortunately
my laptop I was developing on is broken and I'm working on a temporary pc
now on which I can't run the virtual pc with my development image :(
Kind regards,
Hans Bakker
2009/9/6 Hans Bakker <hansmbakker@gmail.com>
> Ok, thank you for your comments :)
>
> 2009/9/5 Thomas Zander <zander@kde.org>
> >
> > On Saturday 5. September 2009 12.04.42 Hans Bakker wrote:
> > > Hi all,
> > > I made a beginning of the closing of bug 171976
> > > <http://bugs.kde.org/171976>.
> >
> > nice!
> >
> > > The patch is attached, but there are still
> > > some things left to do that I'd like some help with:
> > >
> > > - when pressing Ctrl, sometimes the cursor is only updated when
> moving
> > > the mouse. Right now at the key*Event methods, event->ignore(); is
> > > called.
> >
> > I notice that on startup neither of the 2 radio buttons are selected and
> > then the ctrl works exactly as expected.
> > Only after selecting one it starts to fail.
> > So I think that the ignore() has nothing to do with it, rather there is a
> > bug in the logic somewhere.
> > I suggest adding some kDebug() statements to figure out what is
> happening.
>
>
> Solved that by setting Qt::NoFocus to the two QRadioButtons in the
> zoom tool options widget so that they don't get the key*Events instead
> of the canvas.
> >
> >
> > > - the birdEyeLabel is not used / updated yet.
> >
> > I suggest to hide the widget (birdEyeLabel->hide()) so it doesn't cause
> the
> > painting issues we see now.
>
> Done
> >
> >
> > Other comments on the patch;
> > please make sure you follow the coding style;
> > http://techbase.kde.org/Policies/Kdelibs_Coding_Style
> >
> > In finishInteraction you duplicated the code; I suggest to instead
> introduce
> > a local boolean that initially gets its value from m_forceZoomOut, but
> that
> > the ctrl key can change. Then you can avoid the duplication.
> >
> > I noticed you added the same 4 lines of 'setCursor' code various times,
> I'm
> > thinking this is a good candidate for a private method. Something like;
> > updateCursor(bool invertZoom);
> > so you can call;
> > updateCursor(event->modifiers() & Qt::ControlModifier);
> > for instance.
>
> Good idea indeed
> >
> >
> > Please re-send the patch after looking into these issues.
>
>
> Done :) Looking forward to your comments.
>
> Kind regards,
>
> Hans Bakker
>
[Attachment #5 (text/html)]
This is a resend of the patch so that Cyrille can commit it - unfortunately my laptop \
I was developing on is broken and I'm working on a temporary pc now on which I \
can't run the virtual pc with my development image :(<br>
<br>Kind regards,<br><br>Hans Bakker<br><br><div class="gmail_quote">2009/9/6 Hans \
Bakker <span dir="ltr"><<a \
href="mailto:hansmbakker@gmail.com">hansmbakker@gmail.com</a>></span><br><blockquote \
class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt \
0pt 0.8ex; padding-left: 1ex;">
Ok, thank you for your comments :)<br>
<br>
2009/9/5 Thomas Zander <<a href="mailto:zander@kde.org">zander@kde.org</a>><br>
<div class="im">><br>
> On Saturday 5. September 2009 12.04.42 Hans Bakker wrote:<br>
> > Hi all,<br>
> > I made a beginning of the closing of bug 171976<br>
> > <<a href="http://bugs.kde.org/171976" \
target="_blank">http://bugs.kde.org/171976</a>>.<br> ><br>
> nice!<br>
><br>
> > The patch is attached, but there are still<br>
> > some things left to do that I'd like some help with:<br>
> ><br>
> > - when pressing Ctrl, sometimes the cursor is only updated when \
moving<br> > > the mouse. Right now at the key*Event methods, \
event->ignore(); is<br> > > called.<br>
><br>
> I notice that on startup neither of the 2 radio buttons are selected and<br>
> then the ctrl works exactly as expected.<br>
> Only after selecting one it starts to fail.<br>
> So I think that the ignore() has nothing to do with it, rather there is a<br>
> bug in the logic somewhere.<br>
> I suggest adding some kDebug() statements to figure out what is happening.<br>
<br>
<br>
</div>Solved that by setting Qt::NoFocus to the two QRadioButtons in the<br>
zoom tool options widget so that they don't get the key*Events instead<br>
of the canvas.<br>
<div class="im">><br>
><br>
> > - the birdEyeLabel is not used / updated yet.<br>
><br>
> I suggest to hide the widget (birdEyeLabel->hide()) so it doesn't cause \
the<br> > painting issues we see now.<br>
<br>
</div>Done<br>
<div class="im">><br>
><br>
> Other comments on the patch;<br>
> please make sure you follow the coding style;<br>
> <a href="http://techbase.kde.org/Policies/Kdelibs_Coding_Style" \
target="_blank">http://techbase.kde.org/Policies/Kdelibs_Coding_Style</a><br> \
><br> > In finishInteraction you duplicated the code; I suggest to instead \
introduce<br> > a local boolean that initially gets its value from m_forceZoomOut, \
but that<br> > the ctrl key can change. Then you can avoid the duplication.<br>
><br>
> I noticed you added the same 4 lines of 'setCursor' code various times, \
I'm<br> > thinking this is a good candidate for a private method. Something \
like;<br> > updateCursor(bool invertZoom);<br>
> so you can call;<br>
> updateCursor(event->modifiers() & Qt::ControlModifier);<br>
> for instance.<br>
<br>
</div>Good idea indeed<br>
<div class="im">><br>
><br>
> Please re-send the patch after looking into these issues.<br>
<br>
<br>
</div>Done :) Looking forward to your comments.<br>
<br>
Kind regards,<br>
<font color="#888888"><br>
Hans Bakker<br>
</font></blockquote></div><br>
--000325559436eff2c50474449478--
["bug171976_juniorjob_corrected.patch" (application/octet-stream)]
Index: libs/flake/tools/KoZoomStrategy.h
===================================================================
--- libs/flake/tools/KoZoomStrategy.h (revision 1019773)
+++ libs/flake/tools/KoZoomStrategy.h (working copy)
@@ -41,6 +41,7 @@
KoZoomStrategy(KoZoomTool *tool, KoCanvasController *controller, const QPointF &clicked);
void forceZoomOut();
+ void forceZoomIn();
/// Execute the zoom
virtual void finishInteraction(Qt::KeyboardModifiers modifiers);
Index: libs/flake/tools/KoZoomToolWidget.ui
===================================================================
--- libs/flake/tools/KoZoomToolWidget.ui (revision 1019773)
+++ libs/flake/tools/KoZoomToolWidget.ui (working copy)
@@ -1,7 +1,8 @@
-<ui version="4.0" >
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
<class>ZoomToolWidget</class>
- <widget class="QWidget" name="ZoomToolWidget" >
- <property name="geometry" >
+ <widget class="QWidget" name="ZoomToolWidget">
+ <property name="geometry">
<rect>
<x>0</x>
<y>0</y>
@@ -9,24 +10,30 @@
<height>261</height>
</rect>
</property>
- <layout class="QGridLayout" >
- <item row="0" column="0" >
- <widget class="QRadioButton" name="zoomInButton" >
- <property name="text" >
+ <layout class="QGridLayout">
+ <item row="0" column="0">
+ <widget class="QRadioButton" name="zoomInButton">
+ <property name="focusPolicy">
+ <enum>Qt::NoFocus</enum>
+ </property>
+ <property name="text">
<string>Zoom In</string>
</property>
</widget>
</item>
- <item row="0" column="1" >
- <widget class="QRadioButton" name="zoomOutButton" >
- <property name="text" >
+ <item row="0" column="1">
+ <widget class="QRadioButton" name="zoomOutButton">
+ <property name="focusPolicy">
+ <enum>Qt::NoFocus</enum>
+ </property>
+ <property name="text">
<string>Zoom Out</string>
</property>
</widget>
</item>
- <item row="1" column="0" colspan="2" >
- <widget class="QLabel" name="birdEyeLabel" >
- <property name="text" >
+ <item row="1" column="0" colspan="2">
+ <widget class="QLabel" name="birdEyeLabel">
+ <property name="text">
<string/>
</property>
</widget>
Index: libs/flake/tools/KoZoomTool.cpp
===================================================================
--- libs/flake/tools/KoZoomTool.cpp (revision 1019773)
+++ libs/flake/tools/KoZoomTool.cpp (working copy)
@@ -31,7 +31,7 @@
KoZoomTool::KoZoomTool(KoCanvasBase *canvas)
: KoInteractionTool(canvas),
- m_temporary(false)
+ m_temporary(false), m_zoomInMode(true)
{
QPixmap inPixmap, outPixmap;
inPixmap.load(KStandardDirs::locate("data", "koffice/icons/zoom_in_cursor.png"));
@@ -49,46 +49,40 @@
void KoZoomTool::mouseReleaseEvent(KoPointerEvent *event)
{
KoInteractionTool::mouseReleaseEvent(event);
- if (m_temporary)
+ if (m_temporary) {
emit KoTool::done();
+ }
}
void KoZoomTool::mouseMoveEvent(KoPointerEvent *event)
{
- if (event->modifiers() & Qt::ControlModifier)
- useCursor(m_outCursor);
- else
- useCursor(m_inCursor);
+ updateCursor(event->modifiers() & Qt::ControlModifier);
- if (m_currentStrategy)
+ if (m_currentStrategy) {
m_currentStrategy->handleMouseMove(event->point, event->modifiers());
+ }
}
void KoZoomTool::keyPressEvent(QKeyEvent *event)
{
event->ignore();
- if (event->modifiers() & Qt::ControlModifier)
- useCursor(m_outCursor);
- else
- useCursor(m_inCursor);
+ updateCursor(event->modifiers() & Qt::ControlModifier);
}
void KoZoomTool::keyReleaseEvent(QKeyEvent *event)
{
event->ignore();
- if (event->modifiers() & Qt::ControlModifier)
- useCursor(m_outCursor);
- else
- useCursor(m_inCursor);
+ updateCursor(event->modifiers() & Qt::ControlModifier);
+
KoInteractionTool::keyReleaseEvent(event);
}
void KoZoomTool::activate(bool temporary)
{
m_temporary = temporary;
- useCursor(m_inCursor, true);
+ updateCursor(false);
}
void KoZoomTool::mouseDoubleClickEvent(KoPointerEvent *event)
@@ -99,14 +93,44 @@
KoInteractionStrategy *KoZoomTool::createStrategy(KoPointerEvent *event)
{
KoZoomStrategy *zs = new KoZoomStrategy(this, m_controller, event->point);
- if (event->button() == Qt::RightButton)
- zs->forceZoomOut();
+ if (event->button() == Qt::RightButton) {
+ if (m_zoomInMode) {
+ zs->forceZoomOut();
+ } else {
+ zs->forceZoomIn();
+ }
+ } else {
+ if (m_zoomInMode) {
+ zs->forceZoomIn();
+ } else {
+ zs->forceZoomOut();
+ }
+ }
return zs;
}
QWidget* KoZoomTool::createOptionWidget()
{
- //return new KoZoomToolWidget(this);
- return 0;
+ return new KoZoomToolWidget(this);
+ //return 0;
}
+void KoZoomTool::setZoomInMode(bool zoomIn)
+{
+ m_zoomInMode = zoomIn;
+ updateCursor(false);
+}
+
+void KoZoomTool::updateCursor(bool swap)
+{
+ bool setZoomInCursor = m_zoomInMode;
+ if (swap) {
+ setZoomInCursor = !setZoomInCursor;
+ }
+
+ if (setZoomInCursor) {
+ useCursor(m_inCursor);
+ } else {
+ useCursor(m_outCursor);
+ }
+}
Index: libs/flake/tools/KoZoomStrategy.cpp
===================================================================
--- libs/flake/tools/KoZoomStrategy.cpp (revision 1019773)
+++ libs/flake/tools/KoZoomStrategy.cpp (working copy)
@@ -36,12 +36,18 @@
{
QRect pixelRect = m_controller->canvas()->viewConverter()->documentToView(selectRect()).toRect();
pixelRect.translate(m_controller->canvas()->documentOrigin());
- if (m_forceZoomOut || modifiers & Qt::ControlModifier)
+
+ bool m_zoomOut = m_forceZoomOut;
+ if (modifiers & Qt::ControlModifier) {
+ m_zoomOut = !m_zoomOut;
+ }
+ if (m_zoomOut) {
m_controller->zoomOut(pixelRect.center());
- else if (pixelRect.width() > 5 && pixelRect.height() > 5)
+ } else if (pixelRect.width() > 5 && pixelRect.height() > 5) {
m_controller->zoomTo(pixelRect);
- else
+ } else {
m_controller->zoomIn(pixelRect.center());
+ }
}
void KoZoomStrategy::cancelInteraction()
@@ -54,3 +60,8 @@
{
m_forceZoomOut = true;
}
+
+void KoZoomStrategy::forceZoomIn()
+{
+ m_forceZoomOut = false;
+}
Index: libs/flake/tools/KoZoomToolWidget.cpp
===================================================================
--- libs/flake/tools/KoZoomToolWidget.cpp (revision 1019773)
+++ libs/flake/tools/KoZoomToolWidget.cpp (working copy)
@@ -21,6 +21,7 @@
#include <QPainter>
#include <QMouseEvent>
#include <kicon.h>
+#include "KoZoomTool.h"
KoZoomToolWidget::KoZoomToolWidget(KoZoomTool* tool, QWidget* parent)
: QWidget(parent), m_tool(tool)
@@ -28,9 +29,15 @@
setupUi(this);
m_dirtyThumbnail = true;
birdEyeLabel->installEventFilter(this);
+ birdEyeLabel->hide(); //remove this when coding on the birdEyeLabel
zoomInButton->setIcon(KIcon("zoom-in"));
zoomOutButton->setIcon(KIcon("zoom-out"));
+
+ connect(zoomInButton, SIGNAL(toggled(bool)), this, SLOT(changeZoomMode()));
+ connect(zoomOutButton, SIGNAL(toggled(bool)), this, SLOT(changeZoomMode()));
+
+ zoomInButton->click();
}
KoZoomToolWidget::~KoZoomToolWidget()
@@ -78,6 +85,7 @@
void KoZoomToolWidget::changeZoomMode()
{
+ m_tool->setZoomInMode(zoomInButton->isChecked());
}
#include <KoZoomToolWidget.moc>
Index: libs/flake/tools/KoZoomTool.h
===================================================================
--- libs/flake/tools/KoZoomTool.h (revision 1019773)
+++ libs/flake/tools/KoZoomTool.h (working copy)
@@ -57,16 +57,21 @@
m_controller = controller;
}
+ void setZoomInMode(bool zoomIn);
+
protected:
QWidget* createOptionWidget();
private:
virtual KoInteractionStrategy *createStrategy(KoPointerEvent *event);
+ void updateCursor(bool swap);
+
KoCanvasController *m_controller;
QCursor m_inCursor;
QCursor m_outCursor;
bool m_temporary;
+ bool m_zoomInMode;
};
#endif
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic