Transform() origin issue

[ionut-gheorghe]

Bug report

Cannot set origin with the transform() method

Also in typescript I cannot use origin like in the documentation

I'm using the 3.0.16 version of the library

[Fuzzyma]

Alright, seems like an error in the d.ts file. Can you provide corrected typings?

[ionut-gheorghe]

Added the folowing to the svg.js.d.ts and works correctly.

Is there a way to get intellisense for origin options?

[Fuzzyma]

Nice! You mean autocompletion for supported strings?

[ionut-gheorghe]

Yep, autocomplete for origin, originX and originY

[Fuzzyma]

Well since originX and Y is only a number you are not getting anything special here. And for the origin case: Any combination of 'topleft', 'top left', 'top-left', 'left_top' or whatever is allowed and therefore we cannot really type it properly. Beside that words, only numbers are possible so nothing else to show here

[luismarcelino]

I also have an issue with Transform() origin: if coordinates are specified as an array [, ] the transformation appears to be fine, however is origin is specified as { x: , y: } results in origin (0, 0).
In this example, the blue and gray boxes should overlap: https://jsfiddle.net/luismarcelino/g2ft9yqb

There is also a problem with the transformation when position is set to y=0 (in the above example, if you try to set the position.y to 0 for the blue rect it will place it as if the origin was not set (but y=0.0001 works fine)

[Fuzzyma]

There is also a problem with the transformation when position is set to y=0 (in the above example, if you try to set the position.y to 0 for the blue rect it will place it as if the origin was not set (but y=0.0001 works fine)

That is most likely because of an unresolved todo in the source lol.

svg.js/src/types/Matrix.js

Lines 70 to 76 in 0d50ae8

	if (isFinite(t.px) || isFinite(t.py)) {
	const origin = new Point(ox, oy).transform(transformer)
	// TODO: Replace t.px with isFinite(t.px)
	const dx = t.px ? t.px - origin.x : 0
	const dy = t.py ? t.py - origin.y : 0
	transformer.translateO(dx, dy)
	}

However, passing an origin of {x, y} should be fine. Have to debug this...

[luismarcelino]

Yes, I think that solves the "==0" issue.
I believe the other problem is in the getOrigin method

svg.js/src/utils/utils.js

Lines 98 to 101 in 0d50ae8

	} else {
	ox = origin[0]
	oy = origin[1]
	}

I was actually preparing a PR with the done TODO and that code changed, but I didn't have the change to try the code properly.

 } else if (origin.x && origin.y) {
    ox = origin.x
    oy = origin.y
  } else {
    ox = origin[0]
    oy = origin[1]
  }

[Fuzzyma]

Wow nice catch! Thats a huge bug. Origin can be passed as ox, oy, originX, originY, [ox, oy], {x: ox, y: oy} and as string. That means that basically only the array, string and ox notation works properly - outch!
However, the getOrigin method is only there to make the support of the string notation work. Every other notation is handled when the object is passed to Matrix.transform(). Therefore, we have to rewrite the code so that the method does nothing when not a string was passed.
But this comes with its own bug. Because 'center' should be assumed when no origin at all was passed. But ox, oy / originX, originY could be set. Only checking if origin is null is not enough. We also have to check if ox and originX have values

So the actual fix could be:

transform.js

  if (!Matrix.isMatrixLike(o)) {
    // Set the origin according to the defined transform
    o = { ...o, ...getOrigin(o, this) }
  }

Matrix.js

export function getOrigin (o, element) {
  const hasOrigin = ox != null || originX != null
  if (hasOrigin) return {}

  const origin = o.origin

  // Allow the user to pass a string to rotate around a given point
  if (typeof origin !== 'string' && origin != null) {
    return {}
  }

  // Get the bounding box of the element with no transformations applied
  const string = (origin || 'center').toLowerCase().trim()
  const { height, width, x, y } = element.bbox()

  // Calculate the transformed x and y coordinates
  const ox = string.includes('left') ? x
    : string.includes('right') ? x + width
    : x + width / 2
  const oy = string.includes('top') ? y
    : string.includes('bottom') ? y + height
    : y + height / 2

  return {origin: [ox, oy]}
}

Maybe its a bit awkward to return an empty object. As an alternative we can do the checks before calling getOrigin and calling it only when it is a string or null and ox and originX are not set

[luismarcelino]

It is nice to have someone controlling the "big picture"! :)
I assumed that getOrigin() would return the origin under all conditions (and I did not explore the Matrix.isMatrixLike()). Maybe getOrigin should be named getOriginFromString! :)

I believe that the first line in the getOrigin() should be (parenthesis just for readability):

  const hasOrigin = (o.ox != null || o.originX != null)

Thanks for looking into the issue and I look forward for your updates.

[Fuzzyma]

Yes ofc you are correct. Looks like I have to set beside some time for fixing all these good issues popping up. Hope I get it fixed soon enough!

[luismarcelino]

Let me know if there is something I can do to help. Thanks

…

On Sat, 29 Feb 2020 at 00:51, Ulrich-Matthias Schäfer < ***@***.***> wrote: Yes ofc you are correct. Looks like I have to set beside some time for fixing all these good issues popping up. Hope I get it fixed soon enough! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1085?email_source=notifications&email_token=AAFNQT5BW4KEJFO6UMCWWGDRFBN2RA5CNFSM4KZBAESKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENKVUJQ#issuecomment-592796198>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFNQT7E6JLO63GF3ACYA5LRFBN2RANCNFSM4KZBAESA> .

[Fuzzyma]

So here is what I came up with:

/**
 * This function adds support for string origins.
 * It searches for an origin in o.origin o.ox and o.originX.
 * This way, origin: {x: 'center', y: 50} can be passed as well as ox: 'center', oy: 50
**/
export function getOrigin (o, element) {
  const origin = o.origin
  // first check if origin is in ox or originX
  let ox = o.ox != null ? o.ox
    : o.originX != null ? o.originX
    : 'center'
  let oy = o.oy != null ? o.oy
    : o.originY != null ? o.originY
    : 'center'

  // then check if origin was used and overwrite in that case
  if (origin != null) {
    [ o.x, o.y ] = Array.isArray(origin) ? origin
      : typeof origin === 'object' ? [ origin.x, origin.y ]
      : [ origin, origin ]
  }

  // make sure to only call bbox when actually needed
  const condX = typeof ox === 'string'
  const condY = typeof oy === 'string'
  if (condX || condY) {
    const { height, width, x, y } = element.bbox()

    // and only overwrite if string was passed for this specific axis
    if (condX) {
      ox = ox.includes('left') ? x
        : ox.includes('right') ? x + width
        : x + width / 2
    }

    if (condY) {
      oy = oy.includes('top') ? y
        : oy.includes('bottom') ? y + height
        : y + height / 2
    }
  }

  // Return the origin as it is if it wasn't a string
  return [ ox, oy ]
}