Differential D12626 Diff 33422 src/QuickEditor/QuickEditor.h

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 <QOpenGLWidget>
		24		#include <QPainter>
		25		#include <QMouseEvent>
			broulikUnsubmitted Done Forward-declare, you only use them as pointers/references, no need to include: class QMouseEvent; broulik: Forward-declare, you only use them as pointers/references, no need to include: ``` class…
		26		#include <QKeyEvent>
		27		#include <QStaticText>
24		28
25	class QuickEditor : public QObject	29		class QuickEditor : public QWidget
26	{	30		{
27	Q_OBJECT	31		Q_OBJECT
28		32
29	public:	33		public:
30		34		explicit QuickEditor(const QPixmap &pixmap, QObject* parent = nullptr);
			broulikUnsubmitted Done 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 `?
			abalajiAuthorUnsubmitted Done 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		35
43	private:	36		private:
		37		enum MouseState : short {
		38		None = 0, // 0000
		39		Inside = 1, // 0001
			broulikUnsubmitted Done Can you use bit shifting/manipulation here maybe? e.g. `1<<0`, `1<<1` broulik: Can you use bit shifting/manipulation here maybe? e.g. `1<<0`, `1<<1`
			abalajiAuthorUnsubmitted Done Sure yeah, makes sense abalaji: Sure yeah, makes sense
		40		Outside = 2, // 0010
		41		TopLeft = 5, //101
		42		Top = 17, // 10001
		43		TopRight = 9, // 1001
		44		Right = 33, // 100001
		45		BottomRight = 6, // 110
		46		Bottom = 18, // 10010
		47		BottomLeft = 10, // 1010
		48		Left = 34, // 100010
		49		TopLeftOrBottomRight = 4, // 100
			broulikUnsubmitted Done Can't you or them together? `TopLeft \| BottomRight`? broulik: Can't you or them together? `TopLeft \| BottomRight`?
		50		TopRightOrBottomLeft = 8, // 1000
		51		TopOrBottom = 16, // 10000
		52		RightOrLeft = 32, // 100000
		53		};
44		54
45	struct ImageStore;	55		inline void acceptSelection();
46	ImageStore *mImageStore;	56		void keyPressEvent(QKeyEvent* event) override;
		57		void mousePressEvent(QMouseEvent* event) override;
		58		void mouseMoveEvent(QMouseEvent* event) override;
		59		void mouseReleaseEvent(QMouseEvent* event) override;
		60		void mouseDoubleClickEvent(QMouseEvent* event) override;
		61		void paintEvent(QPaintEvent*) override;
		62		inline void setMouseCursor(const QPoint& pos);
		63		MouseState whereIsTheMouse(const QPoint& pos);
		64
		65		static const int mouseAreaSize;
		66		static const qreal cornerHandleRadius;
		67		static const qreal midHandleRadius;
		68		static const int selectionSizeThreshold;
			broulikUnsubmitted Done `mousePosition` or `mouseLocation`? broulik: `mousePosition` or `mouseLocation`?
			abalajiAuthorUnsubmitted Done 👍 abalaji: :+1:
			alexeyminUnsubmitted Done Some constants are initialized in .cpp-file, some here in header, why so inconsistent? And this line is also using constexpr, while others don't alexeymin: Some constants are initialized in .cpp-file, some here in header, why so inconsistent? And…
			abalajiAuthorUnsubmitted Done 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.
		69
		70		QColor mMaskColour;
		71		QColor mStrokeColor;
		72		QRect mSelection;
		73		QPoint mStartPos;
		74		QPoint mInitialTopLeft;
		75		QStaticText mMidHelpText;
		76		QFont mMidHelpTextFont;
		77		QStaticText mBottomHelpText;
		78		QFont mBottomHelpTextFont;
		79		MouseState mMouseDragState;
		80		QPixmap mPixmap;
47		81
48	struct QuickEditorPrivate;	82		signals:
			broulikUnsubmitted Done Color without a u; I know that's been wrong in the original code, too. broulik: Color without a u; I know that's been wrong in the original code, too.
			abalajiAuthorUnsubmitted Done 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
49	Q_DECLARE_PRIVATE(QuickEditor);	83		void grabDone(const QPixmap &pixmap);
50	QuickEditorPrivate *d_ptr;	84		void grabCancelled();
			broulikUnsubmitted Done Doesn't compile, probably needs to be `Q_SIGNALS`? broulik: Doesn't compile, probably needs to be `Q_SIGNALS`?
51	};	85		};
52		86
53	#endif // QUICKEDITOR_H	87		#endif // QUICKEDITOR_H