Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tutorial: Big Ben #78

Merged
merged 22 commits into from
Jul 7, 2022
Merged

Tutorial: Big Ben #78

merged 22 commits into from
Jul 7, 2022

Conversation

srush
Copy link
Collaborator

@srush srush commented Jul 3, 2022

Kind of an extended tutorial demo to replicate a diagram of the Big Ben clockface. Would love thoughts or comments for simplifying further.

https://srush.github.io/pydiagrams/examples/bigben/

image

Here's what it really looks like:

image

This is not ready to check in yet, as there are some major breaking changes. These include:

  • Switching everything user facing to degrees from radians.
  • Changing the way the show_* functions work.
  • Monkey patching geometry so it works in the OO style.
  • Fixing some bugs with arcs and circles.
  • Making line_width not scale in SVG (vector-effect style)

Sorry these probably should be different PRs, but most of them came up in the process of making the demo.

@danoneata
Copy link
Collaborator

This is beautiful! Looking forward to taking a closer look at the example!


def get_envelope(self) -> Envelope:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed in the Big Ben tutorial that the under_arc curve is chopped off when rendered. On closer inspection, it seems that there is a slight issue with the envelope. An arc can, in principle, be specified by negative angles:

 arc(radius=1, angle0=-50, angle1=50)

but in the wrapped function the direction d takes only positive angles; so if a direction's angle is in the [310, ..., 360] range then the if condition enters through the second branch ("point outside of arc"). For the above example, the envelope looks like this (see the upper half of the envelope, which corresponds to the [-50, ..., 0] range):
o

@srush srush changed the title [WIP] Tutorial: Big Ben Tutorial: Big Ben Jul 5, 2022
@srush
Copy link
Collaborator Author

srush commented Jul 5, 2022

Okay, think this roughly works now, however I realize there are some big questions that I may have broken.

  • I made line-thickness scale invariant for SVG. As a user, I think you really need this for scaling compositionality to work. But it is counterintuitive that you can't really scale your diagrams after making them. Other backends probably need this as well. Maybe it should be optional?

  • There are some inconsistencies with angles. I can't decide if they should go clockwise or counterclockwise, i.e. this seems bad
    image

@srush
Copy link
Collaborator Author

srush commented Jul 5, 2022

Oh, as always the Diagrams manual answers my questions.

@danoneata
Copy link
Collaborator

danoneata commented Jul 5, 2022

I've made a small change to the computation of the arc's envelope. I hope I didn't break anything and it works fine now—is this the behaviour you would expect?

arc(radius=1, angle0=-45, angle1=90)

o

arc(radius=1, angle0=-45, angle1=360)

o

There are some inconsistencies with angles. I can't decide if they should go clockwise or counterclockwise, i.e. this seems bad

Yes, I have to fix this for arcs as well: currently, negative angles are in the top half and the arc is drawn clockwise...

If we go with the counterclockwise convention, maybe we should have unit_y = Vec2(0, -1), such that, unit_x.rotate(90) == unit_y?

Edit: I think that swapping the angles and negating them draws the arc with the same convention as rotate (counterclockwise and starting from the right). I don't know if this breaks anything else.

diff --git a/chalk/shape.py b/chalk/shape.py
index 62bfb30..6dbc980 100644
--- a/chalk/shape.py
+++ b/chalk/shape.py
@@ -255,6 +255,7 @@ class Arc(Shape):
     angle1: float

     def __post_init__(self) -> None:
+        self.angle0, self.angle1 = -self.angle1, -self.angle0
         surface = cairo.SVGSurface("undefined.svg", 1280, 200)
         self.ctx = cairo.Context(surface)

@danoneata
Copy link
Collaborator

I've tried to make the "decal" corners appear a bit tighter by fixing the envelope for arcs and some additional tweaking:
o
And here is how it looks in the final rendering:
Screenshot 2022-07-06 at 17 57 09

@srush
Copy link
Collaborator Author

srush commented Jul 6, 2022

Thanks. this is great, that was bugging me. (I am pushing a couple other tweaks I made as well, realize the colors are off in your version).

Another related thing is that it would be nice if Path and maybe Trail could have arcs in it. Wanted to have the hands use an arc_between, but couldn't fill.

@danoneata
Copy link
Collaborator

Another related thing is that it would be nice if Path and maybe Trail could have arcs in it. Wanted to have the hands use an arc_between, but couldn't fill.

Yes, that's a good point! I've added an issue as a reminder. It might be also a good opportunity to allow for more flexible curves, such as Bézier curves or splines (as you suggested in #35).

@srush
Copy link
Collaborator Author

srush commented Jul 7, 2022

Gonna merge this in. Seems like it now fixes more than it breaks. Made issues for the remaining problems.

@srush srush merged commit 5e268ff into master Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants