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

decoration: Allow dynamic amplitudes #756

Merged
merged 4 commits into from
Dec 6, 2024

Conversation

johannes-wolf
Copy link
Member

@johannes-wolf johannes-wolf commented Nov 24, 2024

Allows passing a function of the form ratio -> float or an array of floats as amplitude: to path decorations.

Adds a square square-wave decoration.

@johannes-wolf johannes-wolf force-pushed the decorations-dynamic-amplitude branch from 3031c13 to 0c5a842 Compare November 24, 2024 18:39
@Jollywatt
Copy link
Contributor

Jollywatt commented Nov 24, 2024

One more nitpick:

  //   ╭ ma ╮        ▲
  //   │    │        │ Up
  // ..a....m....b.. '
  //        │    │
  //        ╰ mb ╯

Since the wave points ma and mb are at different positions along the curve, you could evaluate the amplitude at both points independently.
This leads to slightly nicer results such as:
Screenshot 2024-11-24 at 18 44 42
compared to what you currently get:
Screenshot 2024-11-24 at 18 45 04

Both of these are made with amplitude: t => calc.min(float(t), 1 - float(t)).
To make the first image, wherever you do

let fn(i, a, b, norm) = {
    let ab = vector.sub(b, a)
    let up = vector.scale(norm, resolve-amplitude(style.amplitude, i, num-segments) / 2)
    let down = vector.scale(
      up, -1)

I replaced it with

let fn(i, a, b, norm) = {
    let ab = vector.sub(b, a)
    let up = vector.scale(norm, resolve-amplitude(style.amplitude, i + 0.25, num-segments) / +2)
    let down = vector.scale(norm, resolve-amplitude(style.amplitude, i + 0.75, num-segments) / -2)

and reverted my earlier suggestion of changing (amplitude)(segment / num-segments * 100%).

This just means there's twice as many amplitude evaluations, but may lead to better results for large segment lengths… what do you think?

@johannes-wolf johannes-wolf force-pushed the decorations-dynamic-amplitude branch from f34eda7 to a9cb203 Compare November 24, 2024 20:40
@@ -59,6 +63,16 @@
factor: 150%,
)

#let resolve-amplitude(amplitude, segment, num-segments) = {
return if type(amplitude) == function {
(amplitude)(segment / (num-segments - 1) * 100%)
Copy link
Contributor

@Jollywatt Jollywatt Nov 24, 2024

Choose a reason for hiding this comment

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

This now needs to be reverted back to (amplitude)(segment / num-segments * 100%), since segment now ranges from 0.25 to num-segments + 0.75 😅

You can see what I mean with the test case

wave(line((0,3), (4,3)), amplitude: t => { calc.sin(t/100%*calc.pi) })

Copy link
Member Author

@johannes-wolf johannes-wolf Nov 24, 2024

Choose a reason for hiding this comment

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

Yeah, I am playing around with it. With the .25 addition, you won't get 0%/100% in the callback, which is a bit unexpected, I think.

Copy link
Contributor

@Jollywatt Jollywatt Nov 24, 2024

Choose a reason for hiding this comment

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

I guess the segment variable is no longer an index, but rather the coordinate along the curve 0 <= t <= num-segments

@johannes-wolf johannes-wolf force-pushed the decorations-dynamic-amplitude branch from 4ef8457 to bdfc993 Compare November 24, 2024 21:24
return if type(amplitude) == function {
(amplitude)(segment / num-segments * 100%)
} else if type(amplitude) == array {
amplitude.at(calc.rem(int(segment), amplitude.len()), default: 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered amplitude.at(calc.rem(int(2*segment), amplitude.len()), default: 0)? That gives the user control over individual peaks and troughs. With int(segment), you can only control them in full-wavelength pairs.

Copy link
Contributor

@Jollywatt Jollywatt left a comment

Choose a reason for hiding this comment

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

I just had one possible suggestion:

Have you considered amplitude.at(calc.rem(int(2*segment), amplitude.len()), default: 0)? That gives the user control over individual peaks and troughs. With int(segment), you can only control them in full-wavelength pairs.

@johannes-wolf
Copy link
Member Author

I just had one possible suggestion:

Have you considered amplitude.at(calc.rem(int(2*segment), amplitude.len()), default: 0)? That gives the user control over individual peaks and troughs. With int(segment), you can only control them in full-wavelength pairs.

Will look into that.

@johannes-wolf johannes-wolf force-pushed the decorations-dynamic-amplitude branch from bdfc993 to 2233dc0 Compare December 6, 2024 01:18
Copy link
Contributor

@Jollywatt Jollywatt left a comment

Choose a reason for hiding this comment

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

Perfect:)
I think length amplitudes should also be allowed, but I'm happy to make that a separate PR. It's just a matter of adding util.resolve-number(ctx, amplitude) and adding some tests.

@johannes-wolf johannes-wolf merged commit ae0ebfb into master Dec 6, 2024
1 check passed
@johannes-wolf johannes-wolf deleted the decorations-dynamic-amplitude branch December 6, 2024 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎁 Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants