Skip to content

Fix WebGL blending bugs #5794

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 11, 2022
Merged

Fix WebGL blending bugs #5794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/core/rendering.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ p5.prototype.createGraphics = function(w, h, renderer) {
* min(A*factor, B).</li>
* <li><code>LIGHTEST</code> - only the lightest colour succeeds: C =
* max(A*factor, B).</li>
* <li><code>DIFFERENCE</code> - subtract colors from underlying image.</li>
* <li><code>DIFFERENCE</code> - subtract colors from underlying image.
* <em>(2D)</em></li>
* <li><code>EXCLUSION</code> - similar to <code>DIFFERENCE</code>, but less
* extreme.</li>
* <li><code>MULTIPLY</code> - multiply the colors, result will always be
Expand Down
10 changes: 8 additions & 2 deletions src/webgl/material.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,10 @@ p5.RendererGL.prototype._applyColorBlend = function(colors) {

const isTexture = this.drawMode === constants.TEXTURE;
const doBlend =
isTexture || colors[colors.length - 1] < 1.0 || this._isErasing;
isTexture ||
this.curBlendMode !== constants.BLEND ||
Copy link
Contributor

@stalgiag stalgiag Sep 11, 2022

Choose a reason for hiding this comment

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

This is a great and important change! I remember when we first added blendmodes to WebGL, we were getting a bunch of issues that all involved transparency or textures. It was easy to forget that people blend fully opaque colors as well 🤦‍♀️ . Great job finding a small improvement that makes a big difference!

Copy link
Contributor Author

@davepagurek davepagurek Sep 11, 2022

Choose a reason for hiding this comment

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

I dread every transparency/blending bug I encounter because it's so easy to mess up in WebGL 😅 I really appreciate your work setting all of it up so that transparency Just Works in p5!

colors[colors.length - 1] < 1.0 ||
this._isErasing;

if (doBlend !== this._isBlending) {
if (
Expand Down Expand Up @@ -1085,10 +1088,13 @@ p5.RendererGL.prototype._applyBlendMode = function() {
const gl = this.GL;
switch (this.curBlendMode) {
case constants.BLEND:
case constants.ADD:
gl.blendEquation(gl.FUNC_ADD);
gl.blendFunc(gl.SRC_ALPHA, gl.ONE_MINUS_SRC_ALPHA);
break;
case constants.ADD:
gl.blendEquation(gl.FUNC_ADD);
gl.blendFunc(gl.SRC_ALPHA, gl.ONE);
break;
case constants.REMOVE:
gl.blendEquation(gl.FUNC_REVERSE_SUBTRACT);
gl.blendFunc(gl.SRC_ALPHA, gl.DST_ALPHA);
Expand Down
1 change: 1 addition & 0 deletions src/webgl/p5.RendererGL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,7 @@ p5.RendererGL.prototype.push = function() {
properties.drawMode = this.drawMode;

properties._currentNormal = this._currentNormal;
properties.curBlendMode = this.curBlendMode;

return style;
};
Expand Down
64 changes: 63 additions & 1 deletion test/unit/webgl/p5.RendererGL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ suite('p5.RendererGL', function() {
test('blendModes change pixel colors as expected', function(done) {
myp5.createCanvas(10, 10, myp5.WEBGL);
myp5.noStroke();
assert.deepEqual([133, 69, 191, 255], mixAndReturn(myp5.ADD, 255));
assert.deepEqual([122, 0, 122, 255], mixAndReturn(myp5.ADD, 0));
Copy link
Contributor Author

@davepagurek davepagurek Sep 9, 2022

Choose a reason for hiding this comment

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

This was updated to use a black background because previously, the background was white, and blending anything onto white with ADD results in more white once ADD works the same way as in 2D mode.

assert.deepEqual([0, 0, 255, 255], mixAndReturn(myp5.REPLACE, 255));
assert.deepEqual([133, 255, 133, 255], mixAndReturn(myp5.SUBTRACT, 255));
assert.deepEqual([255, 0, 255, 255], mixAndReturn(myp5.SCREEN, 0));
Expand All @@ -447,6 +447,68 @@ suite('p5.RendererGL', function() {
assert.deepEqual([0, 0, 0, 255], mixAndReturn(myp5.DARKEST, 255));
done();
});

test('blendModes match 2D mode', function(done) {
myp5.createCanvas(10, 10, myp5.WEBGL);
myp5.setAttributes({ alpha: true });
const ref = myp5.createGraphics(myp5.width, myp5.height);
ref.translate(ref.width / 2, ref.height / 2); // Match WebGL mode

const testBlend = function(target, colorA, colorB, mode) {
target.clear();
target.push();
target.background(colorA);
target.blendMode(mode);
target.noStroke();
target.fill(colorB);
target.rectMode(target.CENTER);
target.rect(0, 0, target.width, target.height);
target.pop();
return target.get(0, 0);
};

const assertSameIn2D = function(colorA, colorB, mode) {
const refColor = testBlend(myp5, colorA, colorB, mode);
const webglColor = testBlend(ref, colorA, colorB, mode);
if (refColor[3] === 0) {
assert.equal(webglColor[3], 0);
} else {
assert.deepEqual(
refColor,
webglColor,
`Blending ${colorA} with ${colorB} using ${mode}`
);
}
};

const red = '#F53';
const blue = '#13F';
assertSameIn2D(red, blue, myp5.BLEND);
assertSameIn2D(red, blue, myp5.ADD);
assertSameIn2D(red, blue, myp5.DARKEST);
assertSameIn2D(red, blue, myp5.LIGHTEST);
assertSameIn2D(red, blue, myp5.EXCLUSION);
assertSameIn2D(red, blue, myp5.MULTIPLY);
assertSameIn2D(red, blue, myp5.SCREEN);
assertSameIn2D(red, blue, myp5.REPLACE);
assertSameIn2D(red, blue, myp5.REMOVE);
done();
});

test('blendModes are included in push/pop', function(done) {
myp5.createCanvas(10, 10, myp5.WEBGL);
myp5.blendMode(myp5.MULTIPLY);
myp5.push();
myp5.blendMode(myp5.ADD);
assert.equal(myp5._renderer.curBlendMode, myp5.ADD, 'Changed to ADD');
myp5.pop();
assert.equal(
myp5._renderer.curBlendMode,
myp5.MULTIPLY,
'Resets to MULTIPLY'
);
done();
});
Copy link
Contributor

@stalgiag stalgiag Sep 11, 2022

Choose a reason for hiding this comment

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

This is a great set of unit tests. This is a great safeguard against unexpected divergence between the renderers down the road.

});

suite('BufferDef', function() {
Expand Down