Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support multi-touch #963

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

support multi-touch #963

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ad0a9c
multi-touch
ThomasLucky13 May 15, 2024
f42ce1e
clear code
ThomasLucky13 Jun 10, 2024
79de84d
changing the code as recommended by letsfindaway
ThomasLucky13 Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/board/UBBoardView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1924,3 +1924,13 @@ bool UBBoardView::hasSelectedParents(QGraphicsItem * item)
return false;
return hasSelectedParents(item->parentItem());
}

void UBBoardView::releaseAllInputDevices()
{
if (mMouseButtonIsPressed || mTabletStylusIsPressed || mPendingStylusReleaseEvent)
{
mMouseButtonIsPressed = false;
mTabletStylusIsPressed = false;
mPendingStylusReleaseEvent = false;
}
letsfindaway marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 2 additions & 0 deletions src/board/UBBoardView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ class UBBoardView : public QGraphicsView

void setMultiselection(bool enable);
bool isMultipleSelectionEnabled() { return mMultipleSelectionIsEnabled; }

void releaseAllInputDevices();
// work around for handling tablet events on MAC OS with Qt 4.8.0 and above
#if defined(Q_OS_OSX)
bool directTabletEvent(QEvent *event);
Expand Down
70 changes: 70 additions & 0 deletions src/domain/UBGraphicsScene.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ UBGraphicsScene::UBGraphicsScene(std::shared_ptr<UBDocumentProxy> document, bool
// connect(this, SIGNAL(selectionChanged()), this, SLOT(selectionChangedProcessing()));
connect(UBApplication::undoStack.data(), SIGNAL(indexChanged(int)), this, SLOT(updateSelectionFrameWrapper(int)));
connect(UBDrawingController::drawingController(), SIGNAL(stylusToolChanged(int,int)), this, SLOT(stylusToolChanged(int,int)));

installEventFilter(this);
}

UBGraphicsScene::~UBGraphicsScene()
Expand Down Expand Up @@ -714,6 +716,7 @@ bool UBGraphicsScene::inputDeviceRelease(int tool)
addPolygonItemToCurrentStroke(poly);
}

if(multiDrawLines.isEmpty()) // is it not polygons drawing by multiDraw
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use braces and indentation to better indicate the scope.

// replace the stroke by a simplified version of it
if ((currentTool == UBStylusTool::Pen && UBSettings::settings()->boardSimplifyPenStrokes->get().toBool())
|| (currentTool == UBStylusTool::Marker && UBSettings::settings()->boardSimplifyMarkerStrokes->get().toBool()))
Expand Down Expand Up @@ -2052,6 +2055,27 @@ bool UBGraphicsScene::isEmpty() const
return mItemCount == 0;
}

bool UBGraphicsScene::eventFilter(QObject *watched, QEvent *event)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to use an eventFilter in the scene instead of overriding event() in the view? I would assume (but have not tested!) that the touch events arrive at the view and can be handled there. If this is the case, then I would also move the event handling to the view, while the multi-touch drawing code could be here in the scene.

What you're touching with your finger is a screen with a widget, which is the view. You don't "touch" the scene. The scene just holds all the graphics items.

{
if( UBApplication::applicationController != NULL ) // it needs to work only on Board mode
if( UBApplication::applicationController->displayMode() != UBApplicationController::Board ||
UBApplication::applicationController->isShowingDesktop())
return false;
if (watched == this)
{
UBStylusTool::Enum currentTool = (UBStylusTool::Enum)UBDrawingController::drawingController()->stylusTool();
if ((event->type() == QEvent::TouchUpdate || event->type() == QEvent::TouchEnd) //for use multiDraw
&& (currentTool == UBStylusTool::Pen || currentTool == UBStylusTool::Marker)) // when Pen or Marker
{
MultiTouchDrawing(static_cast<QTouchEvent*>(event), currentTool);
if (event->type() == QEvent::TouchEnd) //end of multiDraw
MultiTouchEndDrawing();
return true;
}
}
return false;
}

QGraphicsItem* UBGraphicsScene::setAsBackgroundObject(QGraphicsItem* item, bool pAdaptTransformation, bool pExpand)
{
if (mBackgroundObject)
Expand Down Expand Up @@ -3122,3 +3146,49 @@ void UBGraphicsScene::initStroke()
{
mCurrentStroke = new UBGraphicsStroke(shared_from_this());
}

void UBGraphicsScene::MultiTouchDrawing(QTouchEvent* event, UBStylusTool::Enum currentTool)
{
QList <QTouchEvent::TouchPoint> touchPoints = event->touchPoints();
foreach (QTouchEvent::TouchPoint point, touchPoints)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qt6 recommends to prefer the C++ range based for over the foreach keyword. See https://doc.qt.io/qt-6/foreach-keyword.html. It is also recommended to use a const reference in a case like this to avoid copying the point.

{
lastPoint_m = point.lastPos();
endPoint_m = point.pos();
Comment on lines +3157 to +3158
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not have lastPoint_m and endPoint_m as local variables if they are reassigned every time?


int distance = sqrt(pow((lastPoint_m.x() - endPoint_m.x()),2) + pow((lastPoint_m.y() - endPoint_m.y()),2)) + 1;
distance = sqrt(distance);
if (distance > 6)
distance = 6;
else if(distance < 4)
distance = 4;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is distance used for? It's defined here but never used.


UBBoardView* boardView = controlView();
QLineF line;
line.setP1(boardView->mapToScene(UBGeometryUtils::pointConstrainedInRect(lastPoint_m.toPoint(), boardView->rect())));
line.setP2(boardView->mapToScene(UBGeometryUtils::pointConstrainedInRect(endPoint_m.toPoint(), boardView->rect())));
if (!multiDrawLines.contains(line)) // to eliminate duplicates
{
multiDrawLines.append(line);
Comment on lines +3164 to +3166
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of performance, QList::contains has linear runtime in the number of lines contained, especially if the line isn't in the list. Are duplicates a serious issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important because there are so many duplicates that the page can take a long time to load

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are spending so much time on searching rather than appending, it might be worth using QSet instead of QList. QSet requires a bit more memory, but look-up is really fast.
Can you test it with QSet and report back whether it makes an appreciable difference in performance? Otherwise I'd just leave it as is.


qreal penWidth = 0;
if (currentTool == UBStylusTool::Pen)
penWidth = UBSettings::settings()->currentPenWidth();
else if (currentTool == UBStylusTool::Marker)
penWidth = UBSettings::settings()->currentMarkerWidth();
penWidth /= UBApplication::boardController->systemScaleFactor();
penWidth /= UBApplication::boardController->currentZoom();

UBGraphicsPolygonItem *polygonItem = lineToPolygonItem(line, penWidth, penWidth);
addPolygonItemToCurrentStroke(polygonItem);
}

lastPoint_m = endPoint_m;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have any effect? It's overwritten later, see above.

}
}

void UBGraphicsScene::MultiTouchEndDrawing()
{
inputDeviceRelease();
multiDrawLines.clear();
controlView()->releaseAllInputDevices();
}
7 changes: 7 additions & 0 deletions src/domain/UBGraphicsScene.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ class UBGraphicsScene: public UBCoreGraphicsScene, public UBItem, public std::en

QRectF annotationsBoundingRect() const;

bool eventFilter(QObject *watched, QEvent *event) override;

public slots:
void updateSelectionFrame();
void updateSelectionFrameWrapper(int);
Expand Down Expand Up @@ -415,6 +417,8 @@ public slots:
void hideMarkerCircle();
void hidePenCircle();
void DisposeMagnifierQWidgets();
void MultiTouchDrawing(QTouchEvent* event, UBStylusTool::Enum currentTool);
void MultiTouchEndDrawing();
letsfindaway marked this conversation as resolved.
Show resolved Hide resolved


virtual void keyReleaseEvent(QKeyEvent * keyEvent);
Expand Down Expand Up @@ -500,6 +504,9 @@ public slots:
UBSelectionFrame *mSelectionFrame;

UBGraphicsCache* mGraphicsCache;

QPointF lastPoint_m, endPoint_m;
QList<QLineF> multiDrawLines;
};


Expand Down