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

[KevinWorkman]

Nature of issue?

• Found a bug

Most appropriate sub-area of p5.js?

• Color
• createGraphics()
• set()

Which platform were you using when you encountered this?

• 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?

[Ajayneethikannan]

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!

[ksen0]

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.

[Mamatha1718]

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.

[ksen0]

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)