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.

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

Conversation

davepagurek
Copy link
Contributor

Resolves #5793

Changes:

  • Applies blending when the blend mode is not BLEND regardless of opacity
  • Handles the ADD blend mode more similarly to in 2D mode
  • Makes sure the blend mode is saved/restored in push()/pop()
  • Updates docs to mention that DIFFERENCE is not implemented in WebGL mode

Screenshots of the change:

In each image, left is 2D mode, and right is WebGL

BeforeAfter
image image

PR Checklist

  • npm run lint passes
  • [Inline documentation] is included / updated
  • [Unit tests] are included / updated

@davepagurek davepagurek requested a review from stalgiag September 9, 2022 21:04
@@ -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

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.

@@ -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

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

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!

'Resets to MULTIPLY'
);
done();
});
Copy link
Contributor

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.

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.

blendMode() not working in WebGL mode
2 participants