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.

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

Use tablet events for tablet input #1059

Merged
merged 5 commits into from Nov 1, 2018

Conversation

Projects
None yet
3 participants
@scribblemaniac
Copy link
Member

scribblemaniac commented Sep 7, 2018

From the Qt docs:

Qt will first send a tablet event, then if it is not accepted by any widget, it will send a mouse event. This allows users of applications that are not designed for tablets to use a tablet like a mouse. However high-resolution drawing applications should handle the tablet events, because they can occur at a higher frequency, which is a benefit for smooth and accurate drawing. If the tablet events are rejected, the synthetic mouse events may be compressed for efficiency.

So I refactored the code to handle the tablet event instead of ignoring it and Qt possibly compressing it as mouse events.

Please test thoroughly as I don't have a tablet to test this on myself. Particularly on a Wacom as I did remove the "patch". As far as I can tell for tablet events, event->posF() should equal event->pos() + mTabletPosition - mTabletPosition.toPoint().

@scribblemaniac scribblemaniac requested a review from chchwy Sep 7, 2018

@chchwy

This comment has been minimized.

Copy link
Member

chchwy commented Sep 7, 2018

Thanks @scribblemaniac will test it as long as I have the tablet with me.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Sep 18, 2018

herp
This is what I'm experiencing now with my tablet..
small strokes are me tapping on the screen, long strokes are continous... something is definitely not right.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Sep 18, 2018

Agh... the problem is so obvious. We don't do anything with tablet events... all our tools expect QMouseEvents which are called from scribblearea::mouse<press,release,move>event(). The corresponding tabletmoveevent does not do anything so of course tablet support is broken now that we're trying to use those events.

The solution is rather simple:
implement virtual tabletmove, release and press functions in basetool and implement the functions in our tools or.. maybe better, in the stroketool.

@scribblemaniac

This comment has been minimized.

Copy link
Member Author

scribblemaniac commented Sep 18, 2018

We don't do anything with tablet events... all our tools expect QMouseEvents which are called from scribblearea::mouse<press,release,move>event(). The corresponding tabletmoveevent does not do anything so of course tablet support is broken now that we're trying to use those events.

@CandyFace Thanks for testing. I don't know how I missed that. I guess I saw the stroke manager call in scribblearea and assumed all of them were being done from there, not just the first and last points.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Sep 19, 2018

As discussed in Discord, I have now implemented the tablet overrides in the drawing tools. I also couldn't resist doing a bit of refactoring.

I have tested the implementation using my Artisul D13 tablet.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Sep 19, 2018

Still need to implement for hand, move and select. Will do that tomorrow

edit: still actively looking into this... I've hit a rather peculiar problem though that affects select and move tool tablet... which means they break.

For some reason the transformation is not the same... which is odd considering that the mouse and tablet both seem to have identical coordinates ( ̄_ ̄ )

@CandyFace CandyFace changed the title Improve tablet resolution? Use tablet events for tablet input Sep 23, 2018

@CandyFace CandyFace self-assigned this Sep 23, 2018

@CandyFace CandyFace force-pushed the scribblemaniac:tablet-improve-1 branch from fc8ef5a to 515dcf2 Sep 26, 2018

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Sep 26, 2018

Alright this should be good to go now. I could still refactor a lot to get that dreadful copy paste away though but functionality wise, all tools should work now with proper tablet input.

@scribblemaniac
Copy link
Member Author

scribblemaniac left a comment

@CandyFace So now most things appear to be working for the mouse. However when I am using a drawing tool and then I move the canvas with the middle mouse button, the program remains stuck in that mode and I cannot draw until I switch tools.

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Sep 27, 2018

Agh... will fix when I get home. I haven't been able to test that because I don't have a middle mouse button but I'll figure something out.

@scribblemaniac

This comment has been minimized.

Copy link
Member Author

scribblemaniac commented Sep 27, 2018

Agh... will fix when I get home. I haven't been able to test that because I don't have a middle mouse button but I'll figure something out.

Okay, I figured it would be easier for me to fix it then. Ended up being a very simple fix. This should be good to go now as far as I can tell. 👍

@chchwy chchwy modified the milestones: 0.6.4, 0.6.3 Oct 5, 2018

@chchwy

This comment has been minimized.

Copy link
Member

chchwy commented Oct 9, 2018

Looking at this PR again, I have a question.
Is it necessary to introduce these 3 functions?

void BaseTool::tabletMoveEvent(QTabletEvent*);
void BaseTool::tabletPressEvent(QTabletEvent*);
void BaseTool::tabletReleaseEvent(QTabletEvent*);

Seems they are doing exactly what mouseEvent functions are doing, Is it correct? Or are there any differences for a specific tool?

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Oct 9, 2018

@chchwy You are correct, the tabletevent and mouseevent functionality is very similar (and identical in some cases) to each other (especially after i've moved all the button related code out) and could be merged. We could consider getting rid of QTabletEvent and QMouseEvent and make it possible to get them as value from Basetool or... create a new custom struct that holds the values we require from each EventType.

@chchwy

This comment has been minimized.

Copy link
Member

chchwy commented Oct 14, 2018

Pencil, pen, brush tools are always active in vector layers if using a tablet. But working ok with a mouse.

@chchwy chchwy merged commit 92e7ae8 into pencil2d:master Nov 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.