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

Update AnimFrame docs with info about when paint happens. #2323

Merged
merged 6 commits into from
Jan 10, 2023

Conversation

xStrom
Copy link
Member

@xStrom xStrom commented Jan 5, 2023

The AnimFrame docs were still from an era before we had partial invalidation. Nowadays an AnimFrame event does not necessarily lead to a paint, because paint is guarded by a check for invalid regions existing.

I also fixed a few rustdoc warnings around doc formating.

@xStrom xStrom added docs concerns documentation S-needs-review waits for review labels Jan 5, 2023
Copy link
Collaborator

@jneem jneem left a comment

Choose a reason for hiding this comment

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

I'll give you the green button because it's clearly an improvement, but I left a comment anyway...

/// Request an animation frame.
/// Request an [`AnimFrame`] event.
///
/// Note that if you call this when handling an [`AnimFrame`] event,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this applies in general, not just when handling an AnimFrame event: request_anim_frame doesn't invalidate anything and so if you're animating then you still need to invalidate. Maybe it's just me, but I find the "request a frame" / "invalidate a rect" distinction more natural than trying to distinguish between "animation frame" and "actual rendered frame"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's definitely general, my writing was just coming from a space where MouseDown never meant paint, but AnimFrame used to mean that (as the comment also said).

I'll see if I can adjust the wording a bit to be more precise about lack of invalidation being the issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

As another note, I actually ran into an issue with an older app of mine that used AnimFrame to measure render fps but the widget itself only painted when the fps changed. Thus the comment's purpose was to make sure it's documented that such tricks no longer work.

@xStrom
Copy link
Member Author

xStrom commented Jan 6, 2023

Alright I rewrote the docs with a more general focus and using common Druid terms like paint instead of talking about rendering which is a Druid internal term.

Does it look better now @jneem?

Copy link
Collaborator

@jneem jneem left a comment

Choose a reason for hiding this comment

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

Yep, that looks clear to me!

@xStrom xStrom removed the S-needs-review waits for review label Jan 10, 2023
@xStrom xStrom merged commit 101dded into linebender:master Jan 10, 2023
@xStrom xStrom deleted the anim_docs branch January 10, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs concerns documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants