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

Investigating portal issues #1010

Open
illwieckz opened this issue Jan 4, 2024 · 25 comments · Fixed by #1018
Open

Investigating portal issues #1010

illwieckz opened this issue Jan 4, 2024 · 25 comments · Fixed by #1018

Comments

@illwieckz
Copy link
Member

illwieckz commented Jan 4, 2024

Our portal implementation is incomplete, it's basically a proof of concept waiting for more to be done.

Our portal code is very similar to the one in RBQUAKE-3, what is believed to be the most recent version of XreaL, so I guess this cannot help us more.

I identified that Tremulous 1.3 by GrangerHub, which is based on ioquake3, has working portal blending in its opengl2 renderer.

The code is much complete there. This seems to be nothing more than ioquake3 implementation and we could look at the ioquake3 impementation as well, but this build makes easier to load tremulous maps to compare with Dæmon.

I noticed we have some very small portal_fp.glsl and portal_vp.glsl shaders while ioquake3 just use generic_fp.glsl and generic_vp.glsl for portals.

I noticed that if I tell the Dæmon engine to use Render_generic instead of Render_portal function (generic GLSL instead of portal GLSL), the portals are rendered the same, and despite blending works on all other textures with generic glsl, this still doesn't work with portals.

This seems to confirm our portals are not rendered like other textures, I always assumed that portals were rendered after everything else is done. For sure they render above translucent textures that are in front of them.

Known portal issues:

@illwieckz
Copy link
Member Author

illwieckz commented Jan 4, 2024

When I modify Render_portal to do:

GL_Bind( tr.blueImage );

The portal is still rendered, instead of a blue texture!

Maybe the portal is written to the wrong framebuffer and written directly on the screen? 🤔️

@illwieckz
Copy link
Member Author

Render_portal() is not executed at all.

@illwieckz
Copy link
Member Author

illwieckz commented Jan 4, 2024

Even if I make sure it runs that does nothing.

Maybe Render_portal() is just used to render the texture that is faded with the portal when using alphaGen portal and this function does not render the portal at all.

@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

Even if I make sure it runs that does nothing.

Maybe Render_portal() is just used to render the texture that is faded with the portal when using alphaGen portal and this function does not render the portal at all.

Could it be in

const RenderCommand *PreparePortalCommand::ExecuteSelf( ) const
and
const RenderCommand *FinalisePortalCommand::ExecuteSelf( ) const
?

The first one seems to be used in depth pre-pass and the second one to actually render the portal.

@illwieckz
Copy link
Member Author

Hmm, PreparePortalCommand was previously named R_PreparePortalCmd, introduced in commit 62c51af by @gimhael.

The commit is named Add support for multiple and nested portals by using a stencil mask.

@illwieckz illwieckz changed the title Investigating portals Investigating portals and why they don't blend Jan 4, 2024
@illwieckz illwieckz changed the title Investigating portals and why they don't blend Investigating portals issues Jan 4, 2024
@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

Hmm, PreparePortalCommand was previously named R_PreparePortalCmd, introduced in commit 62c51af by @gimhael.

The commit is named Add support for multiple and nested portals by using a stencil mask.

Oh, true, depth is written somewhere between calls to PreparePortalCommand::ExecuteSelf( ) and FinalisePortalCommand::ExecuteSelf( ).

Looks like this

static bool R_MirrorViewBySurface(drawSurf_t *drawSurf)
is where the view for the portal is rendered and the above functions are called. The rendering cmds aren't actually executed at that point though from what I can gather?

@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

It looks like both methods add new rendering cmds for the backend to render. It seems to be rendering them in the order they were added, and the only draw call using a texture rendered from a portal seems to be between those 2 cmds. So I think the actual rendering of the portal surface happens somewhere here:

// save old viewParms so we can return to it after the mirror view
viewParms_t oldParms = tr.viewParms;
newParms.portalLevel++;
// convert screen rectangle to scissor test
// OPTIMIZE: could do better with stencil test in renderer backend
newParms.scissorX = surfRect.coords[0];
newParms.scissorY = surfRect.coords[1];
newParms.scissorWidth = surfRect.coords[2] - surfRect.coords[0] + 1;
newParms.scissorHeight = surfRect.coords[3] - surfRect.coords[1] + 1;
R_MirrorPoint(oldParms.orientation.origin, &surface, &camera, newParms.orientation.origin);
R_MirrorVector(oldParms.orientation.axis[0], &surface, &camera, newParms.orientation.axis[0]);
R_MirrorVector(oldParms.orientation.axis[1], &surface, &camera, newParms.orientation.axis[1]);
R_MirrorVector(oldParms.orientation.axis[2], &surface, &camera, newParms.orientation.axis[2]);
// restrict view frustum to screen rect of surface
R_SetupPortalFrustum(oldParms, camera, newParms);
// render the mirror view
R_RenderView( &newParms );
tr.viewParms = oldParms;

Presumably in R_RenderView( &newParms )? Might be wrong though.

@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

Maybe the portal is written to the wrong framebuffer and written directly on the screen? 🤔️

The framebuffers seem to be correct... At least the drawing of the actual map seems to use the same framebuffer for everything including portals, then a different framebuffer is used for the UI.

@illwieckz
Copy link
Member Author

What I mean is that maybe the portal should be rendered to a specific framebuffer to be able to render it?

Or is it simply expected to be rendered first so other stages are blended later on it?

@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

What I mean is that maybe the portal should be rendered to a specific framebuffer to be able to render it?

Or is it simply expected to be rendered first so other stages are blended later on it?

The latter is the case I think. It doesn't seem to be using separate framebuffers, instead it sets the matrices for the shader that renders the portals so that it's rendered as if you were at the other side of the portal itself. And they are indeed rendered first, so probably supposed to blend with stuff on top yeah.

@illwieckz
Copy link
Member Author

What can prevent a translucent object in front of a portal to not blend with it?

As seen in:

I guess we can get that result if:

  • The portal is painted on top of everything,
  • The renderer wrongly thinks there is something opaque above the translucent texture.

Is there any other possibility?

@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

What can prevent a translucent object in front of a portal to not blend with it?

As seen in:

I guess we can get that result if:

  • The portal is painted on top of everything,
  • The renderer wrongly thinks there is something opaque above the translucent texture.

Is there any other possibility?

Not sure... Translucent stuff seems to always be rendered last (before the player though), so things that are in front of them get rendered properly because the depth test fails there. It would seem like neither of these are the case with things rendered by portals though. The translucent stuff also seems to be rendered with this shader: https://github.com/DaemonEngine/Daemon/blob/6d1441070ebc8d3faa775754c201cdc95079c578/src/engine/renderer/glsl_source/generic_fp.glsl. I'm not seeing anything in it that would make it fail, maybe the inputs there should be checked (e. g. by outputting the values from texture samplers directly).

@VReaperV
Copy link
Contributor

VReaperV commented Jan 4, 2024

Actually, look at this:
unvanquished_2024-01-05_023205_000
I noclipped into the window (the window has 2 sides), you can see that on the right it's rendered is if I was in the room behind the window, on the left it's rendered as if I was behind the second window pane, but in the middle it appears to blend correctly. The window is being clipped this way because of zNear (/r_znear 3, which is the default) and my position.

@illwieckz
Copy link
Member Author

Ah yes, I now remember having noticed that in the past but I forgot it.

Here we can see both sides of the window being rendered above the portal, but at some point this disappear:

unvanquished_2024-01-05_004612_000

@illwieckz
Copy link
Member Author

The geometry of the what the portal renders have an effect on the window:

unvanquished_2024-01-05_004949_000

@illwieckz
Copy link
Member Author

unvanquished_2024-01-05_005142_000

unvanquished_2024-01-05_005225_000

@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

This also affects building outlines, here's with z_near 100:
unvanquished_2024-01-05_025741_000

@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

This also makes me think it might have something to do with depth (perhaps how it's used in generic_fp.glsl or the input values there?):

unvanquished_2024-01-05_031651_000
unvanquished_2024-01-05_031804_000
unvanquished_2024-01-05_034743_000

@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

It's also worth noting that you can't do that on grangermaze:
unvanquished_2024-01-05_032817_000
That's probably because the depth there is set to 1.0 since there's nothing behind the portal.

@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

@illwieckz I believe I found the issue. These are the color buffer and depth buffer as seen in Nsight on pulse near the window:
debug_a1
debug_a1d
The stuff rendered in portals has z-values around 0-0.5, mostly towards 0.0, but everything else has values close to 1.0.
This is the same place but noclipping into the window:
debug_a2
debug_a2d
You can see where the values in the depth buffer are different in places that actually got blended.
And this is the same thing with /r_znear 50:
debug_a3
debug_a3d
This results in more of the depth buffer being populated normally, rather than just values closer to 1.0. The issue is that the depth outputted by the portal rendering is incorrect (well, it may be correct for the portal's "camera", but not for this).

@illwieckz
Copy link
Member Author

What can be the fix? multiply/shift the portal depth-from-portal-point-of-view with current surface depth?

@illwieckz illwieckz changed the title Investigating portals issues Investigating portal issues Jan 5, 2024
@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

Maybe, although that might introduce issues with depth testing during portal rendering. We can just draw the portal surface again after everything in the portal is rendered, but with ColorMask set to false on all colours, depth test set to GL_ALWAYS and depth write enabled. In fact, the engine appears to be already doing so, but with depth test disabled, which prevents the depth write (from refpages: "GL_DEPTH_TEST
If enabled, do depth comparisons and update the depth buffer. Note that even if the depth buffer exists and the depth mask is non-zero, the depth buffer is not updated if the depth test is disabled. See glDepthFunc and glDepthRange.").

Also, it's worth noting that GL_State does not take anything other than GL_LEQUAL, GL_LESS, and GL_EQUAL as its values (or rather it defaults to GL_LEQUAL), so that should probably be expanded.

@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

I think changing this

GL_State( GLS_DEPTHMASK_TRUE | GLS_DEPTHTEST_DISABLE | GLS_COLORMASK_BITS );
to GL_State( GLS_DEPTHMASK_TRUE | GLS_COLORMASK_BITS ); should fix it.

@VReaperV
Copy link
Contributor

VReaperV commented Jan 5, 2024

Actually, also need to add a state bit for GL_ALWAYS or manually do glDepthFunc( GL_ALWAYS).

VReaperV added a commit to VReaperV/Daemon that referenced this issue Jan 5, 2024
Sets glDepthFunc to GL_ALWAYS when rendering the portal surface and enables depth test. Fixes DaemonEngine#1012, Unvanquished/Unvanquished#2476, and partially fixes DaemonEngine#1010.

Introduces GLS_DEPTHFUNC_ALWAYS which sets glDepthFunc to GL_ALWAYS. Shifted some enums to avoid conflicts in GL_State().
illwieckz pushed a commit that referenced this issue Jan 6, 2024
Sets glDepthFunc to GL_ALWAYS when rendering the portal surface and enables depth test. Fixes #1012, Unvanquished/Unvanquished#2476, and partially fixes #1010.

Introduces GLS_DEPTHFUNC_ALWAYS which sets glDepthFunc to GL_ALWAYS. Shifted some enums to avoid conflicts in GL_State().
@illwieckz illwieckz reopened this Jan 6, 2024
@VReaperV
Copy link
Contributor

#1005 and #1021 will be partially fixed by #1031, full fix in a later pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants