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

EaseFunction svg graphs in doc #17461

Merged
merged 5 commits into from
Feb 8, 2025

Conversation

SpecificProtagonist
Copy link
Contributor

@SpecificProtagonist SpecificProtagonist commented Jan 21, 2025

Objective

The docs of EaseFunction don't visualize the different functions, requiring you to check out the Bevy repo and running the easing_function example.

Solution

  • Add tool to generate suitable svg graphs. This only needs to be re-run when adding new ease functions.
  • works with all themes
  • also add missing easing functions to example.

Showcase

Graphs

@SpecificProtagonist SpecificProtagonist added C-Docs An addition or correction to our documentation A-Math Fundamental domain-agnostic mathematical operations S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 21, 2025
@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label Jan 21, 2025
@BenjaminBrienen BenjaminBrienen added the D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes label Jan 21, 2025
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

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

I think this is a good idea. The documentation becomes massively more approachable when it's more than just type definitions; examples, tables, figures, all make it much nicer.

I do have a concern around needing to maintain this tool separately to bevy_math. Perhaps it would be worth trying to refactor this into a proc-macro? Especially considering we're using the doc attribute to just include the SVG string in the documentation anyway.

@SpecificProtagonist
Copy link
Contributor Author

Perhaps it would be worth trying to refactor this into a proc-macro?

I like this idea, my only concern would be whether this would affect build time.

@bushrat011899
Copy link
Contributor

bushrat011899 commented Jan 21, 2025

I like this idea, my only concern would be whether this would affect build time.

Could use cfg_attr(doc, ...) to make the macro only run when building documentation?

@SpecificProtagonist
Copy link
Contributor Author

Will do! I'm assuming the doc would be a feature enabled on docs.rs? Or is there a way to find out if it's getting build by rustdoc?

@bushrat011899
Copy link
Contributor

Will do! I'm assuming the doc would be a feature enabled on docs.rs? Or is there a way to find out if it's getting build by rustdoc?

https://doc.rust-lang.org/rustdoc/advanced-features.html

doc is a built-in config set by rust-doc when it builds documentation. So if you do:

#[cfg_attr(doc, my_special_macro(...))]

It'll only include my_specual_macro when building documentation.

@bushrat011899
Copy link
Contributor

After some experimentation, I don't think it'll be possible to use a proc-macro to generate the graphs. It'd require a cyclic dependency between the macro crate and bevy_math, which is forbidden outside of dev-dependencies.

@SpecificProtagonist SpecificProtagonist added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 7, 2025
@mockersf mockersf enabled auto-merge February 8, 2025 09:38
@mockersf mockersf added this pull request to the merge queue Feb 8, 2025
Merged via the queue into bevyengine:main with commit 7c2d54c Feb 8, 2025
32 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Feb 8, 2025
# Objective

After #17461, the ease function labels in this example are a bit
cramped, especially in the bottom row.

This adjusts the spacing slightly and centers the labels.

## Solution

- The label is now a child of the plot and they are drawn around the
center of the transform
- Plot size and extents are now constants, and this thing has been
banished:
  
  ```rust
  i as f32 * 95.0 - 1280.0 / 2.0 + 25.0,
  -100.0 - ((j as f32 * 250.0) - 300.0),
  0.0,
  ```

- There's room for expansion in another row, so make that easier by
doing the chunking by row
- Other misc tidying of variable names, sprinkled in a few comments,
etc.

## Before

<img width="1280" alt="Screenshot 2025-02-08 at 7 33 14 AM"
src="https://github.com/user-attachments/assets/0b79c619-d295-4ab1-8cd1-d23c862d06c5"
/>

## After

<img width="1280" alt="Screenshot 2025-02-08 at 7 32 45 AM"
src="https://github.com/user-attachments/assets/656ef695-9aa8-42e9-b867-1718294316bd"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Docs An addition or correction to our documentation D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants