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

Fix portal blending with portal surfaces #1019

Closed

Conversation

VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Jan 6, 2024

Enables writing to color buffer when rendering the portal surface in FinalisePortalCommand::ExecuteSelf(). Changes glStencilOp to avoid stencil test failing in light pass, and clears stencil buffer in case there were multiple portals on the screen. Fixes #1011.

glStencilOp( GL_KEEP, GL_KEEP, GL_DECR ) was likely intended to prevent incorrect rendering with multiple portals on the screen, but it was preventing the portal surface from rendering.

Here's some screenshots:
unvanquished_2024-01-07_020340_000
unvanquished_2024-01-07_020416_000
unvanquished_2024-01-07_020443_000

The colour is slightly off because for some reason the portal surfaces are rendered twice, so they blend with themselves. The colour is off due to light pass, which may or may not be a bug with the lighting system. Using Nsight confirms that the first draw produces the expected result (this is not the final colour that will be in the frame, but it illustrates the issue):
debug_portal_blend1
debug_portal_blend2

Enables writing to color buffer when rendering the portal surface in FinalisePortalCommand::ExecuteSelf(). Changes glStencilOp to avoid depth test failing and clears stencil buffer in case there were multiple portals on the screen. Fixes DaemonEngine#1011.
@illwieckz
Copy link
Member

Awesome ! 😲️

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 7, 2024

The colour is slightly off because for some reason the portal surfaces are rendered twice, so they blend with themselves. Using Nsight confirms that the first draw produces the expected result (this is not the final colour that will be in the frame, but it illustrates the issue):

Apparently that is the light stage. If the output from it is incorrect, then that issue is likely with lighting. It could also be that tremulous was rendering it wrong.

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 7, 2024

glStencilOp( GL_KEEP, GL_KEEP, GL_DECR ) was likely intended to prevent incorrect rendering with multiple portals on the screen, but it was preventing the portal surface from rendering.

On second thought, that might be for recursive portals. This pr might break them, but we don't seem to have any maps with those, so I'd need a test case map for that.

@illwieckz
Copy link
Member

On second thought, that might be for recursive portals. This pr might break them, but we don't seem to have any maps with those, so I'd need a test case map for that.

spacetracks map, viewpos 128 -1950 602 90 0, you should see the two mirrors at the same time on the left and on the right.

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 7, 2024

spacetracks map, viewpos 128 -1950 602 90 0, you should see the two mirrors at the same time on the left and on the right.

They don't seem to be in view of each other though? I've just got a test map from @cu-kai and it looks like recursive portals don't even work in 0.54.1.

@illwieckz
Copy link
Member

It probably works, but not entirely. The fact we can see the two mirrors of spacetracks at the same time in 0.54.1 is only possible because I enabled portal recursion some time ago. If we disable recursion, only one portal is displayed at a time and a warning is printed.

I also made a test map and recursion seems to only be half working.

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 7, 2024

Interesting, I think it might be linked to one of the issues described in #1005 (comment), since it looks like things that should be drawn through a recursive portal aren't added correctly.

@illwieckz
Copy link
Member

illwieckz commented Jan 8, 2024

Apparently that is the light stage. If the output from it is incorrect, then that issue is likely with lighting. It could also be that tremulous was rendering it wrong.

For now ignore any lightmap issue, there is an ongoing PR to fix many lightmap issues and trying to fix lightmap issues before merging that other PR is likely to introduce more bugs and create strong merge conflicts:

That PR does much more than what the title says.

Also I have some ideas about why there may be lightmap issues with portals, but I will not investigate those ideas before that lightmap PR is merged.

So any lightmap issue remaining when rendering portals should not block the merge of that PR.

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 8, 2024

Ah, that's good to hear.

@VReaperV
Copy link
Contributor Author

Closing this in favour of #1031, which fixes this issue among some other things without breaking recursive portals.

@VReaperV VReaperV closed this Jan 15, 2024
@VReaperV VReaperV deleted the portal-surface-blend-fix branch January 15, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cancelled
Development

Successfully merging this pull request may close these issues.

Portals don't blend
2 participants