Skip to content

Make noise generation resolution aware#1909

Merged
TrueDoctor merged 2 commits into
masterfrom
fix-mandelbrot
Aug 9, 2024
Merged

Make noise generation resolution aware#1909
TrueDoctor merged 2 commits into
masterfrom
fix-mandelbrot

Conversation

@TrueDoctor
Copy link
Copy Markdown
Member

@TrueDoctor TrueDoctor commented Aug 7, 2024

Implements footprint based noise generation. I originally intended to also fix the Mandelbrot node in this pr but it turns out that the node itself was never broken and it only stopped working due to an regression somewhere else which has since been fixed.

Changes:

  • Remove dimension parameter
    The dimensions are new inferred from the footprint
  • Add clip parameter
    The user can now choose if they want to constrain the noise

Alternatives considered

  • Make dimension optional
    We could instead make the dimension parameter optional, which would eliminate the need for a transform node after the noise node. This might provide a nicer to use api and could be implemented in a follow-up pr.

We now only render what is visible on the screen and also support clipping to the viewport itself:

image

@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 7, 2024 13:13 Inactive
@Keavon
Copy link
Copy Markdown
Member

Keavon commented Aug 8, 2024

Nice! That's also great to see the Mandelbrot node has gotten fixed already too. My observation is that we'll want to remove the "Clip" parameter so it's always boundless, and always have it be infinite, then use clipping masks or other nodes to actually constrain the bounds as the user desires. We'll also want to inverse the Scale parameter, I think.

@TrueDoctor
Copy link
Copy Markdown
Member Author

TrueDoctor commented Aug 8, 2024

Nice! That's also great to see the Mandelbrot node has gotten fixed already too. My observation is that we'll want to remove the "Clip" parameter so it's always boundless, and always have it be infinite, then use clipping masks or other nodes to actually constrain the bounds as the user desires. We'll also want to inverse the Scale parameter, I think.

Doing this would hurt the performance as you would generate a bunch of useless information which only later gets clipped. This would even evaluate the texture if the artboard is not even in view.

We'll also want to inverse the Scale parameter, I think.

So using 1./scale ? We could instead rename scale to frequency, as we now just pass the value through instead of dividing it by some magic constant

@github-actions github-actions Bot temporarily deployed to graphite-dev (Preview) August 9, 2024 09:42 Inactive
@TrueDoctor TrueDoctor merged commit c5dde18 into master Aug 9, 2024
@TrueDoctor TrueDoctor deleted the fix-mandelbrot branch August 9, 2024 10:03
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.

2 participants