Merge pull request #1555 from DaTweety/fix/element-text-resize-corner

Symbol text width handles: the point the text is aligned on stays in place

sources/qetgraphicsitem/dynamicelementtextitem.cpp

@@ -16,6 +16,9 @@
 	along with QElectroTech.  If not, see <http://www.gnu.org/licenses/>.
 */
 #include "dynamicelementtextitem.h"
+#include <QScopedPointer>
+#include <QTextDocument>
+#include "../QetGraphicsItemModeler/textresizehandles.h"
 #include "../shownkinds.h"
 #include "../qetproject.h"
 #include "../QPropertyUndoCommand/qpropertyundocommand.h"
@@ -1050,7 +1053,53 @@ void DynamicElementTextItem::handlerMousePressEvent(QetGraphicsHandlerItem *hand
 		//A concrete baseline for the live drag's delta math, which can't
 		//start from -1.
 	m_resize_baseline_width = (m_resize_original_width < 0) ? frameRect().width() : m_resize_original_width;
-	m_resize_start_local_x = mapFromScene(event->scenePos()).x();
+		//The text moves while it is dragged, to keep its alignment point
+		//in place: read the mouse in the text's coordinates as they were
+		//at the press, otherwise the delta grows with every step
+	m_resize_scene_to_local = sceneTransform().inverted();
+	m_resize_start_local_x = m_resize_scene_to_local.map(event->scenePos()).x();
+
+		//The point the text is aligned on stays where it is: remember it
+		//in the parent's coordinates, which take the text's rotation
+		//into account
+	m_resize_start_pos = pos();
+	m_resize_fixed_parent = mapToParent(resizeAnchor());
+}
+
+/**
+	@brief DynamicElementTextItem::minimumTextWidth
+	@return the narrowest width the text can be dragged to: that of its
+	widest word. Narrower, a long word runs out of the text's box while
+	the short lines are still aligned inside it, and a centred or right
+	aligned text falls apart (a lone "1" in the middle under its words).
+*/
+qreal DynamicElementTextItem::minimumTextWidth() const
+{
+	QScopedPointer<QTextDocument> narrowest(document()->clone());
+	narrowest->setTextWidth(0);
+	return qMax(narrowest->idealWidth(), qreal(10));
+}
+
+/**
+	@brief DynamicElementTextItem::resizeAnchor
+	@return the point of the text, in its own coordinates, that a drag on
+	a width handle keeps in place: the point it is aligned on, one of the
+	nine of its alignment (left, centre or right; top, middle or bottom).
+	A text aligned top right keeps its top right corner whichever handle
+	is dragged, and a text that wraps onto more lines while it narrows
+	grows away from that point instead of moving into its neighbours.
+*/
+QPointF DynamicElementTextItem::resizeAnchor() const
+{
+	const QRectF br = boundingRect();
+	const Qt::Alignment a = alignment();
+	const qreal x = (a & Qt::AlignRight) ? br.right()
+				  : (a & Qt::AlignHCenter) ? br.center().x()
+				  : br.left();
+	const qreal y = (a & Qt::AlignBottom) ? br.bottom()
+				  : (a & Qt::AlignVCenter) ? br.center().y()
+				  : br.top();
+	return QPointF(x, y);
 }
 
 /**
@@ -1065,21 +1114,26 @@ void DynamicElementTextItem::handlerMousePressEvent(QetGraphicsHandlerItem *hand
 */
 void DynamicElementTextItem::handlerMouseMoveEvent(QetGraphicsHandlerItem *handle, QGraphicsSceneMouseEvent *event)
 {
-	qreal local_x = mapFromScene(event->scenePos()).x();
+	qreal local_x = m_resize_scene_to_local.map(event->scenePos()).x();
 	qreal delta = local_x - m_resize_start_local_x;
 	if (handle == m_left_resize_handle)
 		delta = -delta;
 
-	qreal new_width = qMax(m_resize_baseline_width + delta, qreal(10));
+	qreal new_width = qMax(m_resize_baseline_width + delta, minimumTextWidth());
 	setTextWidth(new_width);
+
+		//Keep the point the text is aligned on in place, whatever the new
+		//width and number of lines
+	setPos(pos() + m_resize_fixed_parent - mapToParent(resizeAnchor()));
 	updateResizeHandlesPos();
 }
 
 /**
 	@brief DynamicElementTextItem::handlerMouseReleaseEvent
-	Push the same QPropertyUndoCommand the properties-panel width spinbox
-	already pushes (sources/ui/dynamicelementtextmodel.cpp) -- the value is
-	already applied live from the drag, so this only makes it undoable.
+	Make the drag undoable with a TextResizeCommand, as the corner handles
+	of a free text do: the width and, after a drag on the left handle, the
+	position that kept the right edge in place. Both are already applied,
+	live from the drag.
 	@param handle
 	@param event
 */
@@ -1089,10 +1143,13 @@ void DynamicElementTextItem::handlerMouseReleaseEvent(QetGraphicsHandlerItem *ha
 	Q_UNUSED(event)
 
 	qreal new_width = textWidth();
-	if (!qFuzzyCompare(m_resize_original_width, new_width) && m_parent_element && m_parent_element->diagram())
+	if ((!qFuzzyCompare(m_resize_original_width, new_width) || pos() != m_resize_start_pos)
+			&& m_parent_element && m_parent_element->diagram())
 	{
-		auto *undo = new QPropertyUndoCommand(this, "textWidth", QVariant(m_resize_original_width), QVariant(new_width));
-		undo->setAnimated(true, false);
+			//Width and position together: undoing a drag on the left
+			//handle puts the text back where it was
+		auto *undo = new TextResizeCommand(this, m_resize_original_width, new_width,
+										   m_resize_start_pos, pos());
 		undo->setText(tr("Resize an element text"));
 		m_parent_element->diagram()->undoStack().push(undo);
 	}

sources/qetgraphicsitem/dynamicelementtextitem.h

@@ -170,6 +170,8 @@ class DynamicElementTextItem : public DiagramTextItem
 		void addResizeHandles();
 		void removeResizeHandles();
 		void updateResizeHandlesPos();
+		QPointF resizeAnchor() const;
+		qreal minimumTextWidth() const;
 		void fitAutoTextWidth();
 		void handlerMousePressEvent(QetGraphicsHandlerItem *handle, QGraphicsSceneMouseEvent *event);
 		void handlerMouseMoveEvent(QetGraphicsHandlerItem *handle, QGraphicsSceneMouseEvent *event);
@@ -213,6 +215,9 @@ class DynamicElementTextItem : public DiagramTextItem
 		qreal m_resize_original_width = -1;
 		qreal m_resize_baseline_width = -1;
 		qreal m_resize_start_local_x = 0;
+		QPointF m_resize_start_pos,
+				m_resize_fixed_parent;
+		QTransform m_resize_scene_to_local;
 		QMetaObject::Connection m_resize_handles_con;
 };
 

tests/qttest/CMakeLists.txt

@@ -130,6 +130,15 @@ if(Python3_Interpreter_FOUND AND CMAKE_GENERATOR STREQUAL "Ninja")
       --source ${CMAKE_CURRENT_SOURCE_DIR}/image_dark_integration_probe.cpp
       --fixture ${CMAKE_CURRENT_SOURCE_DIR}/fixtures/unlinked_contact_label.qet)
   set_tests_properties(tst_darkimageintegration PROPERTIES TIMEOUT 300 RUN_SERIAL TRUE)
+  # Dragging the left width handle of a symbol's text keeps its right
+  # edge in place, the right handle its left edge; undo restores both.
+  add_test(NAME tst_elementtextresizeintegration
+    COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/run_element_text_resize_probe.py
+      --build ${CMAKE_BINARY_DIR} --binary $<TARGET_FILE:qelectrotech>
+      --source ${CMAKE_CURRENT_SOURCE_DIR}/element_text_resize_probe.cpp
+      --fixture ${CMAKE_CURRENT_SOURCE_DIR}/fixtures/unlinked_contact_label.qet)
+  set_tests_properties(tst_elementtextresizeintegration PROPERTIES TIMEOUT 300 RUN_SERIAL TRUE
+    ENVIRONMENT "QT_QPA_PLATFORM=offscreen")
 endif()
 
 add_executable(${PROJECT_NAME} tst_mytest.cpp)

tests/qttest/element_text_resize_probe.cpp

@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Width handles of a symbol's text, linked against the application's objects.
+//
+// Dragging a width handle of a symbol's text changes its width and keeps
+// the point the text is aligned on in place, one of the nine of its
+// alignment: a text aligned top right keeps its top right corner, one
+// centred its middle. Before, the text always kept its left edge, so a
+// drag on the left handle moved the right edge and the text jumped. A
+// text narrowed until it wraps grows away from that point. Undo puts back
+// the width and the position together. Checked for four alignments, on
+// an upright and on a rotated text.
+#include <QApplication>
+#include <QFontDatabase>
+#include <QGraphicsSceneMouseEvent>
+#include <QSettings>
+#include <QTemporaryDir>
+#include <QTextBlock>
+#include <QTextDocument>
+#include <QTextLayout>
+#include <cstdio>
+#include <stdexcept>
+
+#include "../../sources/diagram.h"
+#include "../../sources/qetproject.h"
+#include "../../sources/qetmessagebox.h"
+#include "../../sources/qetgraphicsitem/element.h"
+#include "../../sources/qetgraphicsitem/dynamicelementtextitem.h"
+#include "../../sources/QetGraphicsItemModeler/qetgraphicshandleritem.h"
+
+static void check(bool value, const char *message)
+{
+	if (!value) throw std::runtime_error(message);
+}
+
+static bool near(const QPointF &a, const QPointF &b)
+{
+	return qAbs(a.x() - b.x()) < 0.01 && qAbs(a.y() - b.y()) < 0.01;
+}
+
+static void mouse(Diagram *scene, QGraphicsItem *item, QEvent::Type type, const QPointF &point)
+{
+	QGraphicsSceneMouseEvent event(type);
+	event.setButton(Qt::LeftButton);
+	event.setButtons(type == QEvent::GraphicsSceneMouseRelease ? Qt::NoButton : Qt::LeftButton);
+	event.setScenePos(point);
+	event.setPos(item->mapFromScene(point));
+	scene->sendEvent(item, &event);
+}
+
+/// The left or right width handle of text, by where it sits on the text.
+static QetGraphicsHandlerItem *handle(DynamicElementTextItem *text, bool left)
+{
+	QetGraphicsHandlerItem *found = nullptr;
+	for (QGraphicsItem *child : text->childItems()) {
+		if (child->type() != QetGraphicsHandlerItem::Type) continue;
+		const qreal x = child->pos().x();
+		if (!found || (left ? x < found->pos().x() : x > found->pos().x()))
+			found = static_cast<QetGraphicsHandlerItem *>(child);
+	}
+	check(found, "the selected symbol's text shows its width handles");
+	return found;
+}
+
+/// The point the text is aligned on, one of its nine, in the folio.
+static QPointF anchor(DynamicElementTextItem *text)
+{
+	const QRectF br = text->boundingRect();
+	const Qt::Alignment a = text->alignment();
+	const qreal x = (a & Qt::AlignRight) ? br.right() : (a & Qt::AlignHCenter) ? br.center().x() : br.left();
+	const qreal y = (a & Qt::AlignBottom) ? br.bottom() : (a & Qt::AlignVCenter) ? br.center().y() : br.top();
+	return text->mapToScene(QPointF(x, y));
+}
+
+/// Drag one handle by dx along the text's own baseline, in 30 steps as a
+/// mouse does: the text moves on every step to keep its alignment point,
+/// so each step must be read the same way as the first.
+static void drag(Diagram *scene, DynamicElementTextItem *text, bool left, qreal dx)
+{
+	QetGraphicsHandlerItem *h = handle(text, left);
+	const QPointF start = h->scenePos();
+	const QPointF end = start + text->mapToScene(QPointF(dx, 0)) - text->mapToScene(QPointF(0, 0));
+	mouse(scene, h, QEvent::GraphicsSceneMousePress, start);
+	for (int i = 1; i <= 30; ++i)
+		mouse(scene, h, QEvent::GraphicsSceneMouseMove, start + (end - start) * i / 30);
+	mouse(scene, h, QEvent::GraphicsSceneMouseRelease, end);
+}
+
+/// Every line lies inside the text's width: none runs out of it.
+static bool linesInside(DynamicElementTextItem *text)
+{
+	QTextDocument *doc = text->document();
+	const qreal inner = doc->textWidth() - 2 * doc->documentMargin();
+	for (QTextBlock b = doc->begin(); b.isValid(); b = b.next())
+		for (int i = 0; i < b.layout()->lineCount(); ++i) {
+			const QTextLine line = b.layout()->lineAt(i);
+			if (line.x() < -0.01 || line.x() + line.naturalTextWidth() > inner + 0.01)
+				return false;
+		}
+	return true;
+}
+
+/// The width changed by exactly as much as the mouse moved.
+static void checkGrowth(DynamicElementTextItem *text, qreal before, qreal moved, const char *message)
+{
+	check(qAbs(text->textWidth() - before - moved) < 0.5, message);
+}
+
+static void checkText(Diagram *scene, Element *element, DynamicElementTextItem *text, Qt::Alignment alignment)
+{
+	text->setTextWidth(-1);
+	text->setAlignment(alignment);
+	element->setSelected(true);
+	const QPointF pos = text->pos();
+	const qreal width = text->boundingRect().width();
+	const QPointF fixed = anchor(text);
+	const qreal frame = text->frameRect().width();
+
+	// Left handle 30 outwards: wider, the alignment point where it was.
+	drag(scene, text, true, -30);
+	checkGrowth(text, frame, 30, "the left handle widens the text by as much as the mouse moved");
+	check(near(anchor(text), fixed), "dragging the left handle keeps the alignment point in place");
+	scene->undoStack().undo();
+	check(near(text->pos(), pos), "undo puts the text back where it was");
+	check(qAbs(text->boundingRect().width() - width) < 0.01, "undo puts the width back");
+	scene->undoStack().redo();
+	check(near(anchor(text), fixed), "redo keeps the alignment point in place");
+	scene->undoStack().undo();
+
+	// Right handle 30 outwards: the same point stays.
+	element->setSelected(true);
+	drag(scene, text, false, 30);
+	checkGrowth(text, frame, 30, "the right handle widens the text by as much as the mouse moved");
+	check(near(anchor(text), fixed), "dragging the right handle keeps the alignment point in place");
+	scene->undoStack().undo();
+	check(near(text->pos(), pos), "undo of the right handle puts the text back");
+
+	// Narrowed until it wraps onto more lines: it grows away from the
+	// alignment point, a text aligned at the top downwards, so that it
+	// does not move into what stands above it.
+	element->setSelected(true);
+	const qreal height = text->boundingRect().height();
+	drag(scene, text, false, 25 - width);
+	check(text->boundingRect().height() > height + 5, "narrowing the text wraps it onto more lines");
+	check(linesInside(text), "a text is not narrower than its widest word: no line runs out of it");
+	check(near(anchor(text), fixed), "a text that wraps keeps its alignment point in place");
+	scene->undoStack().undo();
+	check(near(text->pos(), pos), "undo puts the wrapped text back");
+	element->setSelected(false);
+}
+
+static void checkAll(Diagram *scene, Element *element, DynamicElementTextItem *text)
+{
+	checkText(scene, element, text, Qt::AlignLeft | Qt::AlignTop);
+	checkText(scene, element, text, Qt::AlignRight | Qt::AlignTop);
+	checkText(scene, element, text, Qt::AlignHCenter | Qt::AlignVCenter);
+	checkText(scene, element, text, Qt::AlignRight | Qt::AlignBottom);
+	checkText(scene, element, text, Qt::AlignLeft | Qt::AlignBottom);
+}
+
+int main(int argc, char **argv)
+{
+	QApplication app(argc, argv);
+	std::freopen(argv[2], "w", stdout);
+
+	QTemporaryDir settings;
+	QSettings::setDefaultFormat(QSettings::IniFormat);
+	QSettings::setPath(QSettings::IniFormat, QSettings::UserScope, settings.path());
+	QCoreApplication::setOrganizationName("QETElementTextResizeRegression");
+	QETProject::setBackupEnabled(false);
+	QET::QetMessageBox::setNonInteractive(true);
+	QFontDatabase::addApplicationFont(":/fonts/LiberationSans-Regular.ttf");
+
+	try
+	{
+		QETProject project(QString::fromLocal8Bit(argv[1]));
+		check(project.state() == QETProject::Ok, "fixture opens");
+		project.buildFolios(project.diagrams());
+		Diagram *scene = project.diagrams().first();
+
+		DynamicElementTextItem *text = nullptr;
+		Element *element = nullptr;
+		for (Element *e : scene->elements())
+			for (DynamicElementTextItem *t : e->dynamicTextItems())
+				if (!text) { text = t; element = e; }
+		check(text, "the fixture has a symbol with a text");
+			// Several words, so that narrowing it makes it wrap
+		text->setTextFrom(DynamicElementTextItem::UserText);
+		text->setText(QStringLiteral("Sockets bathroom first floor, 5"));
+
+		checkAll(scene, element, text);
+		// The same on a text turned by 90 degrees: the fixed point is
+		// found along the text's own baseline, not the folio's x axis.
+		text->setRotation(90);
+		checkAll(scene, element, text);
+
+		std::puts("PASS: a symbol text's width handles keep its alignment point in place, undo restores both");
+	}
+	catch (const std::exception &error)
+	{
+		std::printf("FAIL: %s\n", error.what());
+		return 1;
+	}
+	return 0;
+}

