Changeset View
Standalone View
src/QuickEditor/QuickEditor.h
Show All 14 Lines | |||||
15 | * along with this program; if not, write to the Free Software | 15 | * along with this program; if not, write to the Free Software | ||
16 | * Foundation, Inc., 51 Franklin Street, Fifth Floor, | 16 | * Foundation, Inc., 51 Franklin Street, Fifth Floor, | ||
17 | * Boston, MA 02110-1301, USA. | 17 | * Boston, MA 02110-1301, USA. | ||
18 | */ | 18 | */ | ||
19 | 19 | | |||
20 | #ifndef QUICKEDITOR_H | 20 | #ifndef QUICKEDITOR_H | ||
21 | #define QUICKEDITOR_H | 21 | #define QUICKEDITOR_H | ||
22 | 22 | | |||
23 | #include <QObject> | 23 | #include <QWidget> | ||
24 | #include <QPainter> | ||||
25 | #include <QMouseEvent> | ||||
broulik: Forward-declare, you only use them as pointers/references, no need to include:
```
class… | |||||
26 | #include <QKeyEvent> | ||||
27 | #include <QStaticText> | ||||
28 | #include <QPair> | ||||
24 | 29 | | |||
25 | class QuickEditor : public QObject | 30 | class QuickEditor : public QWidget | ||
26 | { | 31 | { | ||
27 | Q_OBJECT | 32 | Q_OBJECT | ||
28 | 33 | | |||
29 | public: | 34 | public: | ||
30 | 35 | explicit QuickEditor(const QPixmap &pixmap, QObject* parent = nullptr); | |||
Previous was correct: QObject *parent, also doesn't this need to be a QWidget *? broulik: Previous was correct: `QObject *parent`, also doesn't this need to be a `QWidget *`? | |||||
Well, I didn't intend to make this stupid change haha. I wrote this file from scratch and the lines happened to line up in the diff. And I think QObject * is correct since QuickEditor is created by X11ImageGrabber, which is a QObject abalaji: Well, I didn't intend to make this stupid change haha. I wrote this file from scratch and the… | |||||
31 | explicit QuickEditor(const QPixmap &pixmap, QObject *parent = 0); | | |||
32 | virtual ~QuickEditor(); | | |||
33 | | ||||
34 | signals: | | |||
35 | | ||||
36 | void grabDone(const QPixmap &pixmap, const QRect &cropRegion); | | |||
37 | void grabCancelled(); | | |||
38 | | ||||
39 | private slots: | | |||
40 | | ||||
41 | void acceptImageHandler(int x, int y, int width, int height); | | |||
42 | 36 | | |||
43 | private: | 37 | private: | ||
38 | enum MouseState : short { | ||||
39 | None = 0, // 0000 | ||||
40 | Inside = 1, // 0001 | ||||
broulik: Can you use bit shifting/manipulation here maybe? e.g. `1<<0`, `1<<1` | |||||
abalaji: Sure yeah, makes sense | |||||
41 | Outside = 2, // 0010 | ||||
42 | TopLeft = 5, //101 | ||||
43 | Top = 17, // 10001 | ||||
44 | TopRight = 9, // 1001 | ||||
45 | Right = 33, // 100001 | ||||
46 | BottomRight = 6, // 110 | ||||
47 | Bottom = 18, // 10010 | ||||
48 | BottomLeft = 10, // 1010 | ||||
49 | Left = 34, // 100010 | ||||
50 | TopLeftOrBottomRight = 4, // 100 | ||||
broulik: Can't you or them together? `TopLeft | BottomRight`? | |||||
51 | TopRightOrBottomLeft = 8, // 1000 | ||||
52 | TopOrBottom = 16, // 10000 | ||||
53 | RightOrLeft = 32, // 100000 | ||||
54 | }; | ||||
44 | 55 | | |||
45 | struct ImageStore; | 56 | inline void acceptSelection(); | ||
46 | ImageStore *mImageStore; | 57 | void keyPressEvent(QKeyEvent* event) override; | ||
58 | void mousePressEvent(QMouseEvent* event) override; | ||||
59 | void mouseMoveEvent(QMouseEvent* event) override; | ||||
60 | void mouseReleaseEvent(QMouseEvent* event) override; | ||||
61 | void mouseDoubleClickEvent(QMouseEvent* event) override; | ||||
62 | void paintEvent(QPaintEvent*) override; | ||||
63 | inline void drawBottomHelpText(QPainter& painter); | ||||
64 | inline void drawDragHandles(QPainter& painter); | ||||
65 | inline void drawMidHelpText(QPainter& painter); | ||||
66 | inline void drawSelectionSizeTooltip(QPainter& painter); | ||||
67 | inline void layoutBottomHelpText(); | ||||
68 | inline void setMouseCursor(const QPoint& pos); | ||||
69 | MouseState whereIsTheMouse(const QPoint& pos); | ||||
broulik: `mousePosition` or `mouseLocation`? | |||||
abalaji: :+1: | |||||
70 | | ||||
71 | static const int mouseAreaSize; | ||||
72 | static const qreal cornerHandleRadius; | ||||
73 | static const qreal midHandleRadius; | ||||
74 | static const int selectionSizeThreshold; | ||||
75 | | ||||
76 | static const int selectionBoxPaddingX; | ||||
77 | static const int selectionBoxPaddingY; | ||||
78 | static const int selectionBoxMarginY; | ||||
Some constants are initialized in .cpp-file, some here in header, why so inconsistent? alexeymin: Some constants are initialized in .cpp-file, some here in header, why so inconsistent?
And… | |||||
So, I'm trying to keep all constants in the .cpp file since they are implementation details. But, since bottomHelpText is an array, and the length of an array is necessary (variable-length arrays not allowed in C++), I'm forced to leave this constant in the header file. constexpr is not really needed here tho, so I can make it just const. abalaji: So, I'm trying to keep all constants in the .cpp file since they are implementation details. | |||||
79 | | ||||
80 | static const int bottomHelpBoxPaddingX; | ||||
81 | static const int bottomHelpBoxPaddingY; | ||||
82 | static const int bottomHelpBoxPairSpacing; | ||||
83 | static const int bottomHelpBoxLineHeight; | ||||
84 | | ||||
85 | QColor mMaskColour; | ||||
86 | QColor mStrokeColor; | ||||
87 | QRect mSelection; | ||||
88 | QPoint mStartPos; | ||||
89 | QPoint mInitialTopLeft; | ||||
90 | QStaticText mMidHelpText; | ||||
91 | QFont mMidHelpTextFont; | ||||
92 | static constexpr int bottomHelpLength = 4; | ||||
93 | QPair<QStaticText, QStaticText> mBottomHelpText[bottomHelpLength]; | ||||
94 | QFont mBottomHelpTextFont; | ||||
95 | QRect mBottomHelpBorderBox; | ||||
96 | QPoint mBottomHelpContentPos; | ||||
97 | int mBottomHelpGridLeftWidth; | ||||
98 | MouseState mMouseDragState; | ||||
99 | QPixmap mPixmap; | ||||
broulik: Color without a u; I know that's been wrong in the original code, too. | |||||
Okay, fair enough in computer land I guess, although colour > color imo haha abalaji: Okay, fair enough in computer land I guess, although colour > color imo haha | |||||
47 | 100 | | |||
48 | struct QuickEditorPrivate; | 101 | signals: | ||
49 | Q_DECLARE_PRIVATE(QuickEditor); | 102 | void grabDone(const QPixmap &pixmap); | ||
50 | QuickEditorPrivate *d_ptr; | 103 | void grabCancelled(); | ||
broulik: Doesn't compile, probably needs to be `Q_SIGNALS`? | |||||
51 | }; | 104 | }; | ||
52 | 105 | | |||
53 | #endif // QUICKEDITOR_H | 106 | #endif // QUICKEDITOR_H |
Forward-declare, you only use them as pointers/references, no need to include: