Changeset View
Standalone View
lib/crop/croptool.cpp
Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Line(s) | |||||
83 | struct CropToolPrivate | 83 | struct CropToolPrivate | ||
84 | { | 84 | { | ||
85 | CropTool* q; | 85 | CropTool* q; | ||
86 | QRect mRect; | 86 | QRect mRect; | ||
87 | QList<CropHandle> mCropHandleList; | 87 | QList<CropHandle> mCropHandleList; | ||
88 | CropHandle mMovingHandle; | 88 | CropHandle mMovingHandle; | ||
89 | QPoint mLastMouseMovePos; | 89 | QPoint mLastMouseMovePos; | ||
90 | double mCropRatio; | 90 | double mCropRatio; | ||
91 | double mLockedCropRatio; | ||||
rkflx: The variable name sounds like a lock, but actually it's a ratio. Perhaps `mLockedCropRatio`? | |||||
91 | CropWidget* mCropWidget; | 92 | CropWidget* mCropWidget; | ||
92 | 93 | | |||
93 | QRect viewportCropRect() const | 94 | QRect viewportCropRect() const | ||
94 | { | 95 | { | ||
95 | return q->imageView()->mapToView(mRect); | 96 | return q->imageView()->mapToView(mRect); | ||
96 | } | 97 | } | ||
97 | 98 | | |||
98 | QRect handleViewportRect(CropHandle handle) | 99 | QRect handleViewportRect(CropHandle handle) | ||
▲ Show 20 Lines • Show All 118 Lines • ▼ Show 20 Line(s) | |||||
217 | 218 | | |||
218 | CropTool::CropTool(RasterImageView* view) | 219 | CropTool::CropTool(RasterImageView* view) | ||
219 | : AbstractRasterImageViewTool(view) | 220 | : AbstractRasterImageViewTool(view) | ||
220 | , d(new CropToolPrivate) | 221 | , d(new CropToolPrivate) | ||
221 | { | 222 | { | ||
222 | d->q = this; | 223 | d->q = this; | ||
223 | d->mCropHandleList << CH_Left << CH_Right << CH_Top << CH_Bottom << CH_TopLeft << CH_TopRight << CH_BottomLeft << CH_BottomRight; | 224 | d->mCropHandleList << CH_Left << CH_Right << CH_Top << CH_Bottom << CH_TopLeft << CH_TopRight << CH_BottomLeft << CH_BottomRight; | ||
224 | d->mMovingHandle = CH_None; | 225 | d->mMovingHandle = CH_None; | ||
225 | d->mCropRatio = 0.; | 226 | d->mCropRatio = 0.; | ||
227 | d->mLockedCropRatio = 0.; | ||||
rkflx: Do you also need to initialize `mCropRatioLock` here? | |||||
Well, that isn't mCropRatio :) That said, we don't need this anymore now we are always setting the ratio from the config, so we can remove (I assume I should update the config save patch, not this one) huoni: Well, that isn't `mCropRatio` :)
That said, we don't need this anymore now we are always… | |||||
Oops, what I meant was, that isn't mCropRatioLock. (gotta proofread my comments more) huoni: Oops, what I meant was, that isn't `mCropRatioLock`.
(gotta proofread my comments more) | |||||
Sorry for being stubborn, but in mouseMoveEvent for me mLockedCropRatio is 1.57687e-316. Yes, it won't do much harm, because as soon as you press the modifier it will be initialized. Still, someone might change things around in the future, so your assumption might break. Always initialize member variables to a known value in the constructor, please. rkflx: Sorry for being stubborn, but in `mouseMoveEvent` for me `mLockedCropRatio` is `1.57687e-316`. | |||||
Sorry I completely misunderstood what you meant here, no need to apologise. huoni: Sorry I completely misunderstood what you meant here, no need to apologise. | |||||
226 | d->mRect = d->computeVisibleImageRect(); | 228 | d->mRect = d->computeVisibleImageRect(); | ||
227 | d->setupWidget(); | 229 | d->setupWidget(); | ||
228 | } | 230 | } | ||
229 | 231 | | |||
230 | CropTool::~CropTool() | 232 | CropTool::~CropTool() | ||
231 | { | 233 | { | ||
232 | // mCropWidget is a child of its container not of us, so it is not deleted automatically | 234 | // mCropWidget is a child of its container not of us, so it is not deleted automatically | ||
233 | delete d->mCropWidget; | 235 | delete d->mCropWidget; | ||
▲ Show 20 Lines • Show All 103 Lines • ▼ Show 20 Line(s) | 309 | { | |||
337 | if (d->mRect.height() < 0) { | 339 | if (d->mRect.height() < 0) { | ||
338 | d->mMovingHandle = d->mMovingHandle ^(CH_Top | CH_Bottom); | 340 | d->mMovingHandle = d->mMovingHandle ^(CH_Top | CH_Bottom); | ||
339 | } | 341 | } | ||
340 | if (d->mRect.width() < 0) { | 342 | if (d->mRect.width() < 0) { | ||
341 | d->mMovingHandle = d->mMovingHandle ^(CH_Left | CH_Right); | 343 | d->mMovingHandle = d->mMovingHandle ^(CH_Left | CH_Right); | ||
342 | } | 344 | } | ||
343 | d->mRect = d->mRect.normalized(); | 345 | d->mRect = d->mRect.normalized(); | ||
344 | 346 | | |||
345 | // Enforce ratio | 347 | // Enforce ratio: | ||
346 | if (d->mCropRatio > 0.) { | 348 | double ratioToEnforce = d->mCropRatio; | ||
349 | // - if user is holding down Ctrl/Shift when resizing rect, lock to current rect ratio | ||||
350 | if (event->modifiers() & (Qt::ControlModifier | Qt::ShiftModifier) && d->mLockedCropRatio > 0) { | ||||
351 | ratioToEnforce = d->mLockedCropRatio; | ||||
352 | } | ||||
353 | // - if user has restricted the ratio via the GUI | ||||
354 | if (ratioToEnforce > 0.) { | ||||
347 | if (d->mMovingHandle == CH_Top || d->mMovingHandle == CH_Bottom) { | 355 | if (d->mMovingHandle == CH_Top || d->mMovingHandle == CH_Bottom) { | ||
348 | // Top or bottom | 356 | // Top or bottom | ||
349 | int width = int(d->mRect.height() / d->mCropRatio); | 357 | int width = int(d->mRect.height() / ratioToEnforce); | ||
350 | d->mRect.setWidth(width); | 358 | d->mRect.setWidth(width); | ||
351 | } else if (d->mMovingHandle == CH_Left || d->mMovingHandle == CH_Right) { | 359 | } else if (d->mMovingHandle == CH_Left || d->mMovingHandle == CH_Right) { | ||
352 | // Left or right | 360 | // Left or right | ||
353 | int height = int(d->mRect.width() * d->mCropRatio); | 361 | int height = int(d->mRect.width() * ratioToEnforce); | ||
354 | d->mRect.setHeight(height); | 362 | d->mRect.setHeight(height); | ||
355 | } else if (d->mMovingHandle & CH_Top) { | 363 | } else if (d->mMovingHandle & CH_Top) { | ||
356 | // Top left or top right | 364 | // Top left or top right | ||
357 | int height = int(d->mRect.width() * d->mCropRatio); | 365 | int height = int(d->mRect.width() * ratioToEnforce); | ||
358 | d->mRect.setTop(d->mRect.bottom() - height); | 366 | d->mRect.setTop(d->mRect.bottom() - height); | ||
359 | } else if (d->mMovingHandle & CH_Bottom) { | 367 | } else if (d->mMovingHandle & CH_Bottom) { | ||
360 | // Bottom left or bottom right | 368 | // Bottom left or bottom right | ||
361 | int height = int(d->mRect.width() * d->mCropRatio); | 369 | int height = int(d->mRect.width() * ratioToEnforce); | ||
362 | d->mRect.setHeight(height); | 370 | d->mRect.setHeight(height); | ||
363 | } | 371 | } | ||
364 | } | 372 | } | ||
365 | 373 | | |||
366 | if (d->mMovingHandle == CH_Content) { | 374 | if (d->mMovingHandle == CH_Content) { | ||
367 | d->mRect.translate(point - d->mLastMouseMovePos); | 375 | d->mRect.translate(point - d->mLastMouseMovePos); | ||
368 | d->mLastMouseMovePos = point; | 376 | d->mLastMouseMovePos = point; | ||
369 | } | 377 | } | ||
Show All 19 Lines | 396 | { | |||
389 | event->accept(); | 397 | event->accept(); | ||
390 | // Make sure cursor is updated when moving over handles | 398 | // Make sure cursor is updated when moving over handles | ||
391 | CropHandle handle = d->handleAt(event->lastPos()); | 399 | CropHandle handle = d->handleAt(event->lastPos()); | ||
392 | d->updateCursor(handle, false /* buttonDown */); | 400 | d->updateCursor(handle, false /* buttonDown */); | ||
393 | } | 401 | } | ||
394 | 402 | | |||
395 | void CropTool::keyPressEvent(QKeyEvent* event) | 403 | void CropTool::keyPressEvent(QKeyEvent* event) | ||
396 | { | 404 | { | ||
405 | // Lock crop ratio to current rect when user presses Control or Shift | ||||
406 | if (event->key() == Qt::Key_Control || event->key() == Qt::Key_Shift) { | ||||
Adding qDebug(), this triggers not only for ⇧ or Ctrl, but for lots of other keys too, e.g. k. rkflx: Adding `qDebug()`, this triggers not only for {key Shift} or {key Ctrl}, but for lots of other… | |||||
407 | d->mLockedCropRatio = 1. * d->mRect.height() / d->mRect.width(); | ||||
408 | } | ||||
409 | | ||||
397 | QDialogButtonBox *buttons = d->mCropWidget->findChild<QDialogButtonBox *>(); | 410 | QDialogButtonBox *buttons = d->mCropWidget->findChild<QDialogButtonBox *>(); | ||
398 | switch (event->key()) { | 411 | switch (event->key()) { | ||
399 | case Qt::Key_Escape: | 412 | case Qt::Key_Escape: | ||
400 | event->accept(); | 413 | event->accept(); | ||
401 | buttons->rejected(); | 414 | buttons->rejected(); | ||
402 | break; | 415 | break; | ||
403 | case Qt::Key_Return: | 416 | case Qt::Key_Return: | ||
404 | case Qt::Key_Enter: | 417 | case Qt::Key_Enter: | ||
▲ Show 20 Lines • Show All 47 Lines • Show Last 20 Lines |
The variable name sounds like a lock, but actually it's a ratio. Perhaps mLockedCropRatio?