View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0015448 | Scribus | Graphics / Image Frames | public | 2018-09-30 08:16 | 2020-06-19 08:14 |
| Reporter | ale | Assigned To | ale | ||
| Priority | normal | Severity | feature | Reproducibility | N/A |
| Status | assigned | Resolution | open | ||
| Product Version | 1.5.5.svn | ||||
| Summary | 0015448: [PATCH] when an image is set to fit the frame, it should just fit it | ||||
| Description | when i set an image to fit the frame i want to be sure that all parts of the images are indeed in the frame. no ("inner") rotation should be allowed and the x,y inner coordinates should be set in a way that a white stripe only can occur in the "direction" that does not fit the frame (if i have an horizontal image in a vertical frame, i should be able to move the image vertically but not horizontally. that is: x will always be 0 but y can vary, as long as the full image fits vertically). if somebody wants to further move the image inside a frame, after having fit it to the frame, she should set it to "free scale". the small further step for the ones is a guarantee for all others, that an image that is set to fit, really fits: with no bad surprise when you get your image printed in thousands of copies. please comment if you do not agree (or even if you agree : - ), since i plan to invest some time to make a patch for this. | ||||
| Tags | patch | ||||
| Attached Files | |||||
| Patch | Yes | ||||
|
|
Thanks Ale, I confess that i've never noticed any problem with this, though it can be an inconvenience to have double-click on an image to allow movement within the frame (when the blue dot appears and which can happen inadvertently) and not to be able to double-click again to re-fix it. Ctrl-z would then undo the accidental movemenr in the frame. If I understand your plan, it seems reasonable. |
|
|
the problem is not about being able to undo an accidental movement. the problem is that, when i say that an image fits to the frame, i don't want to discover that it did not fit by 1 mm. if you have a border around the image you might see it very well on print, but not always on the screen. see the attached screencast where i try to align a fitted image to the bottom border: - does it really fit the bottom border? - does it fit the left and right borders? you don't know without exactly checking. that's the issue. |
|
|
Yes, it's really problem. |
|
|
here you see the issue in action: https://youtu.be/nB4dTL1IwI4?t=4m42s |
|
|
I agree : Automatic resizing of image to fit frame (or width or height btw) should reset x & y |
|
|
first bits of code: the image can't move in the "wrong" direction in the PP and in the "in frame" editing. https://my.mixtape.moe/lqjqzs.webm |
|
|
patch! i'm not 100% sure of the undo code in pageitem.cpp https://github.com/aoloe/scribus/blob/image-move-inner/scribus/pageitem.cpp#L4247 it tries to also undo the reset of the rotation. move-image-in-frame.diff (10,472 bytes)
diff --git a/scribus/canvasmode_edit.cpp b/scribus/canvasmode_edit.cpp
index bde12c678..ce6b91d25 100644
--- a/scribus/canvasmode_edit.cpp
+++ b/scribus/canvasmode_edit.cpp
@@ -417,13 +417,17 @@ void CanvasMode_Edit::mouseMoveEvent(QMouseEvent *m)
{
if (m->modifiers() & Qt::ShiftModifier)
{
- m_view->setCursor(IconManager::instance()->loadCursor("Rotieren2.png"));
- QTransform p = currItem->getTransform();
- p.translate(currItem->imageXOffset()*currItem->imageXScale(), currItem->imageYOffset()*currItem->imageYScale());
- QPointF rotP = p.map(QPointF(0.0, 0.0));
- double itemRotation = xy2Deg(mousePointDoc.x() - rotP.x(), mousePointDoc.y() - rotP.y());
- currItem->setImageRotation(itemRotation);
- m_canvas->displayRotHUD(m->globalPos(), itemRotation);
+ // only rotate if free scale
+ if (currItem->ScaleType)
+ {
+ m_view->setCursor(IconManager::instance()->loadCursor("Rotieren2.png"));
+ QTransform p = currItem->getTransform();
+ p.translate(currItem->imageXOffset()*currItem->imageXScale(), currItem->imageYOffset()*currItem->imageYScale());
+ QPointF rotP = p.map(QPointF(0.0, 0.0));
+ double itemRotation = xy2Deg(mousePointDoc.x() - rotP.x(), mousePointDoc.y() - rotP.y());
+ currItem->setImageRotation(itemRotation);
+ m_canvas->displayRotHUD(m->globalPos(), itemRotation);
+ }
}
else
{
diff --git a/scribus/pageitem.cpp b/scribus/pageitem.cpp
index 74aa708dc..e900f713d 100644
--- a/scribus/pageitem.cpp
+++ b/scribus/pageitem.cpp
@@ -4248,12 +4248,19 @@ void PageItem::setImageScalingMode(bool freeScale, bool keepRatio)
ss->set("OLD_IMAGEYOFFSET", m_imageYOffset);
ss->set("OLD_IMAGEXSCALE", m_imageXScale);
ss->set("OLD_IMAGEYSCALE", m_imageYScale);
+ ss->set("OLD_IMAGEROTATION", m_imageRotation);
}
}
if (keepRatio != AspectRatio)
ss->set("ASPECT_RATIO", keepRatio);
undoManager->action(this, ss);
}
+ if (!freeScale)
+ {
+ m_imageXOffset = 0.0;
+ m_imageYOffset = 0.0;
+ m_imageRotation = 0;
+ }
ScaleType = freeScale;
AspectRatio = keepRatio;
adjustPictScale();
@@ -7110,10 +7117,12 @@ void PageItem::restoreImageScaleMode(SimpleState *state, bool isUndo)
double oscy = state->getDouble("OLD_IMAGEYSCALE");
double ox = state->getDouble("OLD_IMAGEXOFFSET");
double oy = state->getDouble("OLD_IMAGEYOFFSET");
+ double orot = state->getInt("OLD_IMAGEROTATION");
Selection tempSelection(this, false);
tempSelection.addItem(this, true);
m_Doc->itemSelection_SetImageScale(oscx, oscy, &tempSelection);
m_Doc->itemSelection_SetImageOffset(ox, oy, &tempSelection);
+ m_Doc->itemSelection_SetImageRotation(orot, &tempSelection);
}
else
{
@@ -7121,6 +7130,7 @@ void PageItem::restoreImageScaleMode(SimpleState *state, bool isUndo)
state->set("OLD_IMAGEYOFFSET", m_imageYOffset);
state->set("OLD_IMAGEXSCALE", m_imageXScale);
state->set("OLD_IMAGEYSCALE", m_imageYScale);
+ state->set("OLD_IMAGEROTATION", m_imageRotation);
}
}
@@ -9770,30 +9780,6 @@ void PageItem::adjustPictScale()
m_imageXScale = xs;
m_imageYScale = ys;
}
- // Disable broken code. Code must be independent from doc in that function
- /*switch (m_Doc->RotMode)
- {
- case 0:
- LocalX = 0;
- LocalY = 0;
- break;
- case 1:
- LocalX = (Width - static_cast<double>(OrigW) * LocalScX) / LocalScX;
- LocalY = 0;
- break;
- case 2:
- LocalX = ((Width - static_cast<double>(OrigW) * LocalScX) / LocalScX) / 2.0;
- LocalY = ((Height - static_cast<double>(OrigH) * LocalScY) / LocalScY) / 2.0;
- break;
- case 3:
- LocalX = 0;
- LocalY = (Height - static_cast<double>(OrigH) * LocalScY) / LocalScY;
- break;
- case 4:
- LocalX = (Width - static_cast<double>(OrigW) * LocalScX) / LocalScX;
- LocalY = (Height - static_cast<double>(OrigH) * LocalScY) / LocalScY;
- break;
- }*/
if (m_Doc && m_Doc->isLoading())
{
m_imageXOffset = imgXOffs;
@@ -10154,23 +10140,39 @@ void PageItem::updateConstants()
m_Doc->constants().insert("height", m_height);
}
-//CB Old ScribusView MoveItemI
-void PageItem::moveImageInFrame(double newX, double newY)
+/**
+ * Apply the mouse movements for moving an image in the frame.
+ * If the image fits the frame, constraint the movements
+ * and make sure that the image does not overflow the frame.
+ */
+void PageItem::moveImageInFrame(double dX, double dY)
{
if (m_ItemType!=PageItem::ImageFrame)
return;
- if (locked())// || (!ScaleType))
+ if (locked())
return;
- double dX=0.0, dY=0.0;
+
if (imageFlippedH())
- dX=-newX;
- else
- dX=newX;
+ dX = -dX;
if (imageFlippedV())
- dY=-newY;
- else
- dY=newY;
+ dY = -dY;
+
+ if (!ScaleType)
+ {
+ if (!AspectRatio)
+ return;
+ if (isImageFittingHorizontal())
+ dX = 0;
+ if (isImageFittingVertical())
+ dY = 0;
+ dX = qMax(dX, -m_imageXOffset);
+ dY = qMax(dY, -m_imageYOffset);
+ dX = qMin(dX, m_width / m_imageXScale - OrigW - m_imageXOffset);
+ dY = qMin(dY, m_height / m_imageYScale - OrigH - m_imageYOffset);
+ }
+
moveImageXYOffsetBy(dX, dY);
+
if (!imageClip.empty())
{
imageClip = pixm.imgInfo.PDSpathData[pixm.imgInfo.usedPath].copy();
@@ -10182,6 +10184,23 @@ void PageItem::moveImageInFrame(double newX, double newY)
}
}
+bool PageItem::isImageFittingHorizontal()
+{
+ // a <= b -> a - b <= 0.1
+ return (m_width / m_height -
+ (double) pixm.width() / (double) pixm.height()
+ <= 0.1);
+
+}
+
+bool PageItem::isImageFittingVertical()
+{
+ // a >= b -> a - b >= -0.1
+ return (m_width / m_height -
+ (double) pixm.width() / (double) pixm.height()
+ >= -0.1);
+}
+
void PageItem::convertClip()
{
if (Clip.count() != 0)
diff --git a/scribus/pageitem.h b/scribus/pageitem.h
index fb2212b83..9683501ec 100644
--- a/scribus/pageitem.h
+++ b/scribus/pageitem.h
@@ -369,6 +369,9 @@ public: // Start public functions
* @todo Move to PageItem_ImageFrame
*/
void moveImageInFrame(double newX, double newY);
+ bool isImageFittingHorizontal();
+ bool isImageFittingVertical();
+
ObjAttrVector* getObjectAttributes();
/*!
* brief Returns a list of attributes with specified name
@@ -1320,8 +1323,8 @@ public: // Start public variables
bool OverrideCompressionQuality;
int CompressionQualityIndex;
bool imageIsAvailable; ///< Flag to hold image file availability
- int OrigW;
- int OrigH;
+ int OrigW; // width of the image in pixels
+ int OrigH; // height of the image in pixels
double BBoxX; ///< Bounding Box-X
double BBoxH; ///< Bounding Box-H
double CurX; ///< Zeichen X-Position
@@ -1849,8 +1852,8 @@ protected: // Start protected variables
bool m_isSelected; ///< Is the item selected?
double m_imageXScale; ///< Scaling X Factor for images
double m_imageYScale; ///< Scaling Y Factor for images
- double m_imageXOffset; ///< Image X Offset to frame
- double m_imageYOffset; ///< Image Y Offset to frame
+ double m_imageXOffset; ///< Horizontal offset in the frame in "unscaled" pixels
+ double m_imageYOffset; ///< Vertical offset in the frame in "unscaled" pixels
double m_imageRotation; ///< Image rotation in frame
FirstLineOffsetPolicy m_firstLineOffset;
bool m_groupClips;
diff --git a/scribus/ui/propertiespalette_image.cpp b/scribus/ui/propertiespalette_image.cpp
index d319144cd..a2b7580df 100644
--- a/scribus/ui/propertiespalette_image.cpp
+++ b/scribus/ui/propertiespalette_image.cpp
@@ -506,15 +506,37 @@ void PropertiesPalette_Image::setCurrentItem(PageItem *item)
keepImageWHRatioButton->setChecked(m_item->AspectRatio);
keepImageDPIRatioButton->setChecked(m_item->AspectRatio);
}
-//CB Why do we need this? Setting it too much here
-// if (setter == true)
-// {
-// keepImageWHRatioButton->setChecked(setter);
-// keepImageDPIRatioButton->setChecked(setter);
-// }
- //imageXOffsetSpinBox->setEnabled(setter);
- //imageYOffsetSpinBox->setEnabled(setter);
- //imageRotation->setEnabled(setter);
+
+ bool xEnabled{true};
+ bool yEnabled{true};
+ bool rotationEnabled{true};
+ double rangeMin{-16777215};
+ double rangeMaxX{16777215 * m_unitRatio};
+ double rangeMaxY{16777215 * m_unitRatio};
+ if (!setter)
+ {
+ if (m_item->AspectRatio)
+ {
+ xEnabled = !m_item->isImageFittingHorizontal();
+ yEnabled = !m_item->isImageFittingVertical();
+ }
+ else
+ {
+ xEnabled = false;
+ yEnabled = false;
+ }
+ rotationEnabled = false;
+ rangeMin = 0.0;
+ rangeMaxX = (m_item->width() - m_item->OrigW * m_item->imageXScale()) * m_unitRatio;
+ rangeMaxY = (m_item->height() - m_item->OrigH * m_item->imageYScale()) * m_unitRatio;
+ }
+ imageXOffsetSpinBox->setEnabled(xEnabled);
+ if (xEnabled)
+ imageXOffsetSpinBox->setValues(rangeMin, rangeMaxX, unitGetPrecisionFromIndex(m_unitIndex), 0);
+ imageYOffsetSpinBox->setEnabled(yEnabled);
+ if (yEnabled)
+ imageYOffsetSpinBox->setValues(rangeMin, rangeMaxY, unitGetPrecisionFromIndex(m_unitIndex), 0);
+ imageRotation->setEnabled(rotationEnabled);
imageXScaleSpinBox->blockSignals(false);
imageYScaleSpinBox->blockSignals(false);
@@ -596,36 +618,10 @@ void PropertiesPalette_Image::handleScaling()
if (!m_ScMW || m_ScMW->scriptIsRunning())
return;
- if (freeScale == sender())
- {
- frameScale->setChecked(false);
- freeScale->setChecked(true);
- cbProportional->setEnabled(false);
-// imageXOffsetSpinBox->setEnabled(true);
-// imageYOffsetSpinBox->setEnabled(true);
- imageXScaleSpinBox->setEnabled(true);
- imageYScaleSpinBox->setEnabled(true);
- imgDpiX->setEnabled(true);
- imgDpiY->setEnabled(true);
-// imageRotation->setEnabled(true);
- keepImageWHRatioButton->setEnabled(true);
- keepImageDPIRatioButton->setEnabled(true);
- }
- if (frameScale == sender())
- {
- frameScale->setChecked(true);
- freeScale->setChecked(false);
- cbProportional->setEnabled(true);
-// imageXOffsetSpinBox->setEnabled(false);
-// imageYOffsetSpinBox->setEnabled(false);
- imageXScaleSpinBox->setEnabled(false);
- imageYScaleSpinBox->setEnabled(false);
- imgDpiX->setEnabled(false);
- imgDpiY->setEnabled(false);
-// imageRotation->setEnabled(false);
- keepImageWHRatioButton->setEnabled(false);
- keepImageDPIRatioButton->setEnabled(false);
- }
+ // here there was code setting which widgets are enabled
+ // when the scaling type changes. but, finally,
+ // handleSelectionChanged() and setCurItem() get called
+ // and overwrite what has been done here.
if ((m_haveDoc) && (m_haveItem))
{
|
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-09-30 08:16 | ale | New Issue | |
| 2018-09-30 08:17 | ale | Description Updated | |
| 2018-09-30 10:48 | Robby A | Note Added: 0045490 | |
| 2018-10-01 08:06 | ale | File Added: image-fit.gif | |
| 2018-10-01 08:06 | ale | Note Added: 0045493 | |
| 2018-10-01 09:09 | ale | Note Edited: 0045493 | |
| 2018-10-01 12:33 | PeterBenedek | Note Added: 0045494 | |
| 2018-10-02 05:41 | ale | Assigned To | => ale |
| 2018-10-02 05:41 | ale | Status | new => assigned |
| 2018-10-04 10:09 | ale | Note Added: 0045496 | |
| 2019-01-25 15:39 | JLuc | Note Added: 0045859 | |
| 2019-01-25 15:57 | ale | File Added: image-vom-inside-frame-mouse.webm | |
| 2019-01-25 15:57 | ale | File Added: image-vom-inside-frame-pp.webm | |
| 2019-01-25 15:57 | ale | Note Added: 0045860 | |
| 2019-01-26 09:49 | ale | Note Edited: 0045860 | |
| 2019-01-27 17:55 | ale | File Added: move-image-in-frame.diff | |
| 2019-01-27 17:55 | ale | Note Added: 0045863 | |
| 2019-01-27 17:56 | ale | Patch | No => Yes |
| 2019-01-27 17:57 | ale | Tag Attached: patch | |
| 2019-01-27 17:57 | ale | Summary | when an image is set to fit the frame, it should just fit it => [PATCH] when an image is set to fit the frame, it should just fit it |
| 2019-03-06 16:30 | ale | Relationship added | related to 0015560 |
| 2019-06-14 17:56 | JLuc | Relationship added | related to 0015713 |
| 2020-06-19 08:14 | JLuc | Relationship added | related to 0016137 |