View Issue Details

IDProjectCategoryView StatusLast Update
0015448ScribusGraphics / Image Framespublic2020-06-19 08:14
Reporterale Assigned Toale  
PrioritynormalSeverityfeatureReproducibilityN/A
Status assignedResolutionopen 
Product Version1.5.5.svn 
Target VersionFixed in Version 
Summary0015448: [PATCH] when an image is set to fit the frame, it should just fit it
Descriptionwhen 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.
Tagspatch
PatchYes

Relationships

related to 0015560 assignedale [PATCH (needs review)] fill a frame with the image 
related to 0015713 closedJLuc Provide new "fixed viewport" image resize mode 
related to 0016137 new Add option to always fill image frame 

Activities

Robby A

2018-09-30 10:48

reporter   ~0045490

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.

ale

2018-10-01 08:06

manager   ~0045493

Last edited: 2018-10-01 09:09

View 2 revisions

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.



image-fit.gif (240,315 bytes)
image-fit.gif (240,315 bytes)

PeterBenedek

2018-10-01 12:33

developer   ~0045494

Yes, it's really problem.

ale

2018-10-04 10:09

manager   ~0045496

here you see the issue in action: https://youtu.be/nB4dTL1IwI4?t=4m42s

JLuc

2019-01-25 15:39

developer   ~0045859

I agree : Automatic resizing of image to fit frame (or width or height btw) should reset x & y

ale

2019-01-25 15:57

manager   ~0045860

Last edited: 2019-01-26 09:49

View 2 revisions

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



image-vom-inside-frame-mouse.webm (235,648 bytes)
image-vom-inside-frame-pp.webm (175,229 bytes)

ale

2019-01-27 17:55

manager   ~0045863

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))
 	{
move-image-in-frame.diff (10,472 bytes)

Issue History

Date Modified Username Field Change
2018-09-30 08:16 ale New Issue
2018-09-30 08:17 ale Description Updated View Revisions
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 View Revisions
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 View Revisions
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