Merge pull request #1024 from arummler/fix-picture-insert

Fix picture and graph primitives issues

sources/diagramevent/diagrameventaddimage.cpp

@@ -117,7 +117,16 @@ void DiagramEventAddImage::mousePressEvent(QGraphicsSceneMouseEvent *event)
 	}
 	else if (m_image && !m_pressed && event->button() == Qt::RightButton)
 	{
-		m_image->setRotation(m_image->rotation() + 90);
+		// rotationAngle()/setRotationAngle(), not QGraphicsItem's own
+		// rotation()/setRotation(): DiagramImageItem's whole handle/undo/
+		// XML-save machinery reads exclusively from its own m_transform
+		// (see diagramimageitem.cpp's toXml() comment) and never looks at
+		// QGraphicsItem's built-in convenience property at all -- using
+		// it here left a rotation that displayed correctly in this tool
+		// but silently vanished the moment the item was saved and
+		// reloaded, and additionally desynced the rotate-handle's pivot
+		// math once the image was later selected for editing.
+		m_image->setRotationAngle(m_image->rotationAngle() + 90);
 		event->setAccepted(true);
 	}
 }
@@ -167,6 +176,22 @@ void DiagramEventAddImage::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
 		}
 		else
 		{
+			// setPivot(QPointF(0, 0)), not left at its default
+			// boundingRect().center(): DiagramImageItem's own m_transform
+			// is pivot-centered, so with the default center pivot the
+			// setPos(qMin(...)) below would no longer land on the image's
+			// actual top-left corner the moment scale != 1 (the corner
+			// only coincides with pos() when the pivot is the local
+			// origin). Anchoring here to (0, 0) -- exactly the same
+			// temporary-anchor trick DiagramImageItem::handlerMousePressEvent()
+			// already uses for its own Resize handles -- makes pos()
+			// keep meaning "scene position of the top-left corner"
+			// regardless of scale, so the qMin(...) line below still
+			// needs no change at all. setPivot() itself is a no-op past
+			// the first call (same pivot value), and compensates pos()
+			// automatically so nothing visibly jumps at the switch.
+			m_image->setPivot(QPointF(0, 0));
+
 			const QSizeF naturalSize = m_image->boundingRect().size();
 			if (naturalSize.width() > 0 && naturalSize.height() > 0)
 			{
@@ -179,7 +204,12 @@ void DiagramEventAddImage::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
 				// free-form one -- breaking aspect ratio on purpose is
 				// its own, separate, larger piece of work.
 				const qreal newScale = qBound(0.01, qMax(scaleX, scaleY), 50.0);
-				m_image->setScale(newScale);
+				// scaleFactorX()/scaleFactorY(), not QGraphicsItem's own
+				// scale(): see the mousePressEvent right-click rotate
+				// comment above for why -- identical reasoning, identical
+				// fix.
+				m_image->setScaleFactorX(newScale);
+				m_image->setScaleFactorY(newScale);
 			}
 			m_image->setPos(qMin(m_press_pos.x(), pos.x()), qMin(m_press_pos.y(), pos.y()));
 		}
@@ -206,6 +236,17 @@ void DiagramEventAddImage::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
 {
 	if (m_image && m_pressed && event->button() == Qt::LeftButton)
 	{
+		// Undo whatever temporary origin-anchoring mouseMoveEvent's
+		// resize-drag branch did (a no-op if it never engaged): every
+		// other DiagramImageItem code path -- handles, undo, XML save --
+		// expects an image's pivot to be its bounding-rect center unless
+		// the user deliberately customized it, exactly like a completed
+		// handle-resize already resets it via resetPivotToBoundingRectCenter().
+		// Doing this before reading pos() below is what makes the pushed
+		// command capture the final, center-pivot position rather than
+		// the drag's temporary corner-anchored one.
+		m_image->setPivot(m_image->boundingRect().center());
+
 		m_diagram->undoStack().push(new AddGraphicsObjectCommand(m_image, m_diagram, m_image->pos()));
 
 		for (QGraphicsView *view : m_diagram->views()) {
@@ -246,10 +287,32 @@ void DiagramEventAddImage::wheelEvent(QGraphicsSceneWheelEvent *event)
 		return;
 	}
 	
-	qreal scaling = m_image->scale();
-	event->delta() > 1? scaling += 0.01 : scaling -= 0.01;
-	if (scaling>0.01 && scaling <= 2) {
-		m_image->setScale(scaling);
+	// scaleFactorX()/scaleFactorY(), not QGraphicsItem's own scale(): see
+	// the right-click rotate comment in mousePressEvent for why. Wheel-
+	// scaling only ever runs while !m_pressed (guarded above), i.e. before
+	// any drag-resize has anchored the pivot to the origin (see
+	// mouseMoveEvent), so the pivot here is still the default
+	// boundingRect().center() and this scales the image in place around
+	// its own middle, exactly like before.
+	//
+	// Step each axis from its own current value rather than reading X and
+	// writing it back to both: scaleFactorX and scaleFactorY cannot
+	// actually differ at this point today (every other mutator in this
+	// class -- the drag-resize branch above, and this same wheelEvent --
+	// only ever sets them to the same value, and mouseReleaseEvent commits
+	// and ends this tool on any left-button release, so a handle-based
+	// non-uniform resize can never happen first and leave this instance
+	// still alive). Stepping both from their own value rather than
+	// collapsing Y to X costs nothing today and removes the trap if that
+	// invariant ever stops holding.
+	qreal scalingX = m_image->scaleFactorX();
+	qreal scalingY = m_image->scaleFactorY();
+	const qreal step = event->delta() > 1 ? 0.01 : -0.01;
+	scalingX += step;
+	scalingY += step;
+	if (scalingX > 0.01 && scalingX <= 2 && scalingY > 0.01 && scalingY <= 2) {
+		m_image->setScaleFactorX(scalingX);
+		m_image->setScaleFactorY(scalingY);
 	}
 	
 	event->setAccepted(true);

sources/diagramevent/diagrameventaddpaste.cpp

@@ -41,7 +41,6 @@
 	DiagramEventAddPaste::DiagramEventAddPaste(Diagram *diagram, const QPointF &start_pos) :
 	DiagramEventInterface(diagram)
 {
-	Q_UNUSED(start_pos);  // items stay at their original XML position
 		//DiagramEventInterface::init() is called by Diagram::setEventInterface
 		//only when it is replacing an earlier interface, so call it here as
 		//DiagramEventAddMacro does.
@@ -77,21 +76,26 @@
 	const QList<QGraphicsItem *> movable = m_content.items(MovableItems);
 	if (movable.isEmpty()) return;
 
-		//Compute the top-left of all items' positions (not bounding
-		//rects) and snap to grid: this is the point that gets placed
-		//under the cursor, and the baseline moveTo() measures from.
-	QPointF top_left;
-	bool first = true;
+		//Compute the top-left of all items' actual on-screen bounding
+		//boxes (not their raw pos()) and snap to grid: this is the point
+		//that gets placed under the cursor, and the baseline moveTo()
+		//measures from. mapToScene(boundingRect()) matters here, not
+		//pos() alone: pos() is the scene location of an item's local
+		//origin, but for anything with a pivot-centered transform (a
+		//scaled or rotated image, in particular) that origin can sit far
+		//from where the item is actually drawn -- pivot + scale*(0 -
+		//pivot) is nowhere near (0, 0) once scale is well under 1. Using
+		//pos() here silently pasted content at the right *delta* from a
+		//point that wasn't actually where the content visually was,
+		//producing a constant, scale-dependent offset between the cursor
+		//and the pasted picture. Diagram::fromXml()'s own position
+		//parameter already gets this right the same way, for the same
+		//reason.
+	QRectF items_rect;
 	for (auto *item : movable) {
-		const QPointF p = item->pos();
-		if (first) {
-			top_left = p;
-			first = false;
-		} else {
-			if (p.x() < top_left.x()) top_left.setX(p.x());
-			if (p.y() < top_left.y()) top_left.setY(p.y());
-		}
+		items_rect = items_rect.united(item->mapToScene(item->boundingRect()).boundingRect());
 	}
+	const QPointF top_left = items_rect.topLeft();
 	QSettings settings;
 	const int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"),
 					  Diagram::xGrid).toInt();
@@ -102,9 +106,24 @@
 			qRound(p.x() / xGrid) * xGrid,
 			qRound(p.y() / yGrid) * yGrid);
 	};
-	const QPointF grid_origin = snapGrid(top_left);
+	const QPointF grid_origin = snapGrid(start_pos);
 
-		//Store each item's original position.  moveTo() applies a
+		//Land the pasted content under the cursor immediately, rather than
+		//leaving it at the copied source's own coordinates: fromXml() above
+		//loads items at their original position purely because it doesn't
+		//know the target yet, not because that is where a paste should end
+		//up. The previous approach instead left items there and warped the
+		//OS cursor to match -- QCursor::setPos() is silently ignored by
+		//many window managers and compositors (Wayland in particular), so
+		//on any of those the warp simply never happened and the paste was
+		//left wherever it had originally been copied from, which could be
+		//anywhere on the folio -- exactly the "far from the cursor" bug.
+	const QPointF initial_delta = grid_origin - snapGrid(top_left);
+	for (auto *item : movable) {
+		item->setPos(item->pos() + initial_delta);
+	}
+
+		//Store each item's now-placed position.  moveTo() applies a
 		//grid-snapped delta from the baseline to these, so items
 		//preserve their layout and move in whole grid steps.
 	for (auto *item : movable) {
@@ -134,14 +153,6 @@
 			if (const auto qde = QETApp::diagramEditorAncestorOf(view)) {
 				m_status_bar = qde->statusBar();
 			}
-				//Warp the cursor to the group's grid-snapped origin so
-				//the actual cursor position matches m_initial_cursor.
-				//Without this the first mouseMoveEvent computes a large
-				//delta (cursor is still at the Ctrl+V press location)
-				//and the items jump on first touch.
-			const QPoint view_pos = view->mapFromScene(m_initial_cursor);
-			const QPoint global_pos = view->viewport()->mapToGlobal(view_pos);
-			QCursor::setPos(global_pos);
 		}
 	}
 	showHint();

