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

[WIP] Added stencil function and depth function #78542

Closed
wants to merge 1 commit into from

Conversation

QbieShay
Copy link
Contributor

This is a rebase on latest master of @apples 's stencil work on https://github.com/apples/godot/tree/depth-function+stencil-buffer

This PR is not here to be merged

This PR is here to give people the opportunity to test it out by easily downloading it via github actions. The goal is to simplify the interface of the stencil operation (interface in the broader human-machine interaction layer sense) and to then create a new PR with the gathered knowledge.

Many thanks to apples for doing the initial bulk of the work and everyone else that participated over the years to this discussion in multiple issues. Last discussion: godotengine/godot-proposals#3373

I'm hoping that by testing it with more people we can come to a common understanding for a simpler and more intuitive interface, language and defaults.

Here's my test project where i tried to achieve the windwaker effect and outlines.

2023-06-22.01-33-27.mp4

stencil.zip

@QbieShay
Copy link
Contributor Author

Improvement note: if compare is not specified, then it should auto-add compare_always. since that's required also for writing it seems?

@apples
Copy link
Contributor

apples commented Jun 23, 2023

Thanks for wrapping this into a PR! I haven't had a lot of time to work on it lately.

Improvement note: if compare is not specified, then it should auto-add compare_always. since that's required also for writing it seems?

I wrote all the stencil render modes to pretty much do nothing by default, even if stencil is enabled. This was to ensure that simply checking stencil_enabled in the material wouldn't actually change the rendering behavior, unless you also modify some of the stencil fields.

I definitely agree there could be better defaults though. I was just being extra cautious when I wrote it.

@QbieShay
Copy link
Contributor Author

No worrie @apples ! I making stencil easier to grasp is painfully complicated. I ended up reading vulkan docs and opengl docs and i started wondering if we shouldn't reimplement something else that covers stencil functionality but has a better UX .. the part that's most nonsensical for me is that ref is the same value that you test against and that you write if you do replace ... WHY

@QbieShay
Copy link
Contributor Author

seeing above's comment, and i think this is a big confusion step, i think operation replace should be disabled when stencil test is on. makes no sense, just use keep..?

@QbieShay
Copy link
Contributor Author

For some reason using discard in fragment causes stencil write to not happen. Hard to make invisible masks. ALPHA = 0 works.

@QbieShay
Copy link
Contributor Author

QbieShay commented Jun 24, 2023

Update: exposed ref value to spatial shaders. I think the way the bitmask is exposed is not ideal because you can't select the entire mask.

Also, like clay suggested before rendering order is, in fact, hell. This works okay for the transparent pipeline because you can use rendering order, but i think render order would be very very important here and should be taken into account for opaque objects as well because then it starts to matter.

( CC @clayjohn , i think we should implement rendering order for opaque as well. Alternatively we could just take all opaque objects that write to stencil and render those beforehand)

@QbieShay
Copy link
Contributor Author

QbieShay commented Jun 24, 2023

Update 2: texture based stencil draw, if it's alpha scssorder, works out of the box.

image

@apples
Copy link
Contributor

apples commented Jun 24, 2023

Alternatively we could just take all opaque objects that write to stencil and render those beforehand

Some stencil materials might use the "depth fail" operation to write stencil values. These materials would need to be rendered after all other opaque materials.

For x-ray/outline effects, simply using render_priority seems to work best (in my experience). I am not sure what the best way to handle other effects would be (e.g. stencil shadows).

@QbieShay
Copy link
Contributor Author

@apples did you manage to use render priority for opaque objects?

@apples
Copy link
Contributor

apples commented Jun 26, 2023

did you manage to use render priority for opaque objects?

Yes, render priority seems to work fine for opaque objects, despite the docs saying otherwise.

However, some render modes can silently move objects to the alpha pass anyways. This initially caused me to think that render priority wasn't working. See relevant issue #73158.

Edit: Also, depth pre-pass can cause some confusing results.

@QbieShay
Copy link
Contributor Author

So clay and I are currently trying to define a better API than raw stencil and we're experimenting with the usecases that were brought up in the issue. I successfully did the windwaker effect but it flickers due to sorting issues that may need to be addressed explicitely when sorting is invovled

Here's the updated project
stencil-withwinwaker.zip

@QbieShay
Copy link
Contributor Author

Superseded by #80710

@QbieShay QbieShay closed this Dec 16, 2023
@AThousandShips AThousandShips removed this from the 4.x milestone Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants