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

p5.MediaElement does not draw on canvas via image() after calling media.hide() and media.size() #6581

Closed
3 of 17 tasks
davepagurek opened this issue Nov 23, 2023 · 3 comments · Fixed by #6588
Closed
3 of 17 tasks

Comments

@davepagurek
Copy link
Contributor

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build Process
  • Unit Testing
  • Internalization
  • Friendly Errors
  • Other (specify if possible)

p5.js version

1.6.0 to 1.8.0

Web browser and version

Firefox 117

Operating System

MacOS 14

Steps to reproduce this

Steps:

  1. Make a video with createVideo
  2. Hide it with video.hide()
  3. Resize it with video.size(w, h)
  4. Draw it to the canvas with image(video, 0, 0)

This shows nothing on canvas post 1.6.0. Omitting steps 2 or 3 makes it work again (it seems it's the combination of both of them that does it.)

Specifically, these two lines evaluate to 0 when setting the size on the hidden element:

p5.js/src/dom/dom.js

Lines 2267 to 2268 in 28740f9

this.width = this.elt.offsetWidth;
this.height = this.elt.offsetHeight;

Snippet:

let movie;

function preload() {
	// my video is 640 x 360
	movie = createVideo("flyboard.mp4");
    movie.volume(0);
}

function setup() {
	createCanvas(640, 360);
	pixelDensity(1);
  
	movie.play();
    // Commenting this out makes it work
	movie.hide();
}

function draw() {
	background(220);
    // Commenting this out makes it work
    // (or moving it to setup it seems)
    movie.size(width, height);
	image(movie, 0, 0);
}

https://editor.p5js.org/davepagurek/sketches/YwkhJXFtr

@davepagurek
Copy link
Contributor Author

I think the issue was introduced here: 86270da

Previously, if elt.width was 0, due to the || check, it would get overridden with a different value. However, after this commit, it only gets overridden if the value is exactly undefined.

elt.width is 0 because offsetWidth is 0 for any element with display: none, which is what we set when we hide a video element. To address the source of this issue, rather than reverting that commit (relying on the difference between 0 and undefined is maybe something we should try to avoid relying on), I recommend replacing these lines:

p5.js/src/dom/dom.js

Lines 2267 to 2268 in 28740f9

this.width = this.elt.offsetWidth;
this.height = this.elt.offsetHeight;

Instead of using offsetWidth/Height, maybe instead we can record what we set style.width to in a local variable, and then set elt.width to that?

@diyaayay
Copy link
Contributor

Hey,
I would like to work on this issue.
I'll start looking into it if that's okay.
Thanks.

@davepagurek
Copy link
Contributor Author

Thanks @diyaayay! I'll assign it to you.

davepagurek added a commit that referenced this issue Nov 30, 2023
Stores the width and height values before any adjustments are made, Solves #6581
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants