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

Support for 3D coordinate on Image #7172

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4326a02
Support for 3D coordinate on Image
Forchapeatl Aug 14, 2024
97efe53
Added function to support 3D image coordinates and texture
Forchapeatl Aug 14, 2024
c2d6142
fixed 2D breakage
Forchapeatl Aug 16, 2024
168e0e3
commit changes
Forchapeatl Aug 16, 2024
aab2120
catch framebuffer or graphic instance
Forchapeatl Aug 19, 2024
7942b73
added scalling
Forchapeatl Aug 20, 2024
2a80879
using vertex for texture mapping instead of plane
Forchapeatl Aug 22, 2024
96997fd
defaulting to 0 in webgl context
Forchapeatl Aug 22, 2024
ca19e53
Added examples and dz param introduction
Forchapeatl Aug 22, 2024
1c76a8d
removed examples
Forchapeatl Aug 22, 2024
6e31086
removed dz param introduction
Forchapeatl Aug 22, 2024
247e855
Added dz Param and examples
Forchapeatl Aug 23, 2024
8d81b73
dz parameter on Contain
Forchapeatl Aug 23, 2024
7ae6f8b
replaced WEBGL check with dz check
Forchapeatl Aug 25, 2024
ab32870
removed Image3D function
Forchapeatl Aug 26, 2024
ed1796a
added support for Z coordiante
Forchapeatl Aug 26, 2024
e764cb1
fixed syntax error
Forchapeatl Aug 26, 2024
038a5ad
fixed syntax error
Forchapeatl Aug 26, 2024
71027b0
fixed lint
Forchapeatl Aug 26, 2024
07beef9
Added support for webgl mode
Forchapeatl Sep 7, 2024
1b53827
dz as last parameter
Forchapeatl Sep 8, 2024
af0b2e8
dz as last parameter on imade2D
Forchapeatl Sep 8, 2024
9e19785
Update p5.Renderer2D.js
Forchapeatl Sep 8, 2024
80844dc
revert changes
Forchapeatl Sep 8, 2024
15d0015
revert changes
Forchapeatl Sep 8, 2024
f0f57cc
removed dz from param docs
Forchapeatl Sep 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 153 additions & 12 deletions src/image/loading_displaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,108 @@ function _sAssign(sVal, iVal) {
* }
* </code>
* </div>
* <div>
* <code>
* let img;
*
* function preload() {
* img = loadImage('assets/moonwalk.jpg');
* }
*
* function setup() {
* // Create a 3D canvas
* background(200);
* createCanvas(400, 400, WEBGL);
* }
* function draw() {
* image(img, 0, 0, -100);
* describe('An image at the center 100 units away from the camera');
* }
* </code>
* </div>
*
* <div>
* <code>
* let img;
* function preload() {
* img = loadImage('assets/moonwalk.jpg');
* }
*
* function setup() {
* // Create a 3D canvas
* createCanvas(400, 400, WEBGL);
* }
*
* function draw() {
* background(200);
* image(img, 0, 0, 400 , 300 , 300);
* describe('Scale image 300 by 300 and zoomin 400 units');
* }
* </code>
* </div>
*
* <div>
* <code>
* let img;
* function preload() {
* img = loadImage('assets/moonwalk.jpg');
* }
*
* function setup() {
* // Create a 3D canvas
* createCanvas(400, 400, WEBGL);
* }
*
* function draw() {
* background(200);
* image(img, 0, 0, -400 , 300 , 300);
* describe('Scale image 300 by 300 and zoomout 400 units');
* }
* </code>
* </div>
*
* <div>
* <code>
* let img;
* function preload() {
* img = loadImage('assets/moonwalk.jpg');
* }
*
* function setup() {
* // Create a 3D canvas
* createCanvas(400, 400, WEBGL);
* }
*
* function draw() {
* background(200);
* image(img, 0, 0, 0, 400, 400,500, 100, 200, 200);
* describe('Draw a subsection of the image from 500 by 100 units position at the center with size 400x400');
* }
* </code>
* </div>
*
* <div>
* <code>
* let img;
* function preload() {
* img = loadImage('assets/moonwalk.jpg');
* }
*
* function setup() {
* // Create a 3D canvas
* createCanvas(400, 400, WEBGL);
* }
*
* function draw() {
* background(200);
* image(img, 0, 0, -200, 400, 400,500, 100, 200, 200);
* describe('Draw a subsection of the image from 500 by 100 units position at the center with size 400x400 and zoomedout by 200 units');
* }
*
* </code>
* </div>
*
*
*/
/**
* @method image
Expand All @@ -1078,6 +1180,8 @@ function _sAssign(sVal, iVal) {
* rectangle in which to draw the source image
* @param {Number} dy the y-coordinate of the destination
* rectangle in which to draw the source image
* @param {Number} dz the z-coordinate (depth) of the destination
* rectangle in which to draw the source image (WEBGL only)
* @param {Number} dWidth the width of the destination rectangle
* @param {Number} dHeight the height of the destination rectangle
* @param {Number} sx the x-coordinate of the subsection of the source
Expand All @@ -1097,6 +1201,7 @@ p5.prototype.image = function(
img,
dx,
dy,
dz,
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 placing this argument here will break calls to image() that don't use z but do use the arguments that follow it.

I think we might need to follow the approach of some other 2D/3D functions like dist here, where we reinterpret the arguments array based on some criteria:

if (args.length === 4) {
//2D
return Math.hypot(args[2] - args[0], args[3] - args[1]);
} else if (args.length === 6) {
//3D
return Math.hypot(args[3] - args[0], args[4] - args[1], args[5] - args[2]);
}

In this case, the criteria will probably depend on the length of the arguments.

Since this is somewhat complex, I think it would also be a good idea to add some unit tests to make sure that this continues to work for the 2D case as well as the new 3D case.

Copy link
Contributor Author

@Forchapeatl Forchapeatl Aug 16, 2024

Choose a reason for hiding this comment

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

Thank you for the review. I went with the length approach initially .

Odd number of parameters → Use 2D mode.
Even number of parameters → Check if the 10th parameter is a number.
If it's not a number, defer to 2D mode.
If it's a number, use 3D mode.

but starting from the third parameter single arguments become optional image(img, dx, dy, dWidth, dHeight, sx, sy, [sWidth], [sHeight], [fit], [xAlign], [yAlign]) hence my even/odd length logic fails as soon as the function starts receiving optional arguments.

Screenshot from 2024-08-16 19-26-04

The best solution I got at the moment was, to shift the values of the input argument to the right in case of 2D_rendering.

  if(this._renderer instanceof p5.RendererGL == false){
    // Store the value of 'yAlign' temporarily
    let   temp = yAlign;
    // From the 3rd arguement shift the assingment one position to the right   
    yAlign = xAlign;
    xAlign = fit;
    fit = sHeight;
    sHeight = sWidth;
    sWidth = sy;
    sy = sx;
    sx = dHeight;
    dHeight = dWidth;
    dWidth = dz;    
  }

The results have been successful in every test .

Screenshot from 2024-08-16 19-31-01

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this also work for 3D mode if the z coordinate is omitted?

Also a possible other approach for the even/odd check: you might need to check if the number of arguments up to the fit parameter (which should be a string instead of a number) is even or odd. That might mean something like:

let endIndex = args.findIndex(arg => arg === constants.COVER || arg === constants.CONTAIN)
if (endIndex === -1) {
  endIndex = args.length
}
const argsUpToFit = args.slice(0, endIndex)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this also work for 3D mode if the z coordinate is omitted?

yes.

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 are in WebGL mode and call image without z but with some of the options after z, does that still work? e.g. this should draw the image fullscreen on the canvas:

imageMode(CENTER)
image(yourImg, 0, 0, width, height, 0, 0, yourImg.width, yourImg.height, COVER)

Just based on the code so far, it looks like in WebGL mode, it never shifts all the arguments over the way it does when this._renderer instanceof p5.RendererGL === false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

It seems like the ImgeMode(CENTER) is a trap on WebGL mode. Is there any way to workaround this ? we could add a new imageModeGL that supports WebGL coordinates ?

dWidth,
dHeight,
sx,
Expand All @@ -1110,6 +1215,23 @@ p5.prototype.image = function(
// set defaults per spec: https://goo.gl/3ykfOq

p5._validateParameters('image', arguments);
// check for P5 Graphics instance
let isP5G = img instanceof p5.Graphics ? true : false;
// check for P5 Framebuffer instance
let isP5Fbo = img instanceof p5.Framebuffer ? true : false;

if(this._renderer instanceof p5.RendererGL === false){
// From the 3rd arguement shift the assingment one position to the right
yAlign = xAlign;
xAlign = fit;
fit = sHeight;
sHeight = sWidth;
sWidth = sy;
sy = sx;
sx = dHeight;
dHeight = dWidth;
dWidth = dz;
}

let defW = img.width;
let defH = img.height;
Expand Down Expand Up @@ -1175,18 +1297,37 @@ p5.prototype.image = function(
_sh
);

// tint the image if there is a tint
this._renderer.image(
img,
vals.sx,
vals.sy,
vals.sw,
vals.sh,
vals.dx,
vals.dy,
vals.dw,
vals.dh
);
//if it is not graphics nor framebuffer but WEGL instance
//default to use 3D rendering
if (this._renderer instanceof p5.RendererGL && !isP5G && !isP5Fbo) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like maybe this check should be a check for whether the dz parameter exists rather than the type of the image? since you could plausibly be drawing a graphic or a framebuffer both in 3D or without specifying a z

if (dz === undefined){dz = 0;}
this._renderer.image3D(
img,
vals.sx,
vals.sy,
dz,
vals.sw,
vals.sh,
vals.dx,
vals.dy,
vals.dw,
vals.dh
);
}else {
// tint the image if there is a tint
// Default 2D rendering
this._renderer.image(
img,
vals.sx,
vals.sy,
vals.sw,
vals.sh,
vals.dx,
vals.dy,
vals.dw,
vals.dh
);
}
};

/**
Expand Down
38 changes: 38 additions & 0 deletions src/webgl/p5.RendererGL.js
Original file line number Diff line number Diff line change
Expand Up @@ -2391,6 +2391,44 @@ p5.RendererGL = class RendererGL extends p5.Renderer {

return triangleVerts;
}

image3D(img,sx,sy,sz,sWidth,sHeight,dx,dy,dWidth,dHeight) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This shares a lot of code with p5.RendererGL.image, maybe we could make that function support a z coordinate rather than duplicating the code for this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code uses 3D primitives and changes to p5.RendererGL has been removed

const viewport = this.GL.getParameter(this.GL.VIEWPORT);
const width = viewport[2];
const height = viewport[3];
dx = (-width / 2) + dx;
dy = (-height / 2) + dy;
this._pInst.push();
this._pInst.noLights();
this._pInst.noStroke();
this._pInst.texture(img);
this._pInst.textureMode(constants.NORMAL);

// Calculate texture coordinates for subsection
let u0 = sx / img.width;
let u1 = (sx + sWidth) / img.width;
let v0 = sy / img.height;
let v1 = (sy + sHeight) / img.height;

// Draw a textured rectangle (plane) with the texture coordinates
this.beginShape();
// Top-left corner
this.vertex(dx, dy, sz, u0, v0);
// Top-right corner
this.vertex(dx + dWidth, dy, sz, u1, v0);
// Bottom-right corner
this.vertex(dx + dWidth, dy + dHeight, sz, u1, u1);
// Bottom-left corner
this.vertex(dx, dy + dHeight, sz, u0, v1);
this.endShape(constants.CLOSE);

this._pInst.pop();

if (this._isErasing) {
this.blendMode(constants.REMOVE);
}
}

};
/**
* ensures that p5 is using a 3d renderer. throws an error if not.
Expand Down
4 changes: 2 additions & 2 deletions test/unit/image/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ suite('displaying images that use fit mode', function() {
test('CONTAIN when source image is larger than destination', function() {
let src = myp5.createImage(400, 1000);
sinon.spy(myp5._renderer, 'image');
myp5.image(src, 0, 0, 300, 400, 0, 0, 400, 1000, myp5.CONTAIN);
myp5.image(src, 0, 0, 0, 300, 400, 0, 0, 400, 1000, myp5.CONTAIN);
assert(myp5._renderer.image.calledOnce);
assert.equal(myp5._renderer.image.getCall(0).args[7], 400 / (1000 / 400)); // dw
assert.equal(myp5._renderer.image.getCall(0).args[8], 1000 / (1000 / 400)); // dh
Expand All @@ -546,7 +546,7 @@ suite('displaying images that use fit mode', function() {
test('CONTAIN when source image is smaller than destination', function() {
let src = myp5.createImage(40, 90);
sinon.spy(myp5._renderer, 'image');
myp5.image(src, 0, 0, 300, 500, 0, 0, 400, 1000, myp5.CONTAIN);
myp5.image(src, 0, 0, 0, 300, 500, 0, 0, 400, 1000, myp5.CONTAIN);
assert(myp5._renderer.image.calledOnce);
assert.equal(myp5._renderer.image.getCall(0).args[7], 40 / (90 / 500)); // dw
assert.equal(myp5._renderer.image.getCall(0).args[8], 90 / (90 / 500)); // dh
Expand Down
Loading