Skip to content

Conversation

ayushman1210
Copy link

Resolves #8123

Changes:

This PR fixes the black frames bug in saveGif() by removing an incorrect frameCount reset.

Problem:
saveGif() was generating GIFs with black or incorrectly rendered frames at the start because it reset the global frameCount variable on line 512, breaking animations that depend on frameCount for timing.

Changes made:

  1. Line 512 in src/image/loading_displaying.js: Removed this.frameCount = frameIterator;

    • The internal frameIterator variable is sufficient for tracking capture progress
    • No need to modify the global frameCount that user sketches depend on
  2. After line 542: Added initial frame rendering with this.redraw() and 16ms delay

    • Ensures canvas is fully initialized before capture begins
    • Prevents initialization-related black frames

Impact:

  • Minimal change - only 2 lines modified
  • No breaking changes to API
  • Preserves all existing functionality
  • Fixes animations that rely on frameCount for timing

PR Checklist

@perminder-17 perminder-17 changed the base branch from main to dev-2.0 October 4, 2025 09:57
@perminder-17 perminder-17 changed the base branch from dev-2.0 to main October 4, 2025 09:57
Copy link
Collaborator

@perminder-17 perminder-17 left a comment

Choose a reason for hiding this comment

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

Minimal change - only 2 lines modified

The diff shows more than a thousand line changes. Are these documentation removal was intentional? Can you please fix this?

// initialize variables for the frames processing
let frameIterator = nFramesDelay;
this.frameCount = frameIterator;
// BUGFIX: Removed line that was resetting frameCount
Copy link
Contributor

Choose a reason for hiding this comment

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

We actually intentionally set the frame count here so that sketches relying on the frame count are effectively reset when recording the gif. (An extension we haven't got to yet is to also override millis() when recording too.) Fully removing it would break the use case where you want to get a recording from the start of the animation.

Maybe there's a way the user can pass an option into saveGif to tell it to pick up recording from whatever the current state of the sketch is, and we don't touch frameCount (and in the future, other timing state) when that's set?

@ayushman1210
Copy link
Author

@perminder-17 Hi maintainer !!
Thanks for reviewing my PR that was not intentional. My change is a very small, targeted edit to [loading_displaying.js] . I will fix this as soon as possible

@ayushman1210
Copy link
Author

@perminder-17 Hi maintainer !!
can you please review the changes

Copy link
Collaborator

@perminder-17 perminder-17 left a comment

Choose a reason for hiding this comment

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

Hi, Sorry for the delay @ayushman1210 , I just read the issue now. The issue clearly says that it happens in 2.x versions.

Set p5.js version to 2.0 or higher

Your PR targets the main branch which is only for 1.x versions, to fix the issue you need to open up a PR targeting dev-2.0 branch. You can close this PR and open up a new one which targets dev-2.0 branch. Other option is, you can change the base from above and try rebasing it.

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.

[p5.js 2.0 Bug Report]: saveGif generates black frames at start of gif
3 participants