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

Added Example on p5.Framebuffer.get method #7166

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Forchapeatl
Copy link
Contributor

@Forchapeatl Forchapeatl commented Aug 11, 2024

Resolves #6750

Changes:

Screenshots of the change:

https://editor.p5js.org/forchapearl1/sketches/QilKPDTvv

PR Checklist

@Qianqianye Qianqianye requested a review from davepagurek August 14, 2024 20:07
* createCanvas(400, 400, WEBGL);
*
* // Create an off-screen WebGL graphics buffer
* let myBuffer = createGraphics(200, 200, WEBGL);
Copy link
Contributor

@davepagurek davepagurek Aug 20, 2024

Choose a reason for hiding this comment

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

Thanks for adding an example here! Right now this would be using the p5.Graphics get method instead of the p5.Framebuffer one. If we instead do this, I think this will be showing an example on the right class:

let myBuffer = createFramebuffer({ width: 200, height: 200 })
myBuffer.draw(() => {
  background(0)
  noStroke()
  // etc
})

myBuffer.loadPixels();
// ...and then the rest from here looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you ! The code has been updated.

* noStroke();
* fill(255, 0, 0); // Set the fill color to red
* push();
* translate(0, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

We maybe don't need the push/translate/pop since the translation is 0 here, but otherwise this is looking good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes! Sorry for all the back-and-forth, I just noticed one more thing, but hopefully that's the last bit!

* // The `pixelColor` is logged to the console, returning red with full opacity.
*
* function setup() {
* createCanvas(400, 400, WEBGL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry one last thing, 400x400 will be rather large for the reference, where examples are typically use 100x100 for pages like https://p5js.org/reference/p5/createFilterShader/. Could we maybe do the same here? There's also nothing wrong with a framebuffer larger than the main canvas, but it may be helpful for illustration purposes to also make myBuffer fit in that size too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries

@perminder-17
Copy link
Contributor

Hi, Great work so far. Thanks for your work @Forchapeatl and also thanks for the review @davepagurek I do have some thoughts on this. Let me know what you feel?

image

The sketch on the reference page looks a bit off. The canvas is 100x100, and we're trying to draw a box of the same size, also it seems slightly shifted or translated. I do have some thoughts on this.

* background(0); // Set the background to black
* noStroke();
* fill(255, 0, 0); // Set the fill color to red
* box(100); // Draw a red box at the center
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make the size of the box to be 50 i.e. box(50) instead of 100

* // Get the color of a pixel at the center of the box (in 2D coordinates)
* myBuffer.loadPixels(); // Load the pixel data for myBuffer
* let pixelColor = myBuffer.get(100, 100); // Get the color at (100, 100)
* console.log('Pixel color at (100, 100):', pixelColor);
Copy link
Contributor

Choose a reason for hiding this comment

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

If you run grunt yui:dev. For looking at the pixel color user needs to open dev tools.
Can we use text() for displaying the pixelColor on the canvas or maybe we could add a line to the description saying (this is what we get in console (in your words) maybe).

* console.log('Pixel color at (100, 100):', pixelColor);
*
* // Display the off-screen buffer on the main canvas
* image(myBuffer, -width / 2, -height / 2); // Draw the buffer on the main canvas
Copy link
Contributor

Choose a reason for hiding this comment

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

we should use -width and -height instead of dividing them with 2.

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.

Add missing examples.
3 participants