Matrix math and other code cleanups

[sashadev-sky]

Fixes #320 (<=== Add issue number here)
#219

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• PR is descriptively titled 📑 and links the original issue above 🔗
• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt
• code is in uniquely-named feature branch and has no merge conflicts 📁
• screenshots/GIFs are attached 📎 in case of UI updates
• @mention the original creator of the issue in a comment below for help or for a review

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

===============

Changes:

• Bump to v0.6.2 -- pending Bump to v0.6.3 if @jywarren scaling gets merged 1st

• Cleanup / Refactoring:

  • img.editing.enable on load has been moved inside of the DistortableImageOverlay class and removed from both the index.html and DistortableCollection.js (for multi-interface) files.
  • renamed this._handled property on EditHandle and all extending handle classes to this._overlay for consistency across repo and clarity
  • major README updates - realized that corners are not in fact required. still need to hash this part out more

Pending

• If make matrices system work, make wiki. Go through original resources provided in the repo as comments, and transfer and / or improve on them

• These 2 were under the old L.DomUtil.getMatrixString:

  • http://franklinta.com/2014/09/08/computing-css-matrix3d-transforms/
  • https://developer.mozilla.org/en-US/docs/Web/CSS/transform-function#M.C3.B6bius'_homogeneous_coordinates_in_projective_geometry
    ===============

[jywarren]

ooh cool! Bump package.json version too if this is a fix, that'd be great!

[sashadev-sky]

@jywarren if you're satisfied with the bug fix for ff I can change the PR title, merge this, and continue working in new PR - I want to make these changes in increments anyway because i'm working with the most buggy part of our code

[sashadev-sky]

CC @rexagod

[sashadev-sky]

@jywarren also #86 working fine - this is a gif from browserstack chrome 40 windows 7:

[jywarren]

Yes please, incremental would be great.

I believe this may have originally been added due to #42, but this was 4 years ago and Firefox is up to speed now! Should we move forward with this and close #42 on merge? @jywarren (see gif below)

Awesome, sounds good!

So is this then just a general refactoring PR, and the min scale will be a different PR?

[jywarren]

Whoa.... are we able to just include that matrix library?

This is a really big refactor! I kind of think we should do a short-term fix for the scale issue because it's really critical, even if it's just completely removing the limit for now, because if we merge this we could potentially introduce new issues just because of how major it is! What do you think?

[sashadev-sky]

Yes! These methods are from an MDN tutorial - they are easy to follow but some of them are not performance optimized - MDN recommended gl-matrix library. Just trying to get a sense of how everything works 1st.

[jywarren]

Let's try to merge #340 first? Then we can get into some of these refactors. But I think we maybe should not try do to this all at once -- there are too many interdependent changes to easily see what's going on, you know? Sorry Sasha, would it be hard to break this up -- do refactoring/cleaning in one, and the matrix math changes in another? Firefox fix in another?

[sashadev-sky]

@jywarren yes sorry will change the name of this PR and open a different one now to address edgeMinWidth! Still don't have a solution for that but will review your comments on #340

The matrix math is really just refactoring - I changed it so that instead of winding up with a CSS property transform: translate3d(n, n, n) matrix3d(n, n, n, n, n, n, n, n,....) the translate3d is now calculated into the matrix3d. It's my 1st time working with transformation matrices, I don't know if I will spot ways to use them to solve bugs. I can't find how to integrate it with the markers / getCorners & setCorners

[sashadev-sky]

@jywarren let me know your thoughts! Its a little hard to rotate right now but when I get the markers in order it'll be a lot easier. And have to fix the transform-origin situation

[sashadev-sky]

if we were to keep the transform-origin at 0 0 0 there would be no problems with translation or rotate because I rotate around the x-axis. But scale would be stuck to rotating around 1 corner which ruins it

[jywarren]

Hmm, so what do you think the best way forward is? Thanks Sasha!

[sashadev-sky]

@jywarren Not sure! There are a few things to consider -

• What is returned by img.getBoundingClientRect() is not actually the proportions of the image, it always returns the markers in a square shape. Since CSS transforms don't update the latlng of the corners, placing the markers and updating the latlng data gets pretty complicated
• I think this MDN article, explains one of our scale distortions (we should be taking into account the image aspect ratio?)
• keeping track of the bounds and the images actual proportions would allow us to stop overriding _animateZoom - but again got complicated because of the corners

Have u considered switching to a canvas implementation or SVGImageOverlay?

[sashadev-sky]

I just realized @rexagod has been doing a lot of work with matrix math and transformations in matcher-core, as well as Canvas. @rexagod any useful insights here on how to fix the distortions? see my attempted code update and read the comments above. I think perhaps you had already figured out the solution to this a while ago!

Thank you in advance!

[sashadev-sky]

closing this! fixed via #380 and #341 !