tests/qttest/run_element_text_resize_probe.py

@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+"""Link the element text resize integration probe using an existing Ninja app build.
+
+Reuses app objects without duplicating the app or introducing a core-library
+refactor. No application source or existing executable is replaced. The
+generated manifest, probe binary, reports and export fixtures stay in build/.
+"""
+import argparse
+import pathlib
+import re
+import shutil
+import subprocess
+import sys
+
+parser = argparse.ArgumentParser()
+parser.add_argument('--build', required=True)
+parser.add_argument('--binary', required=True)
+parser.add_argument('--source', required=True)
+parser.add_argument('--fixture', required=True)
+args = parser.parse_args()
+build = pathlib.Path(args.build).resolve()
+binary = pathlib.Path(args.binary).resolve()
+source = pathlib.Path(args.source).resolve()
+fixture = pathlib.Path(args.fixture).resolve()
+manifest = (build / 'build.ninja').read_text(encoding='utf-8')
+match = re.search(r'^build (\S*?/sources/main\.cpp\.(?:obj|o)):', manifest, re.MULTILINE)
+if not match:
+    sys.exit('Cannot find the application main object in the Ninja build.')
+main_object = match.group(1)
+probe_object = main_object.replace('main.cpp.', 'element_text_resize_probe.cpp.')
+probe_binary = binary.with_name('element_text_resize_probe' + binary.suffix)
+def escape(path):
+    return pathlib.Path(path).as_posix().replace('$', '$$').replace(':', '$:').replace(' ', '$ ')
+main_source = source.parents[2] / 'sources' / 'main.cpp'
+manifest = manifest.replace(main_object, probe_object)
+manifest = manifest.replace(escape(main_source), escape(source))
+if binary.suffix:
+    manifest = manifest.replace(binary.name, probe_binary.name)
+else:
+    old_target = escape(binary.name if binary.parent == build else binary)
+    new_target = escape(probe_binary.name if probe_binary.parent == build else probe_binary)
+    manifest = manifest.replace('build ' + old_target + ':', 'build ' + new_target + ':')
+    old_file = binary.name if binary.parent == build else binary.as_posix()
+    new_file = probe_binary.name if probe_binary.parent == build else probe_binary.as_posix()
+    manifest = re.sub(r'^  TARGET_FILE = ' + re.escape(old_file) + r'$',
+                      '  TARGET_FILE = ' + new_file, manifest, flags=re.MULTILINE)
+probe_manifest = build / 'element-text-resize-probe.ninja'
+probe_manifest.write_text(manifest, encoding='utf-8')
+ninja = shutil.which('ninja')
+if not ninja:
+    sys.exit('Ninja is required for the integration probe.')
+target = probe_binary.name if probe_binary.parent == build else str(probe_binary)
+subprocess.run([ninja, '-C', str(build), '-f', probe_manifest.name, target], check=True)
+output = build / 'element-text-resize'
+output.mkdir(exist_ok=True)
+report = output / 'results.txt'
+result = subprocess.run([str(probe_binary), str(fixture), str(report), str(output)], timeout=90)
+if report.exists():
+    print(report.read_text(encoding='utf-8', errors='replace'))
+sys.exit(result.returncode)