improve the zooming interaction #5454
Conversation
improve the zooming interaction
|
|
Fix a bug when using loop() and noloop(), pmouseX and pmouseY might not be updated. So we need an extra frame to wait for pmouse to update when pmouse is far away from mouse location;
|
hi @yangshuzhan could you share more about the problem you are solving with this PR? if you are able to attach before and after gifs to show how zooming works differently, that would be very helpful! |
Sorry that I didn't describe it clearly. |
|
Thanks for this @yangshuzhan and for the documentation of the example. I do find that this improves the quality of interaction for the zooming with Once you open an issue to propose this change then you can fix the linting and testing errors. There is more info on this in the Contributors Docs (linked above). If you have trouble, feel free to ping me. |
|
Thanks @yangshuzhan! Like @stalgiag mentioned, an issue needs to be opened before a pull request is opened, then you can tag the issue in the pull request. You can read more about it in the GitHub Issue Flow. This is necessary for tracking the development and keeping discussions clear. Thanks! |
There was a problem hiding this comment.
Thanks for this change, the zooming motion definitely is smoother! I think before merging, we just need two things:
- Right now, in making zoom smoother, we accidentally also made orbiting much more sensitive, so we should make the scale amount be relative to the canvas size again
- The eslint check is currently failing due to these issues, which we'll need to resolve:
Additionally, while not an error, it would be great if we could match the spacing in the rest of the repo by putting spaces around operators, and adding spaces between
/home/runner/work/p5.js/p5.js/src/webgl/interaction.js:96:30: Expected '!==' and instead saw '!='. [Error/eqeqeq] /home/runner/work/p5.js/p5.js/src/webgl/interaction.js:108:1: This line has a length of 84. Maximum allowed is 80. [Error/max-len] /home/runner/work/p5.js/p5.js/src/webgl/interaction.js:109:84: Trailing spaces not allowed. [Error/no-trailing-spaces]ifand the condition.
| @@ -88,20 +88,22 @@ p5.prototype.orbitControl = function(sensitivityX, sensitivityY, sensitivityZ) { | |||
| this._setProperty('wheelDefaultDisabled', true); | |||
| } | |||
|
|
|||
| const scaleFactor = this.height < this.width ? this.height : this.width; | |||
|
|
|||
| const scaleFactor = 50; | |||
There was a problem hiding this comment.
Do you think we can make this scale factor be relative to the width and height? It's also used to orbit via click and drag, and when the canvas is much larger than 50, it means dragging across the canvas rotates more than one would expect. Here's an example of the current behaviour, and then after a quick change to try to make it something relative to the canvas size:
| // ZOOM if there is a change in mouseWheelDelta | ||
| if (this._mouseWheelDeltaY !== this._pmouseWheelDeltaY) { | ||
| if (this._mouseWheelDeltaY !=0) { |
There was a problem hiding this comment.
Nice, this definitely feels a lot smoother!
| } | ||
|
|
||
| if (this.mouseIsPressed) { | ||
| // ORBIT BEHAVIOR | ||
| if(Math.abs(this.mouseX-this.pmouseX)>50||Math.abs(this.mouseY-this.pmouseY)>50) | ||
| return this;//when noloop() is disabled,need a frame to reset pmouse position |
There was a problem hiding this comment.
Makes sense, I see that if I start off with noLoop() and then call loop() in e.g. mousePressed, the orbit jumps. Maybe we can rephrase the comment a bit to make this clearer, something like:
"Ignore abnormally large mouse movements, which might occur if the draw loop has been paused and just became unpaused"
Changes:
Improve the zooming interaction in webgl mode. Now you can zoom smoothly.