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

Stencil buffer fix #2325

Merged
merged 6 commits into from
Oct 28, 2024
Merged

Stencil buffer fix #2325

merged 6 commits into from
Oct 28, 2024

Conversation

zzuegg
Copy link
Member

@zzuegg zzuegg commented Oct 28, 2024

This fixes the use of the stencil buffers.

As reference:
https://hub.jmonkeyengine.org/t/how-to-do-outline-with-stencil-buffer/47954/5

Also added the TestStencilOutline.java to the examples.
Note: Somehow the TestChooser app is overwriting the appsettings in the main method.
As a fix i added a postprocessor to the example application. This is not required when running
the test directly.

Copy link
Contributor

@codex128 codex128 left a comment

Choose a reason for hiding this comment

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

These changes look fine to me. I'd just suggest that some spacing be added. I'm a bit surprised that RenderState didn't already copy those parameters.

Would you mind explaining what the bug turned out being exactly?

frontStencilMask=state.frontStencilMask;
frontStencilReference=state.frontStencilReference;
backStencilMask=state.backStencilMask;
backStencilReference=state.backStencilReference;
Copy link
Contributor

@codex128 codex128 Oct 28, 2024

Choose a reason for hiding this comment

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

Add spacing around '='.

state.frontStencilMask=frontStencilMask;
state.frontStencilReference=frontStencilMask;
state.backStencilMask=backStencilMask;
state.backStencilReference=backStencilMask;
Copy link
Contributor

@codex128 codex128 Oct 28, 2024

Choose a reason for hiding this comment

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

Add spacing around '='.

state.frontStencilMask=additionalState.frontStencilMask;
state.frontStencilReference=additionalState.frontStencilMask;
state.backStencilMask=additionalState.backStencilMask;
state.backStencilReference=additionalState.backStencilMask;
Copy link
Contributor

@codex128 codex128 Oct 28, 2024

Choose a reason for hiding this comment

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

Add spacing around '='.

@zzuegg
Copy link
Member Author

zzuegg commented Oct 28, 2024

Thanks for point out the spacing error.
Due the fact that the Mask/Reference was not copied/included in the hash, the actual values have not been send to opengl, you were stuck with the reference set to 0 and the mask to 255.

Since i added those values back in 2022, and the fix was already present in my local fork, it seems i forgot to commit them back then. As far i remember the original usecase used only Increment and NotEqual to 0 so it slipped trough the testing.

@stephengold stephengold merged commit b34fed9 into jMonkeyEngine:master Oct 28, 2024
14 checks passed
@stephengold stephengold added the bug Something that is supposed to work, but doesn't. More severe than a "defect". label Oct 28, 2024
@stephengold stephengold added this to the Future Release milestone Oct 28, 2024
@zzuegg zzuegg deleted the stencil-buffer branch October 28, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to work, but doesn't. More severe than a "defect".
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants