Make rotations/angles consistent

[matthew-e-brown]

See #274 for details—I'm only a little bit sure this is actually a fix.
Closes #274.

Here's the MWE I mentioned, demonstrating that angles are still working as intended. The label on the blue line is spun around to demonstrate that its anchors and rotation follow along as they should.

(generated with Typst v0.8 and CetZ at feb07cc)

#import "/src/lib.typ" as cetz

#set page(width: auto, height: auto, margin: 0.5in)

#let angle2-new(a, b) = cetz.vector.angle2(a, b)
#let angle2-old(a, b) = calc.atan2(a.at(1) - b.at(1), a.at(0) - b.at(0)) + 90deg

#let test(a, a-label: $$, line-label-side: "bottom") = cetz.canvas(length: 1.75cm, {
  import cetz.draw: *
  import cetz.vector

  let o = (0, 0)

  // Grid and unit circle
  grid((-2, -2), (2, 2), step: 0.5, stroke: 0.5pt + silver)
  line((-2, 0), (2, 0), stroke: 0.25pt + black)
  line((0, -2), (0, 2), stroke: 0.25pt + black)
  circle((0, 0), radius: 1, stroke: 0.75pt + black)
  line((0, 0), (1, 0), stroke: 1pt + black) // slightly thicker

  // Vector and the two methods of drawing the lines between them
  line(o, a, stroke: (paint: blue, cap: "round"))
  arc(o, anchor: "origin", radius: 0.30, start: 0deg, stop: angle2-new(o, a), name: "arc-line")
  cetz.angle.angle(o, (1, 0), a, stroke: (dash: "dotted"), radius: 0.70, name: "angle-line")

  // Intersection dots
  set-style(circle: (radius: 0.04, fill: black, stroke: none))
  circle(o)
  circle(a)

  // Labels on point and line
  content((o, 1.20, a), angle: angle2-new(o, a), padding: 0.08, $alpha$)
  content(
    (o, 0.5, a), angle: angle2-new(o, a),
    padding: 0.11, $arrow(alpha)$, anchor: line-label-side, name: "line-label"
  )

  // Legends
  set-style(content: (frame: "rect", padding: 0.125, fill: white, stroke: 0.5pt + silver))
  content((-2, 2), padding: 0.15, anchor: "top-left", $ alpha = #a-label $)
  content((2, -2), anchor: "bottom-right", $
    #raw("angle2-new")lr(( (0,0) thin,thin alpha )) &= #angle2-new(o, a)\
    #raw("angle2-old")lr(( (0,0) thin,thin alpha )) &= #angle2-old(o, a)
  $)

  // Draw all anchors
  set-style(circle: (radius: 0.03, stroke: 0.5pt + black))

  circle("line-label.top", fill: red)
  circle("line-label.right", fill: yellow)
  circle("line-label.bottom", fill: green)
  circle("line-label.left", fill: blue)

  circle("arc-line.start", fill: purple)
  circle("arc-line.end", fill: fuchsia)

  circle("angle-line.start", fill: purple)
  circle("angle-line.end", fill: fuchsia)
})

#align(center, grid(
  columns: 2,
  gutter: 1em,
  test(( 1/2,  calc.sqrt(3)/2), a-label: $ ( 1/2 thin,thin  sqrt(3)/2) $),
  test((  -1,               0), a-label: $ (  -1 thin,thin          0) $),
  test((-1/2, -calc.sqrt(3)/2), a-label: $ (-1/2 thin,thin -sqrt(3)/2) $,  line-label-side: "top"),
  test(( 1/2, -calc.sqrt(3)/2), a-label: $ ( 1/2 thin,thin -sqrt(3)/2) $,  line-label-side: "top"),
))

#let dot(color) = box(circle(radius: 0.25em, fill: color, stroke: 0.5pt + black))
#let my-line(dash) = box(baseline: -0.25em, line(length: 1.25em, stroke: (dash: dash)))
#set terms(separator: [~=~])

#stack(dir: ltr, spacing: 2em, [
  / #dot(red): Content's `top`
  / #dot(yellow): Content's `right`
  / #dot(green): Content's `bottom`
  / #dot(blue): Content's `left`
  / #dot(purple): Line's `start`
  / #dot(fuchsia): Line's `end`
  / #my-line("solid"): Drawn using `draw.arc`
  / #my-line("dotted"): Drawn using `angle.angle`
], [
  #show par: set block(spacing: 2em)

  Content inside of Typst's `rotate` function at $20 degree$\
  (and therefore also CetZ's `content`'s previous behaviour):
  #align(center, rotate(20deg, [Hello!]))

  CetZ's updated `content` function with an angle of $20 degree$:
  #align(center, cetz.canvas({ cetz.draw.content((), angle: 20deg, [Hello!]) }))
])

[fenjalien]

Thanks for the fixes! @johannes-wolf can this be merged into redesign-internals before the arc-through PR?

@matthew-e-brown Do you mind adding fixes to make sure all the rotation stuff goes in the same direction? like rotate and angle coordinates :)

[johannes-wolf]

Yes, this can go into redesign-internals.

[matthew-e-brown]

Do you mind adding fixes to make sure all the rotation stuff goes in the same direction? like rotate and angle coordinates :)

Sure 🙂 I will take a look at the other styles of rotation as soon as I finish up the homework that this distracted me from 😆 Hopefully later tonight or tomorrow morning.

