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

Refactor: Removed CachedVisibilityMap to use previous frame data, fixed another crash. #3

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Jaskowicz1
Copy link
Owner

@Jaskowicz1 Jaskowicz1 commented Feb 13, 2025

This PR removes the CachedVisibilityMap to use the previous frame's data to check if objects should still be occluded or not.

This saves, at a minimum, 188 bytes of memory. In a scene with 1000 objects (this is quite easy to hit, reminder, each Static Mesh inside of a blueprint is another object), this is roughly ((9*1000)+188)=9188 bytes (9.1KBs) of memory (9 is the size of FPrimitiveSceneID + bool), actual saved memory might be smaller or bigger, depends on usage.

This PR also stops sending every object in the FrameResult's VisibilityMap to the GameThread if it hasn't changed. This improves the frame time quite a lot in a heavy populated world (I've seen about a ~0.2-0.4ms improvement in my test scene), as we no longer send a task for every object in the world, only ones that need to be occluded (and if their visibility doesn't match what the occluder wants).

Closes #1

@Jaskowicz1 Jaskowicz1 self-assigned this Feb 13, 2025
@Jaskowicz1 Jaskowicz1 marked this pull request as draft February 13, 2025 09:26
@Jaskowicz1
Copy link
Owner Author

Draft PR whilst I work out why the crash exists.

Source/SoftwareOC/Private/OcclusionSceneView.cpp Outdated Show resolved Hide resolved
Source/SoftwareOC/Private/SoftwareOCSubsystem.cpp Outdated Show resolved Hide resolved
Source/SoftwareOC/Private/OcclusionSceneView.cpp Outdated Show resolved Hide resolved
Source/SoftwareOC/Private/OcclusionSceneView.cpp Outdated Show resolved Hide resolved
Source/SoftwareOC/Public/Unreal/OcclusionMeshData.h Outdated Show resolved Hide resolved
Source/SoftwareOC/Public/Unreal/FOcclusionFrameResults.h Outdated Show resolved Hide resolved
Source/SoftwareOC/Private/OcclusionSceneView.cpp Outdated Show resolved Hide resolved
Source/SoftwareOC/Private/SoftwareOCSubsystem.cpp Outdated Show resolved Hide resolved
@Jaskowicz1
Copy link
Owner Author

Jaskowicz1 commented Feb 13, 2025

Figured out the cause of the Crash in Packaged Builds.

IndexBuffer.AccessStream16() returns nullptr as any mesh passed through to FOcclusionMeshData doesn't have bAllowCPUAccess set to true.

This defaults to false on all Static Meshes. Meaning cooked builds cannot access data from any IndexBuffer on a static mesh.

All meshes would have to do this:
image

How wonderful...

@Jaskowicz1
Copy link
Owner Author

Whilst people could in theory make all their static meshes change to support this and whatnot, it'll increase RAM but decrease VRAM.

This may be fine tbh (especially for platforms that have shared RAM because you're not actually losing anything, it's just now in a different place).

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.

Crash in ProcessOccluderGeom when loading a new world in an exported game.
1 participant