Skip to content

Adding support for override of Dots instance ID #6614

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 16, 2021
Merged

Adding support for override of Dots instance ID #6614

merged 2 commits into from
Dec 16, 2021

Conversation

kecho
Copy link
Contributor

@kecho kecho commented Dec 14, 2021 •
edited
Loading

Purpose of this PR

If we are gonna do visibility rendering, we will require to convert the visibility sample to a Dots instance id in order to grab material data.
This approach makes it so its easy to bootstrap the dots instance id as soon as the pixel shader starts.

Doing some codegen analysis we can see that there is only 1 instruction change, which just uses a register to load from a constant buffer (tested on FXC)
image

Testing status

Tested on the gpudriven branch, with and without deferred materials.
Waiting on Yamato

Comments to reviewers

Change was extracted from this PR #6461

@kecho kecho requested a review from JussiKnuuttila December 14, 2021 18:30
@github-actions
Copy link

github-actions bot commented Dec 14, 2021 •
edited
Loading

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/jobDefinition/.yamato%252F_abv.yml%2523all_project_ci_trunk
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@github-actions
Copy link

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

@kecho kecho force-pushed the graphics/override-dots-instance-id branch from 27b5ff1 to 7a00371 Compare December 15, 2021 14:19
kecho added 2 commits December 15, 2021 14:56
@kecho
** Adding override of dots instance ID for dots instancing **
8b61a46 
* Correcting issues

* Undoing unused API

* Adding redundant conditional
@kecho
Preserving sampling of index 0 of dots instancing cbuffer: this data …
5353559 
…must remain in index 0 of the batch
@kecho kecho force-pushed the graphics/override-dots-instance-id branch from 7a00371 to 5353559 Compare December 15, 2021 20:55
@sebastienlagarde sebastienlagarde merged commit c394a0c into master Dec 16, 2021
@sebastienlagarde sebastienlagarde deleted the graphics/override-dots-instance-id branch December 16, 2021 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JussiKnuuttila JussiKnuuttila JussiKnuuttila approved these changes

+1 more reviewer

@sebastienlagarde sebastienlagarde sebastienlagarde approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

HDRP SRP

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kecho @sebastienlagarde @JussiKnuuttila