sources/diagramevent/diagrameventaddpdf.cpp

@@ -84,7 +84,10 @@ void DiagramEventAddPdf::mousePressEvent(QGraphicsSceneMouseEvent *event)
 	}
 	else if (m_image && event->button() == Qt::RightButton)
 	{
-		m_image->setRotation(m_image->rotation() + 90);
+		// rotationAngle()/setRotationAngle(), not QGraphicsItem's own
+		// rotation()/setRotation(): see DiagramEventAddImage's identical
+		// fix (mousePressEvent) for why -- same class, same reasoning.
+		m_image->setRotationAngle(m_image->rotationAngle() + 90);
 		event->setAccepted(true);
 	}
 }
@@ -132,14 +135,35 @@ void DiagramEventAddPdf::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event)
 */
 void DiagramEventAddPdf::wheelEvent(QGraphicsSceneWheelEvent *event)
 {
-	if (!m_is_added || !m_image || event->modifiers() != Qt::CTRL) {
+	// event->modifiers() & Qt::ControlModifier, not != Qt::CTRL: the same
+	// exact-equality bug already found and fixed elsewhere this session --
+	// Ctrl held together with any other modifier would silently fail to
+	// register as Ctrl at all.
+	if (!m_is_added || !m_image || !(event->modifiers() & Qt::ControlModifier)) {
 		return;
 	}
 
-	qreal scaling = m_image->scale();
-	event->delta() > 1 ? scaling += 0.01 : scaling -= 0.01;
-	if (scaling > 0.01 && scaling <= 2) {
-		m_image->setScale(scaling);
+	// scaleFactorX()/scaleFactorY(), not QGraphicsItem's own scale(): see
+	// DiagramEventAddImage's identical fix for why. No drag-to-resize
+	// exists here, and the pivot is never touched elsewhere in this
+	// class, so it stays at its default boundingRect().center() and this
+	// scales the page in place around its own middle, exactly like
+	// before.
+	//
+	// Step each axis from its own current value rather than reading X and
+	// writing it back to both: scaleFactorX and scaleFactorY cannot
+	// actually differ here today (nothing in this class ever sets them to
+	// different values, and there is no drag-resize at all), but stepping
+	// both independently costs nothing and removes the trap if that ever
+	// changes.
+	qreal scalingX = m_image->scaleFactorX();
+	qreal scalingY = m_image->scaleFactorY();
+	const qreal step = event->delta() > 1 ? 0.01 : -0.01;
+	scalingX += step;
+	scalingY += step;
+	if (scalingX > 0.01 && scalingX <= 2 && scalingY > 0.01 && scalingY <= 2) {
+		m_image->setScaleFactorX(scalingX);
+		m_image->setScaleFactorY(scalingY);
 	}
 
 	event->setAccepted(true);

sources/diagramevent/diagrameventaddshape.cpp

@@ -87,9 +87,9 @@ DiagramEventAddShape::~DiagramEventAddShape()
 	Applies a drag/click position to the in-progress shape, honouring two
 	modifiers that mirror how the very same shape can already be edited
 	afterward, once placed:
-	  - Ctrl, for Rectangle/Ellipse only: the first click becomes the
-	    shape's *center* rather than a corner, growing symmetrically as
-	    the cursor moves away from it -- the same meaning Ctrl already
+	  - Ctrl, for Rectangle/Ellipse only: the first click's point acts as
+	    the shape's *center* rather than a corner, growing symmetrically
+	    as the cursor moves away from it -- the same meaning Ctrl already
 	    has on a Resize handle (anchor at center). Deliberately not
 	    offered for Line: unlike the Rectangle/Ellipse case, there's no
 	    established convention for "a line grows symmetrically from its
@@ -100,12 +100,18 @@ DiagramEventAddShape::~DiagramEventAddShape()
 	    dragged dimensions is currently larger and mirroring that onto
 	    the other, preserving the direction the user is actually
 	    dragging in.
-	Both can combine (Ctrl+Shift: a centered square/circle). Whether or
-	not Ctrl is currently held, the non-anchored branch always rebuilds
-	from m_anchor_point rather than nudging the existing rect/line --
-	otherwise, if Ctrl had been held earlier in the same drag (moving the
-	shape's own first point to a mirrored position), releasing it would
-	leave that point stuck there instead of actually restoring it.
+	Both can combine (Ctrl+Shift: a centered square/circle). Whether Ctrl
+	currently anchors from the center is re-decided on every call, from
+	the live modifiers passed in here -- not frozen at whatever was held
+	on the first click. Every comparable tool (Illustrator, Photoshop,
+	Figma, Inkscape...) lets you press or release the center-origin
+	modifier at any point mid-drag, with the shape immediately jumping to
+	match; that jump is the expected feedback for changing which point is
+	anchored, not a glitch. Freezing the choice at the first click instead
+	meant holding Ctrl anywhere other than the initial mouse-down did
+	nothing visible -- which, tried the more natural way (drag first,
+	then reach for Ctrl once you decide you want it centered), read as
+	"Ctrl doesn't work" rather than as a deliberate one-shot decision.
 */
 void DiagramEventAddShape::applyPosition(const QPointF &pos, Qt::KeyboardModifiers mods)
 {
@@ -125,14 +131,13 @@ void DiagramEventAddShape::applyPosition(const QPointF &pos, Qt::KeyboardModifie
 		return;
 	}
 
-	// m_center_anchored is decided once, in mousePressEvent, not
-	// re-checked here on every call -- re-checking it live meant
-	// releasing Ctrl mid-drag (something you'd naturally do the moment
-	// your hand gets tired holding it, long before you're done resizing)
-	// silently snapped the shape back to corner-anchored, discarding
-	// what felt like an already-made decision. Deciding it once at the
-	// first click matches "I held Ctrl when I clicked, so this shape is
-	// centered" -- a single, predictable rule instead of a live toggle.
+	m_center_anchored = (mods & Qt::ControlModifier)
+			&& (m_shape_type == QetShapeItem::Rectangle || m_shape_type == QetShapeItem::Ellipse);
+	if (m_center_anchored)
+		showCenterMarker(m_anchor_point);
+	else
+		hideCenterMarker();
+
 	QPointF target = pos;
 
 	if ((mods & Qt::ShiftModifier)
@@ -218,10 +223,9 @@ void DiagramEventAddShape::mousePressEvent(QGraphicsSceneMouseEvent *event)
 		{
 			m_shape_item = new QetShapeItem(pos, pos, m_shape_type);
 			m_anchor_point = pos;
-			// Decided once, here, rather than re-checked on every mouse
-			// move for the rest of the drag -- see applyPosition()'s doc
-			// comment for why continuous re-checking made releasing Ctrl
-			// mid-drag feel like a bug rather than a deliberate choice.
+			// Initial feedback only -- applyPosition() re-decides this
+			// live on every subsequent move, from whatever Ctrl state is
+			// held at the time.
 			m_center_anchored = (event->modifiers() & Qt::ControlModifier)
 					&& (m_shape_type == QetShapeItem::Rectangle || m_shape_type == QetShapeItem::Ellipse);
 			if (m_center_anchored)

sources/diagramevent/diagrameventaddshape.h

@@ -59,7 +59,7 @@ class DiagramEventAddShape : public DiagramEventInterface
 		QGraphicsLineItem       *m_help_horiz, *m_help_verti;
 		QPointF                  m_anchor_point;   // the shape's first-click point -- meaningful once m_shape_item exists
 		QGraphicsEllipseItem    *m_center_marker = nullptr;   // shown only while Ctrl-anchoring is actually in effect, so it doubles as confirmation that it is
-		bool                     m_center_anchored = false;   // decided once, at the first click -- see applyPosition()'s doc comment for why
+		bool                     m_center_anchored = false;   // re-decided live on every applyPosition() call, from current Ctrl state
 		QPointF                  m_last_mouse_scene_pos;      // raw, unsnapped -- lets a modifier-only change re-snap correctly when reapplied
 };
 

sources/diagramview.cpp

@@ -584,6 +584,7 @@ void DiagramView::mousePressEvent(QMouseEvent *e)
 */
 void DiagramView::mouseMoveEvent(QMouseEvent *e)
 {
+	m_last_mouse_pos = e->pos();
 	setToolTip(tr("X: %1 Y: %2").arg(e->pos().x()).arg(e->pos().y()));
 	if (m_event_interface && m_event_interface->mouseMoveEvent(e)) return;
 

sources/diagramview.h

@@ -55,6 +55,7 @@ class DiagramView : public PaletteGraphicsView
 		QAction			 *m_multi_paste = nullptr;
 		QAction          *m_create_template = nullptr;
 		QPoint            m_paste_here_pos;
+		QPoint            m_last_mouse_pos = QPoint(-1, -1);
 		QPointF           m_drag_last_pos;
 		bool              m_fresh_focus_in,
 						  m_first_activation = true;
@@ -71,6 +72,12 @@ class DiagramView : public PaletteGraphicsView
 		void editSelection();
 		void setEventInterface (DVEventInterface *event_interface);
 		QList<QAction *> contextMenuActions() const;
+		/// Last mouse position seen by mouseMoveEvent(), in viewport
+		/// coordinates -- (-1, -1) if the mouse hasn't moved over this
+		/// view yet. Filled from ordinary Qt mouse events, not a global
+		/// cursor query (QCursor::pos()/setPos() are silently ignored by
+		/// several window managers and compositors, Wayland included).
+		QPoint lastMousePos() const { return m_last_mouse_pos; }
 	
 	protected:
 		void mouseDoubleClickEvent(QMouseEvent *) override;

sources/qetdiagrameditor.cpp

@@ -367,10 +367,18 @@ void QETDiagramEditor::setUpActions()
 			//original, where it was easy to miss entirely; now it appears
 			//under the cursor and follows it until a click, Return, or Escape
 			//to cancel -- the same interaction as placing a new element.
-		const QPoint view_pos = dv->viewport()->mapFromGlobal(QCursor::pos());
-		const QPointF start_pos = dv->viewport()->rect().contains(view_pos)
-				? dv->mapToScene(view_pos)
-				: dv->mapToScene(dv->viewport()->rect().center());
+			//
+			//dv->lastMousePos() (an ordinary Qt mouse-move position), not
+			//QCursor::pos() (a global, OS-level cursor query): several
+			//window managers and compositors -- Wayland in particular --
+			//silently refuse that query, returning a stale or wrong
+			//position, which is exactly what made the pasted content land
+			//far from the cursor instead of under it.
+		const QPoint last_pos = dv->lastMousePos();
+		const QPoint view_pos = (last_pos.x() >= 0 && dv->viewport()->rect().contains(last_pos))
+				? last_pos
+				: dv->viewport()->rect().center();
+		const QPointF start_pos = dv->mapToScene(view_pos);
 
 		dv->diagram()->setEventInterface(
 					new DiagramEventAddPaste(dv->diagram(), start_pos));

sources/qetgraphicsitem/diagramimageitem.cpp

@@ -890,7 +890,17 @@ void DiagramImageItem::handlerMouseReleaseEvent(int index)
 */
 void DiagramImageItem::dragResize(int index, const QPointF &localPos, Qt::KeyboardModifiers mods)
 {
-	const QPointF pivotScene = pos() + m_transform.pivot;
+	// mapToScene(), not the pos() + m_transform.pivot shortcut: that
+	// shortcut only holds because the pivot is a fixed point of
+	// m_transform.toMatrix() -- true exactly as long as m_transform is
+	// the item's ONLY transform. It silently breaks the moment anything
+	// else is composed on top (QGraphicsItem's own scale()/rotation(),
+	// or a future parent-group transform), giving a scenePivot that
+	// doesn't match where the pivot handle is actually drawn (rebuildHandles()
+	// already uses mapToScene() for that, which is why the handle looks
+	// right even when a drag computed from this shortcut doesn't).
+	// mapToScene() is correct regardless of what else is composed in.
+	const QPointF pivotScene = mapToScene(m_transform.pivot);
 	QTransform undoRotation;
 	undoRotation.rotate(-m_transform.rotation);
 	const QPointF postScaleOffset = undoRotation.map(mapToScene(localPos) - pivotScene);
@@ -961,7 +971,9 @@ void DiagramImageItem::dragResize(int index, const QPointF &localPos, Qt::Keyboa
 */
 void DiagramImageItem::dragRotateHandle(int cornerIndex, const QPointF &scenePos, Qt::KeyboardModifiers mods)
 {
-	const QPointF scenePivot = pos() + m_transform.pivot;
+	// mapToScene(), not pos() + m_transform.pivot: see dragResize()'s
+	// identical comment for why.
+	const QPointF scenePivot = mapToScene(m_transform.pivot);
 	const qreal angleMouse = qRadiansToDegrees(qAtan2(scenePos.y() - scenePivot.y(), scenePos.x() - scenePivot.x()));
 
 	const QPointF reference = scaleAndShearOffset(cornerPosition(cornerIndex, pixmap_.width(), pixmap_.height()));
@@ -995,7 +1007,9 @@ void DiagramImageItem::dragSkewHandle(int edgeIndex, const QPointF &scenePos, Qt
 
 	const qreal rad = qDegreesToRadians(m_transform.rotation);
 	const qreal c = qCos(rad), s = qSin(rad);
-	const QPointF targetRel = scenePos - pos() - pivot;
+	// mapToScene(pivot), not pos() + pivot: see dragResize()'s identical
+	// comment for why.
+	const QPointF targetRel = scenePos - mapToScene(pivot);
 	const QPointF M(targetRel.x() * c + targetRel.y() * s,
 	               -targetRel.x() * s + targetRel.y() * c);
 

sources/qetgraphicsitem/qetshapeitem.cpp

@@ -2385,7 +2385,7 @@ void QetShapeItem::dragResize(int index, const QPointF &localPos, Qt::KeyboardMo
 			: QetGraphicsHandlerUtility::rectForPosAtIndex(localRect(), localPos, index);
 
 	if (mods & Qt::ShiftModifier)
-		newRect = lockAspectRatio(localRect(), newRect, index, mirrored);
+		newRect = lockAspectRatio(QRectF(m_old_P1, m_old_P2).normalized(), newRect, index, mirrored);
 
 	setRect(newRect.normalized());
 }