p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

Should P5.Graphics support set() with another P5.Graphics?

Open KevinWorkman opened this issue 7 years ago • 3 comments

Nature of issue?

  • [X] Found a bug

Most appropriate sub-area of p5.js?

  • [X] Color
  • createGraphics()
  • set()

Which platform were you using when you encountered this?

  • [X] Desktop/Laptop

Details about the bug:

  • p5.js version: 0.6.1 (using the alpha web editor)
  • Web browser and version: Chrome 67.0.3396.62
  • Operating System: Linux
  • Steps to reproduce this:

From this Stack Overflow question.

It appears that the set() function does not work with P5.Graphics values. Is this expected?

let redRect;

function setup() {
  createCanvas(100,100);
  redRect = createGraphics(100,100);
  redRect.fill(255, 0, 0);
  redRect.rect(20,20,40,40);
	
  const blueRect = createGraphics(20, 20);
  blueRect.background(0, 0, 255);
  redRect.set(30, 30, blueRect);
  redRect.updatePixels();
}

function draw() {
  background(0, 255, 0);
  image(redRect,0,0);
}

I would expect this to create a graphics that contains a red rectangle, and then draw a blue rectangle to that first graphics. Instead, only a single pixel is set, and it's set to transparent?

red rectangle on green background

KevinWorkman avatar Jun 08 '18 17:06 KevinWorkman

I think that this feature should be present . The reason why it is setting a single transparent pixel is the current set method checks whether the third parameter imgOrCol is an p5.Image instance or a color value. In case it is neither if them, it sets the value of the pixel mentioned as [0, 0, 0, 0]. We can check its implementation here .

We can check if the imgOrCol parameter is p5.Image instance or p5.Graphics instance, then we can set the pixels of one p5.Graphics instance from another.

Also , if we pass a image as the third parameter set(x, y, image) the image fills the entire canvas from x, y onwards. Is this the intended behavior ?
Should we pass the height and width here as

this.drawingContext.drawImage(imgOrCol.canvas, x, y, imgOrCol.width, imgOrCol.height);

Would love your suggestions , Thank you!

Ajayneethikannan avatar Mar 16 '19 17:03 Ajayneethikannan

This is still the behavior on 1.11 as well as in 2.0-beta.8, so this is open for work. To move forward, this issue needs an implementation proposal and volunteer!

If you're new to p5.js contribution, welcome! Please be sure to read through the contributor guidelines, and keep in mind that you should not file a pull request (or start working on code changes) without a corresponding issue or before an issue has been approved for implementation and assigned. So please comment here not only to be assigned, but also first explaining a bit how you would approach this problem.

ksen0 avatar Apr 17 '25 11:04 ksen0

Hi @ksen0 , @KevinWorkman , @Ajayneethikannan , @outofambit , @Qianqianye , @limzykenneth , I'd like to work on this issue. I checked the original Stack Overflow post that led to this bug report, and the issue is clear — users expect to be able to use set(x, y, graphics) just like set(x, y, image), but right now it only works with p5.Image.

My plan is to update the set() function so that if the third argument is a p5.Graphics object, we convert it to a p5.Image using .get():

if (imgOrCol instanceof p5.Graphics) { imgOrCol = imgOrCol.get(); } This way, the existing logic (which handles images correctly) can take over and it will work as users expect.

Will you please let me know if this approach is okay, or not. Thanks in Advance.

Mamatha1718 avatar Apr 25 '25 12:04 Mamatha1718

Hi @Mamatha1718 , apologies that I didn't see this notification earlier. Please let me know if you are still interested and available to work on this. Thank you!

(If you do work on this, please start with the dev-2.0 branch)

ksen0 avatar Jun 23 '25 16:06 ksen0