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.

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

Updated Stencil Test Case to prevent it from disable after every draw #7578

Open
wants to merge 7 commits into
base: dev-2.0
Choose a base branch
from
+140 −5
Open

Updated Stencil Test Case to prevent it from disable after every draw #7578

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b42d2a1
Updated Stencil Test Case to prevent it from disable after every draw…
Vaivaswat2244 Feb 26, 2025
2e88c82
updated beginClip and pop
Vaivaswat2244 Feb 28, 2025
38abe63
fixed breaking the tests
Vaivaswat2244 Feb 28, 2025
bc4b5f8
Added internal functions and some logic changes
Vaivaswat2244 Mar 2, 2025
ed26828
Minor changes and added tests for the stencil tests
Vaivaswat2244 Mar 2, 2025
f300db5
Merge branch 'dev-2.0' into stencil_test
Vaivaswat2244 Mar 2, 2025
441468e
fixed the linting which happened because of merging conflicts mb
Vaivaswat2244 Mar 2, 2025
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
67 changes: 64 additions & 3 deletions src/webgl/p5.RendererGL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,43 @@ class RendererGL extends Renderer {
this.drawShapeCount = 1;

this.scratchMat3 = new Matrix(3);

this.isStencilTestOn = false; // Track stencil test state
this._userEnabledStencil = false; // Track whether user enabled stencil
// Override WebGL enable function
const prevEnable = this.drawingContext.enable;
this.drawingContext.enable = (key) => {
if (key === this.drawingContext.STENCIL_TEST) {
if (!this._clipping) {
this._userEnabledStencil = true;
}
this.isStencilTestOn = true;
}
return prevEnable.call(this.drawingContext, key);
};

// Override WebGL disable function
const prevDisable = this.drawingContext.disable;
this.drawingContext.disable = (key) => {
if (key === this.drawingContext.STENCIL_TEST) {
if (this._clipDepth === this._pushPopDepth) {
this.isStencilTestOn = this._userEnabledStencil;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a little unintuitive that calling disable() elsewhere in the code might not actually disable. How do you feel about making enable/disable only record the last value so that getEnabled is fast, and then place this logic of what value to revert to elsewhere?

The idea would be, write the code so that the behaviour would be the same even if we haven't overridden enable and disable: e.g. when we begin clipping, we can record this.isStencilTestOn = this.drawingContext.getEnabled(this.drawingContext.STENCIL_TEST), and then in pop where we turn off stencil testing currently, we would either turn it on or off based on the value of this.isStencilTestOn.

Then, this overriding of enable/disable just makes the getEnabled call run faster, without changing what it does or its side effects.

} else {
this.isStencilTestOn = false;
this._userEnabledStencil = false;
}
}
return prevDisable.call(this.drawingContext, key);
};

// Override WebGL getEnabled function
const prevGetEnabled = this.drawingContext.getEnabled;
this.drawingContext.getEnabled = (key) => {
if (key === this.drawingContext.STENCIL_TEST) {
return this.isStencilTestOn;
}
return prevGetEnabled.call(this.drawingContext, key);
};
}

//////////////////////////////////////////////
Expand Down Expand Up @@ -1024,7 +1061,10 @@ class RendererGL extends Renderer {
//Clear depth every frame
this.GL.clearStencil(0);
this.GL.clear(this.GL.DEPTH_BUFFER_BIT | this.GL.STENCIL_BUFFER_BIT);
this.GL.disable(this.GL.STENCIL_TEST);
if (!this.isStencilTestOn) {
this.GL.disable(this.GL.STENCIL_TEST);
}

}

/**
Expand Down Expand Up @@ -1387,7 +1427,10 @@ class RendererGL extends Renderer {
this.drawTarget()._isClipApplied = true;

const gl = this.GL;
gl.clearStencil(0);
this._savedStencilTestState = this._userEnabledStencil;
this._preClipStencilState = this.drawingContext.getEnabled(this.drawingContext.STENCIL_TEST);
// this.isStencilTestOn = this.drawingContext.getEnabled(this.drawingContext.STENCIL_TEST);
gl.clearStencil(0);
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 there's a trailing space at the end here that we can take out.

Copy link
Author

Choose a reason for hiding this comment

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

Sure

gl.clear(gl.STENCIL_BUFFER_BIT);
gl.enable(gl.STENCIL_TEST);
this._stencilTestOn = true;
Expand Down Expand Up @@ -1739,6 +1782,22 @@ class RendererGL extends Renderer {
this._pushPopDepth === this._clipDepths[this._clipDepths.length - 1]
) {
this._clearClip();
// if (this.isStencilTestOn) {
// this.GL.enable(this.GL.STENCIL_TEST);
// } else {
// this.GL.disable(this.GL.STENCIL_TEST);
// }
// if (!this._savedStencilTestState) {
// this.GL.disable(this.GL.STENCIL_TEST);
// }
if (this._preClipStencilState) {
this.GL.enable(this.GL.STENCIL_TEST);
} else {
this.GL.disable(this.GL.STENCIL_TEST);
}

// Reset saved state
this._userEnabledStencil = this._savedStencilTestState;
}
super.pop(...args);
this._applyStencilTestIfClipping();
Expand All @@ -1750,7 +1809,9 @@ class RendererGL extends Renderer {
this.GL.enable(this.GL.STENCIL_TEST);
this._stencilTestOn = true;
} else {
this.GL.disable(this.GL.STENCIL_TEST);
if (!this.isStencilTestOn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this only disable if the user has not manually called disable? If so, maybe we need to be using the _userEnabledStencil for the if condition?

this.GL.disable(this.GL.STENCIL_TEST);
}
this._stencilTestOn = false;
}
}
Expand Down