Skip to content

Conversation

Homaid
Copy link

@Homaid Homaid commented Oct 19, 2025

Resolves #7968

Changes:

  • Fixed missing p5 reference when calling loadPixels from p5.MediaElement in ES module contexts.
  • Ensured compatibility with both global and modular usage.

Testing:

  • Verified MediaElement.loadPixels works in both global and ES module imports.

@welcome
Copy link

welcome bot commented Oct 19, 2025

🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors!
🤔 Please ensure that your PR links to an issue, which has been approved for work by a maintainer; otherwise, there might already be someone working on it, or still ongoing discussion about implementation. You are welcome to join the discussion in an Issue if you're not sure!
🌸 Once your PR is merged, be sure to add yourself to the list of contributors on the readme page !

Thank You!

@Homaid
Copy link
Author

Homaid commented Oct 19, 2025

Hii @ksen0 @perminder-17

This PR fixes the p5 reference error in p5.MediaElement.loadPixels (#7968).
Could you please review when you have a moment? Thank you!

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 @Homaid , I think the tests aren't passing, can you please have a look once?

@Homaid
Copy link
Author

Homaid commented Oct 19, 2025

hey @perminder-17 Linting is passing, but the tests are still failing locally I’m investigating that part.

@Homaid Homaid force-pushed the fix/media-element-loadpixels branch from f91f1b2 to c4dcb8f Compare October 22, 2025 07:24
@Homaid
Copy link
Author

Homaid commented Oct 22, 2025

hey @perminder-17 i have updated the pr and fixed the linting issues can you recheck thank you

@Homaid
Copy link
Author

Homaid commented Oct 22, 2025

and about the issue it was unrelated test failures (should work with tint() and should work with updatePixels) occur in the p5.prototype.createVideo suite.
All other tests and linting pass.

@Homaid Homaid requested a review from perminder-17 October 22, 2025 07:33
renderer = this._pInst._renderer;
} else if (typeof p5 !== 'undefined' && p5.instance && p5.instance._renderer) {
// Fallback for global mode
renderer = p5.instance._renderer;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in a WebGL main canvas, loadPixels will exist but won't work on a MediaElement; we need the 2D renderer version specifically. I had to deal with a similar issue in #8176 where the class was referencing a global p5, and was updated to use p5 from an addon function instead where it's explicitly passed in, maybe we can try that same approach here too?

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]: p5 reference error for p5.MediaElement

4 participants