[matthew-e-brown]

@johannes-wolf Since my branch is based off of your cetz-206-arc-through already, it's already got 2ed733a in it. Merging this into redesign-internals would just supercede #273, wouldn't it? Is that alright?

[fenjalien]

Sure 🙂 I will take a look at the other styles of rotation as soon as I finish up the homework that this distracted me from 😆 Hopefully later tonight or tomorrow morning.

Thanks and good luck!

Since my branch is based off of your cetz-206-arc-through already, it's already got 2ed733a in it. Merging this into redesign-internals would just supercede #273, wouldn't it? Is that alright?

No as we'll squash commit so the arc-through commit would get lost. Either #273 gets merged first or this pr rebases onto the current head of redesign-internals.

[johannes-wolf]

@matthew-e-brown, your example picture looks very nice btw. 👍

[johannes-wolf]

Can this get merged @matthew-e-brown?

[matthew-e-brown]

Hi @johannes-wolf sorry for disappearing. I said I'd finish up my schoolwork and then get to this PR, but my university had other plans. I've been beyond busy for the last few weeks. 🫠🪦

If I recall correctly I still had yet to look into the rotation matrices before un-drafting it. I believe it was finding a problem with the matrices, even after making my other tweaks, that made me decide to open that big issue—so I think they were definitely on my checklist. Of course I also wanted to do a full scan through and look at all the other places where rotations were used, and make sure nothing was broken and that all their angles were correct. Finally I was hoping to add a test or two if needed.

That said, I believe the commits that are already here are totally fine. So this could be merged if you wanted to get it in for the next version, but I'm pretty sure (again, it's been a while) there are a few more tweaks to make. At the very least, there's a few double checks to do.

I would love to be the one to finish up this PR because I'd be proud to contribute, but I'd hate for it to hold things up. So if someone else wants to take over this fix to get it across the finish line, I'd be cool with that. If not, I promise I haven't forgotten about it! 😅 I'd love to get back to it eventually. I'll be back into using Typst for another assignment soon, so I may be able to find an hour or two to poke around in CetZ sometime this weekend... but that is still a ways away.

Let me know what you'd like to do. Sorry again!! 🍁

[johannes-wolf]

I'll wait, take your time. :)

[johannes-wolf]

I discovered that the rotation matrix for xyz euler angles was wrong, so I fixed it and also changed the rotate(..) function to use CCW angles. See #324.

[matthew-e-brown]

Hi @johannes-wolf, I see there's been quite a lot of work done since I last looked into this. Notably, the redesign-internals PR is gone (did that become the 0.2.0 branch?), and #324 seems to add a general transformation function.

I feel like it would make sense to rebase this to a more up-to-date starting point, to avoid double-checking and fixing things that have already been reworked (e.g. all the matrices you guys've been working on). Or do you think it still makes sense to merge it into 206-arc-through? Let me know how you think I should proceed. I wouldn't be surprised if the interals-redesign process has already covered all the things in this PR! 😄

[johannes-wolf]

Hi @matthew-e-brown, yes, the redesign-internals branch got renamed to 0.2.0. I think you should rebase on that branch, and we merge arc-through after merging this PR. I think we should merge #324 first (done).

[johannes-wolf]

Ok, I suggest rebasing this onto 0.2.0 and merging it. I think all other rotation problems are fixed, this is the only things missing having rotation be CCW everywhere.

[matthew-e-brown]

Alright, rebased onto 0.2.0.

Hopefully removing the *-1's in fb54cf2 doesn't break anything from v0.2.0... it didn't break anything back in October, but there was a 73-commit difference between the branches! 😄

[johannes-wolf]

Alright, rebased onto 0.2.0.

Hopefully removing the *-1's in fb54cf2 doesn't break anything from v0.2.0... it didn't break anything back in October, but there was a 73-commit difference between the branches! 😄

Can you drop the arc-through commit from this PR please? The CI tests should fail. Are you able to commit new test images, or should I do that (bash: just update-test)?

[matthew-e-brown]

Oh, right... Not gonna lie, it totally slipped my mind that I could do that with a rebase 🤣

[matthew-e-brown]

@johannes-wolf I can commit new test images if you'd like, but there's a small problem... The rotations being "correct" now (i.e., reversed; always CCW), means that tests that had manually-rotated things have flipped labels now. Mostly, that's the charts:

Other things are also flipped, but those're mostly inconsequential:

Should I go in an flip the labels' angles in the charts before committing?

[johannes-wolf]

That would be nice, yes :)

[matthew-e-brown]

Alright, ran the script. I checked the images that differed, and they all seem to be free of major visual changes.

Only thing is that running it on Typst v0.9.0 means that some of the U+002D HYPHEN - in axis labels became U+2122 MINUS −, which makes a few tick markers a tiny bit wider. Hopefully that's okay. I'd have to roll-back my Typst install to undo that; but, the reference images are bound to update to v0.9.0 eventually anyways... 😁

[matthew-e-brown]

Ah, whoops... I updated Typst to 0.9.0 on my laptop and re-generated the images there—but then this morning I fixed the labls on my desktop and re-ran it... which I have not yet updated to 0.9.0.

One sec!!

[matthew-e-brown]

Much better. Now the only tests with changes are the content and content/rotate. That makes more sense. 🙂

[johannes-wolf]

Merged :) Thank you!