cornerstone3D
cornerstone3D copied to clipboard
feat: added touch events to @cornerstonejs/tools
Adds the following cornerstone events:
TOUCH_START,
TOUCH_START_ACTIVATE,
MULTI_TOUCH_START,
MULTI_TOUCH_START_ACTIVATE,
TOUCH_DRAG,
MULTI_TOUCH_DRAG,
TOUCH_END
Currently only uses native browser events (touchstart, touchmove, touchend)
Deploy Preview for cornerstone-3d-docs ready!
Name | Link |
---|---|
Latest commit | ec264ffe988bd235f32c0f259b5c50948cf45ff8 |
Latest deploy log | https://app.netlify.com/sites/cornerstone-3d-docs/deploys/63d41aaedfdab00009c7464f |
Deploy Preview | https://deploy-preview-247--cornerstone-3d-docs.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
@sedghi let me know what you think
This looks great at first sight. So are you plannig to uncommente the touch event listeners in annotation tools later? Like here https://github.com/cornerstonejs/cornerstone3D-beta/blob/main/packages/tools/src/tools/annotation/LengthTool.ts#L490
Also, are you saying we don't need the hammerJS anymore?
I think either hammerjs or contactjs will be needed to trigger the pinch/pan functionality
So I had some questions regarding hammerjs usage in the cornerstoneTools v2.
I noticed that hammerjs has a press and tap functionality, but cornerstone touch eventlistener seem to internally implement the press/tap. Was there a reason for this? https://github.com/cornerstonejs/cornerstoneTools/pull/120
Last question in cornerstoneTools v2, there were some touch specific tool copies (pan vs panmultitouch) would it make more sense to have the pan tool have configuration which enables multitooltouch instead of these being two separate tools?
Hammer.js would add MULTI_TOUCH_PAN, PINCH, and ROTATE, PRESS (longer and tap)
Example to get shown need to be added here
In case of changing serve, we need to update the docs
Is this ready to go?
Not yet, am in the process of adding the multitouch and gestures. I think we can avoid hammerjs dependency as well
@sedghi this is almost ready for your review. This is becoming a larger change than I initially intended.
Highlights
- We don't use the hammerjs dependency
- Multi touch is "reduced" to single touch via taking means.
- Distances are calculated as the mean distance of unique unordered points
- In the case of a drag event where the user lifts a finger for example, the deltas of 3 last touch => 2 current touch are calculated as the mean point of the 3touch case minus the mean point of the 2touch case
- Rotation is still todo, but following the case above we can take the mean of the 3 touch case and the mean of the 2 touch case. We can first translate the the 2 touch points by looking at the difference in mean points from 2points => 3points. Then for each matched point identifier we can calculate the theta and take the mean of all thetas.
TODO:
- Since touch handles an array of touch points we have an array of IPoints[] as the currentPoints, lastPoints, and startPoints. This doesn't play nice with the mouse interface since IPoints is not an array.
- I'll probably keep the IPoints[] under a different key (currentPointsList?). Then I'll reserve the original key with a datatype compatible with IPoints
Hey all,
Is there any news on this PR? This is a very handy feature.
@BardiaKh currently in progress!
@sedghi @wayfarer3130 this should be ready for review. Main changes to note:
-
TouchEvents and MouseEvents Types have been better merged to be InteractionEvent types. Tools that are agnostic to Touch and Mouse can use this event type instead. Touch and Mouse events then inherit from the interaction event in case there are specific needs (mouseButton, touch pressure, etc).
-
We no longer use hammerjs for touch compared to v2
@diegohennrich @Ouwen Let me know when this is ready for review Also you need to remove the type, we don't need that anymore
Could you please review it? @Ouwen @sedghi
@sedghi lmk if there are any outstanding changes required aside from the above... thanks again for taking the time to review this large diff which pokes other existing files.
@Ouwen probably yes, but I think I would like to do one last review. Just let me know when it is ready for it
Could you review it, please? @Ouwen @sedghi
I think everything was done. Could you review it, please? @Ouwen @sedghi
@sedghi, I went through another pass at it, I think it is ready for review