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

Enhancement limit WebGL textures to gl.MAX_TEXTURE_SIZE #6651

Merged
merged 11 commits into from
Mar 19, 2024
2 changes: 1 addition & 1 deletion src/core/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ p5.prototype.pixelDensity = function(val) {
let returnValue;
if (typeof val === 'number') {
if (val !== this._pixelDensity) {
this._pixelDensity = val;
this._pixelDensity = this._maxAllowedPixelDimensions = val;
}
returnValue = this;
this.resizeCanvas(this.width, this.height, true); // as a side effect, it will clear the canvas
Expand Down
1 change: 1 addition & 0 deletions src/core/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ class p5 {
this._preloadDone = false;
// for handling hidpi
this._pixelDensity = Math.ceil(window.devicePixelRatio) || 1;
this._maxAllowedPixelDimensions = 0;
this._userNode = node;
this._curElement = null;
this._elements = [];
Expand Down
4 changes: 4 additions & 0 deletions src/core/p5.Graphics.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ p5.Graphics = class extends p5.Element {

if (r === constants.WEBGL) {
this._renderer = new p5.RendererGL(this.canvas, this, false);
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 we want this for both graphics and the main canvas, so maybe we should do the check inside RendererGL instead?

const { adjustedWidth, adjustedHeight } =
this._renderer._adjustDimensions(w, h);
w = adjustedWidth;
h = adjustedHeight;
} else {
this._renderer = new p5.Renderer2D(this.canvas, this, false);
}
Expand Down
10 changes: 10 additions & 0 deletions src/core/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ p5.prototype.createCanvas = function(w, h, renderer, canvas) {
if (r === constants.WEBGL) {
this._setProperty('_renderer', new p5.RendererGL(c, this, true));
this._elements.push(this._renderer);
const dimensions =
this._renderer._adjustDimensions(w, h);
w = dimensions.adjustedWidth;
h = dimensions.adjustedHeight;
} else {
//P2D mode
if (!this._defaultGraphicsCreated) {
Expand Down Expand Up @@ -198,6 +202,12 @@ p5.prototype.resizeCanvas = function(w, h, noRedraw) {
}
this.width = w;
this.height = h;
if (!(this._renderer instanceof p5.Renderer2D)) {
const dimensions =
this._renderer._adjustDimensions(w, h);
w = dimensions.adjustedWidth;
h = dimensions.adjustedHeight;
}
// Make sure width and height are updated before the renderer resizes so
// that framebuffers updated from the resize read the correct size
this._renderer.resize(w, h);
Expand Down
18 changes: 11 additions & 7 deletions src/webgl/p5.Framebuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,14 @@ class Framebuffer {
console.warn('Antialiasing is unsupported in a WebGL 1 context');
this.antialias = false;
}
this.density = settings.density || target.pixelDensity();
const gl = target._renderer.GL;
this.gl = gl;
if (settings.width && settings.height) {
this.width = settings.width;
this.height = settings.height;
const dimensions =
target._renderer._adjustDimensions(settings.width, settings.height);
this.width = dimensions.adjustedWidth;
this.height = dimensions.adjustedHeight;
this.autoSized = false;
} else {
if ((settings.width === undefined) !== (settings.height === undefined)) {
Expand All @@ -147,11 +152,6 @@ class Framebuffer {
this.height = target.height;
this.autoSized = true;
}
this.density = settings.density || target.pixelDensity();

const gl = target._renderer.GL;
this.gl = gl;

this._checkIfFormatsAvailable();

if (settings.stencil && !this.useDepth) {
Expand Down Expand Up @@ -227,6 +227,10 @@ class Framebuffer {
*/
resize(width, height) {
this.autoSized = false;
const dimensions =
this.target._renderer._adjustDimensions(width, height);
width = dimensions.adjustedWidth;
height = dimensions.adjustedHeight;
this.width = width;
this.height = height;
this._handleResize();
Expand Down
32 changes: 32 additions & 0 deletions src/webgl/p5.RendererGL.js
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,38 @@ p5.RendererGL = class RendererGL extends p5.Renderer {
}
}

_getParam() {
const gl = this.drawingContext;
return gl.getParameter(gl.MAX_TEXTURE_SIZE);
}

_adjustDimensions(width, height) {
if (!this._maxTextureSize) {
this._maxTextureSize = this._getParam();
}
let maxTextureSize = this._maxTextureSize;
let maxAllowedPixelDimensions = p5.prototype._maxAllowedPixelDimensions;

maxAllowedPixelDimensions = Math.floor(
maxTextureSize / this.pixelDensity()
);
let adjustedWidth = Math.min(
width, maxAllowedPixelDimensions
);
let adjustedHeight = Math.min(
height, maxAllowedPixelDimensions
);

if (adjustedWidth !== width || adjustedHeight !== height) {
console.warn(
'Warning: The requested width/height exceeds hardware limits. ' +
`Adjusting dimensions to width: ${adjustedWidth}, height: ${adjustedHeight}.`
);
}

return { adjustedWidth, adjustedHeight };
}

//This is helper function to reset the context anytime the attributes
//are changed with setAttributes()

Expand Down
13 changes: 13 additions & 0 deletions test/unit/core/p5.Graphics.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,5 +164,18 @@ suite('Graphics', function() {
graph.resizeCanvas(19, 16);
assertValidPixels(graph, 19, 16, 2);
});

test('it resizes the canvas and the graphics based on max texture size', function() {
let glStub;
glStub = sinon.stub(p5.RendererGL.prototype, '_getParam');
const fakeMaxTextureSize = 100;
glStub.returns(fakeMaxTextureSize);

var graph = myp5.createGraphics(200, 200, myp5.WEBGL);
graph.pixelDensity(1);
assertValidGraphSizes(graph, 100, 100, 1);

glStub.restore();
});
});
});
26 changes: 26 additions & 0 deletions test/unit/core/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,32 @@ suite('Rendering', function() {
assert.equal(fbo.width, 5);
assert.equal(fbo.height, 15);
});

test('should resize the dimensions of canvas based on max texture size', function() {
let glStub;
glStub = sinon.stub(p5.RendererGL.prototype, '_getParam');
const fakeMaxTextureSize = 100;
glStub.returns(fakeMaxTextureSize);
myp5.createCanvas(200, 200, myp5.WEBGL);
assert.equal(myp5.canvas.width, 100);
assert.equal(myp5.canvas.height, 100);

glStub.restore();
});

test('should resize the dimensions of canvas by resizeCanvas based on max texture size', function() {

let glStub;
glStub = sinon.stub(p5.RendererGL.prototype, '_getParam');
const fakeMaxTextureSize = 100;
glStub.returns(fakeMaxTextureSize);
myp5.createCanvas(10, 10, myp5.WEBGL);
myp5.resizeCanvas(200, 200);
assert.equal(myp5.canvas.width, 100);
assert.equal(myp5.canvas.height, 100);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think myp5.width and myp5.height should also be 100 here? I think currently they aren't because we set this.width and this.height before adjusting the sizes, but I'm not sure just yet what the expected result would be. I think I lean towards having those values also be 100 for consistency, what do you think?


glStub.restore();
});
});

suite('p5.prototype.blendMode', function() {
Expand Down
30 changes: 30 additions & 0 deletions test/unit/webgl/p5.Framebuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,36 @@ suite('p5.Framebuffer', function() {
// The texture should not be recreated
expect(fbo.color.rawTexture()).to.equal(oldTexture);
});

test('resizes the framebuffer using its constructor based on max texture size', function() {
let glStub;
glStub = sinon.stub(p5.RendererGL.prototype, '_getParam');
const fakeMaxTextureSize = 100;
glStub.returns(fakeMaxTextureSize);

myp5.createCanvas(10, 10, myp5.WEBGL);
const fbo = myp5.createFramebuffer({ width: 200, height: 200 });
expect(fbo.width).to.equal(100);
expect(fbo.height).to.equal(100);

glStub.restore();
});

test('resizes the framebuffer using resize method based on max texture size', function() {
let glStub;
glStub = sinon.stub(p5.RendererGL.prototype, '_getParam');
const fakeMaxTextureSize = 100;
glStub.returns(fakeMaxTextureSize);

myp5.createCanvas(10, 10, myp5.WEBGL);
const fbo = myp5.createFramebuffer({ width: 10, height: 10 });
myp5.resize(200, 200);
expect(fbo.width).to.equal(100);
expect(fbo.height).to.equal(100);


glStub.restore();
});
});
});

Expand Down
Loading