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

3D Sky Depth Fixes, Path-Traced Skyboxes Introduction #73

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

KingDavidW
Copy link
Contributor

I am finally creating this PR so I can have people take a look and see if I'm screwing up the general implementation.

Basics

  • The goal was firstly to fix depth not being shared for Skyboxes in Source Engine games.
  • The second and more ambitious goal was to enable path-tracing of the skybox instances. Since we can not path-trace more than one camera, moving the instances to the main camera was the route taken.
  • However, because skyboxes may have a larger scale, the sub-objective was to support 3D Skyboxes with such scaling properties.

Results

  • Shared depth option has been added to the 3D Sky options. This is not relevant to the path tracing but ensures the rasterized sky matte is behaving properly
  • Configuration options for enabling Path Traced skyboxes, in addition to fixed and automatic scale calibration was added. This includes offsetting the skybox camera to the main camera.
  • Since the source engine has a very specific formula for translating main position to sky position, and other games may also have their own, the formula used for this application is configurable and addable via a switch statement.
  • For general purpose source engine levels, since the scale is an integer but the offset may not be, a corroborated approximation formula is used for auto detection instead of an integer based reversed hyperbolic formula.

Notes

  • Is this the correct way to add properties to a main class? Is there a better way to handle the history side of it?
  • This mostly relies on switch statements, should this be consolidated into a function?

In Progress

  • 3D Skybox instances when 3D Skybox Pathtracing is enabled needs to be re-captured BEFORE the matrix is applied so they do not "re-render" during main view
  • More experimentation with formulas on auto detection modes needs to be done for stability. So far DELTA can be the most problematic with flickering. Currently have VTMB people testing that out

Known Issues

  • Translating the skybox instances into the world can sometimes cause geometry to overlap with the main world geometry. Is there some way to prioritize instances? IE: "cut" one out once they clip with another instance?
  • Skybox lights are not translated
  • Skybox fog is currently uniform, but fog in the skybox may be different than that of the main world in source engine games

@KingDavidW KingDavidW marked this pull request as draft May 8, 2024 04:37
@KingDavidW
Copy link
Contributor Author

This PR needs to be sync'd with the latest branch and enact some of anchorlight's contribution guidelines, so I'm moving this to draft for now

@NV-LL
Copy link

NV-LL commented May 8, 2024

REMIX-3062 for tracking

- Rebased current branch
- Reapplied skybox changes
- Removed excess properties
@KingDavidW
Copy link
Contributor Author

Okay, the branch is sync'd to the current DXVK as of this comment, and the changes have been squashed and simplified to a single commit. I still need to double check some of the guidelines, and there will likely be a follow up commit later today that adds more stability to the delta auto-scale formula, but I think this can start getting code reviewed.

@KingDavidW KingDavidW marked this pull request as ready for review May 11, 2024 21:54
- Moved debug data to Sky Settings
- Improved consistency of Delta
- Made skyscale actually default to the set scale before any formulas are applied on camera cuts
- Added myself to the credits!
- Rebased current branch
- Reapplied skybox changes
- Removed excess properties

Fix a crash on window focus change, by updating the frameId from the dxvk-cs thread instead of main.

See merge request lightspeedrtx/dxvk-remix-nv!835

Review Improvements

- Moved debug data to Sky Settings
- Improved consistency of Delta
- Made skyscale actually default to the set scale before any formulas are applied on camera cuts
- Added myself to the credits!
@KingDavidW
Copy link
Contributor Author

Squashed the commits.. I think I may have gotten one of the merged commits in the crossfire.

@sultim-t-nv
Copy link
Contributor

That's a good idea!

But yes, reconstructing the original world transform might be tricky, as we don't have main camera's transforms yet. We were trying to experiment further, and found that some of the draw call data can be re-ordered until we actually have main camera transform, and then:

skyViewToMainWorld = mainViewToWorld * mainProjectionToView * skyViewToProjection * scale

newObjectToWorld = skyViewToMainWorld * transformData.worldToView * transformData.objectToWorld

So yeah, with re-ordering, we have a solution to have precise and exact matrices (no heuristics, no previous frame data).

@KingDavidW Do you wanna proceed further with this pull request, or we can work together, or you can also delegate this pull request? -- In any case, you will be credited.

@KingDavidW
Copy link
Contributor Author

That's a good idea!

But yes, reconstructing the original world transform might be tricky, as we don't have main camera's transforms yet. We were trying to experiment further, and found that some of the draw call data can be re-ordered until we actually have main camera transform, and then:

skyViewToMainWorld = mainViewToWorld * mainProjectionToView * skyViewToProjection * scale

newObjectToWorld = skyViewToMainWorld * transformData.worldToView * transformData.objectToWorld

So yeah, with re-ordering, we have a solution to have precise and exact matrices (no heuristics, no previous frame data).

@KingDavidW Do you wanna proceed further with this pull request, or we can work together, or you can also delegate this pull request? -- In any case, you will be credited.

Honestly I'd love to go in together because I'm very much interested in learning the whole platform but the time it'd take me to find out what is where may slow down the actual work with it. For example, I have a vague idea where I can shift the order of the rendering, but it'd be some trial and error. That really has been the biggest issue for me. There are a bunch of other features I'd like to add so a guided look would go very far.

Let me know how you want to go through with that 😄

svc-remix-github pushed a commit that referenced this pull request Jun 18, 2024
Enable path-tracing of the skybox instances. Since we can not path-trace more than one camera, moving the instances to the main camera was the route taken.

#73
@KingDavidW
Copy link
Contributor Author

I'm reviving this discussion because the automated scale detection did NOT make it into the merge. Were there any issues with it? The new scaling technique works very well but now maps in Dark Messiah have issues between them and it's unreasonable to tell a person playing the game to just edit the scale value

@KingDavidW
Copy link
Contributor Author

Woops, ignore that push, I was resyncing and it appears it was on a different branch

@Safemilk
Copy link

@sultim-t-nv Did you get any traction on this? It's still the only version of sky rendering that seemed to fix/address everything source games needed.

Right now the sky doesn't light correctly, culls constantly, and also lags behind camera rotation and position movements creating large gaps between skybox meshes and connected level meshes.

Would love to see this branch get merged if possible!

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

Successfully merging this pull request may close these issues.

4 participants