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

Reworked TAA PR by HifiExperiments #501

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

Reworked TAA PR by HifiExperiments #501

wants to merge 35 commits into from

Conversation

ksuprynowicz
Copy link
Member

This one needs a lot of testing both in desktop mode and VR to make sure nothing got broken.
TAA quality is a lot nicet than in main branch.

@ksuprynowicz ksuprynowicz added needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested labels Jul 2, 2023
@ksuprynowicz
Copy link
Member Author

It looks like I broke antialiasing mode toggle while rebasing

@ksuprynowicz ksuprynowicz added the work in progress Do not merge yet label Jul 2, 2023
@ksuprynowicz
Copy link
Member Author

It looks like against skybox only top and right edge of the contour is antialiased properly.
image

@ksuprynowicz
Copy link
Member Author

With this version some ghosting occurs when avatar is moving. I wonder how difficult it would be to add armature-related movement to velocity buffer?

@ksuprynowicz
Copy link
Member Author

Sadly it also breaks bloom. Black square with four small white squares often appears in places where bloom occurs.
image

@HifiExperiments
Copy link
Member

With this version some ghosting occurs when avatar is moving. I wonder how difficult it would be to add armature-related movement to velocity buffer?

yeah, the velocity buffer currently just uses the entity transform to calculate velocity so this breaks for skinned meshes. we'd need all information necessary to calculate skinning for the previous frame. the key on the shader side is this block here:

<@if HIFI_USE_DEFORMED or HIFI_USE_DEFORMEDDQ@>

we'd need an evalPrevMeshDeformer that does the same general thing but calculates prevPositionMS using the previous frame's buffers and uses that to get _prevPositionCS (instead of transformModelToWorldEyeClipPosAndPrevClipPos)

shouldn't be that hard but I forget how the cluster buffers work on the c++ side. probably need two and then ping pong between them

@HifiExperiments
Copy link
Member

Sadly it also breaks bloom. Black square with four small white squares often appears in places where bloom occurs. image

hmmm I don't remember this being an issue all those years ago...maybe renderdoc would help if you could catch this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested NLnet work in progress Do not merge yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants