fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
28.92k stars 3.5k forks source link

"Wobble" in angle during rotation possibly from qrDecompose() #6941

Open bskrypnyk opened 3 years ago

bskrypnyk commented 3 years ago

Version

4.3.1

Test Case

Docs: http://fabricjs.com/using-transformations JSFiddle: https://jsfiddle.net/tourist604/uzv286h4/25/

Information about environment

First off --- thank you for the great library! This was tested in latest Chrome on a Mac. There is a pretty noticeable 'wobble' occurring during rotation when a qrDecompose()'ed props are applied to a minion object.

Steps to reproduce

Using the page in your documentation: http://fabricjs.com/using-transformations it is easy to reproduce the issue. It becomes even more noticeable when the rotated object is an image.

Expected Behavior

Smoother, better fidelity transformation.

Actual Behavior

Currently, debugging through console messages, it is possible to see that while the "boss" object is being rotated in a consistently smooth way, the angle calculated for the "minion" jumps around a bit. See attached screenshots.

wobble wobble-console

asturur commented 3 years ago

i wonder if the example is wrong or what Someone already pointed out in another issue, but we never figured out what is it.

bskrypnyk commented 3 years ago

Making the boss rotate programmatically seems to eliminate the wobble:

https://jsfiddle.net/tourist604/uzv286h4/111/

Perhaps, this is the result of how the mouse movement is interpreted into rotation relative to the box controls?

ShaMan123 commented 2 years ago

Great! #7614 I have tests failing because of this

ShaMan123 commented 2 years ago

I bet it's invertTransform that calls transfromPoint that ignores offset.

asturur commented 2 years ago

imho the wobbling was something related some code happening on frame x and other code with the data of frame x+1

ShaMan123 commented 2 years ago

hmmm interesting. I found fabric.util.calcDimensionsMatrix to be the cause or at least to show the issue.

ShaMan123 commented 2 years ago

or composeMatrix

ShaMan123 commented 2 years ago

I have managed to create a failing test a27e2d2f

ShaMan123 commented 2 years ago

look, the test doesn't test precision: https://github.com/fabricjs/fabric.js/blob/38cfcda72507351a9c0974aff1703899e7de831e/test/unit/util.js#L913

asturur commented 2 years ago

no tests can't test precision, because precision is ephemeral with float in JS. different browsers and node give different results, and sometimes browsers running on different OS do too.

If i have to point the finger at precision, i would blame our simplified cos/sin functions. But i do not understand why the big square does bobble and other do no. As @bskrypnyk pointed out, if you do it by code, it does not wobble.

To be clear @ShaMan123 i m not saying you may not be right, i just have this memory of this old issue and me having a clarifying moment with the order in which event are processed, but i forgot it.

ShaMan123 commented 2 years ago

did you look at the new test I've added? a27e2d2 To my understanding it is agnostic to the system it runs in

asturur commented 2 years ago

It could be on the surface, but probably it isn't. I think it depends from which Math library the javascript engine links in. You can have tests failing in node and not in browser or failing in firefox and not in chrome, even just with matrices code. At leat it happened to me and that is why some tests have the toFixed() hack

I would expect m1 to work and m/m2 to fail because for sure have some sort of skewing. The failure is small tho, well beyond 6 decimals, so we talk about 1/1000000 of pixel (very rough estimate), not that large wobbly we see in this case.

ShaMan123 commented 1 year ago

Please check this with #8767