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

VideoCommon: add a pixel shader asset #12009

Merged
merged 1 commit into from Jul 3, 2023

Conversation

iwubcode
Copy link
Contributor

@iwubcode iwubcode commented Jun 29, 2023

Here's an asset that describes a pixel shader. I've based my design somewhat off of Unity game engine but I believe other modern graphics renders follow something somewhat similar.

This shader asset contains the shader code for the shader itself, as well as a description of the inputs which match what we want to expose from the shader code. The inputs are just the type (and eventually the default data value), not the actual data that is being held. This makes these assets highly reusable. At the moment, I've only exposed a 2D texture sampler type. But in the future there will be uniform support for colors/integers/etc and likely will also include inputs for cube-maps, 3d textures, and named render targets.

With the data type defined, where does the actual data come from? That is the job of the material asset. That is coming in a subsequent review.

I will be using this in a series of future reviews.

@iwubcode

This comment was marked as outdated.

@iwubcode
Copy link
Contributor Author

I guess I should add. There are many types of shaders. Just from a graphics renderer alone we have:

  • Vertex
  • Pixel
  • Compute
  • Geometry
  • Tessellation

(if you want to get technical, I believe there are some for Ray Tracing as well but I haven't delved into that)

Dolphin supports the top 4 shader types. I am not sure how many custom variants I want to support.

Unity chose to group the vertex/pixel shader together. Presumably because of how linked they are. Not sure if I want to follow that path.

What is more...we might want to distinguish between the same type based on usage (a custom graphics draw pixel shader is much different than a custom post processing pixel shader!)

I'll be honest, I haven't wrapped my head around all this. This is just a small splash in a complicated topic. I had to start somewhere!

@AdmiralCurtiss
Copy link
Contributor

The code seems fine but I can't say if this is a sensible design or not. If you want me to merge this I will.

@iwubcode
Copy link
Contributor Author

iwubcode commented Jul 2, 2023

Thanks for looking it over @AdmiralCurtiss . Anything you're particularly concerned about or any ideas for a better design?

I too don't know if this will stand the test of time. This is (part of) what allows for the shader editing I showed off a few weeks ago so at least it has a neat application.

@AdmiralCurtiss
Copy link
Contributor

Nothing in particular, no. I just am not particularly familiar with loaded asset management stategies.

@iwubcode
Copy link
Contributor Author

iwubcode commented Jul 2, 2023

I just am not particularly familiar with loaded asset management stategies.

I am not either. I tried to look up details but didn't find much.

If you feel comfortable merging it, please do as it is pretty critical for the features I'm working on (I have one more related PR coming after this is merged). But if you don't, I understand. I can keep looking for more details.

@Pokechu22
Copy link
Contributor

Unity chose to group the vertex/pixel shader together. Presumably because of how linked they are. Not sure if I want to follow that path.

I feel like that path is a weird choice as it makes it hard to use a geometry shader. Granted, for Dolphin, we generate arbitrary vertex shaders and arbitrary pixel shaders and need to mix them together, but for normal games you probably have a small set of them. But for Dolphin we'd probably want to keep them separate so that each separate component can be added to the generated shaders.

@AdmiralCurtiss AdmiralCurtiss merged commit 0366122 into dolphin-emu:master Jul 3, 2023
14 checks passed
@iwubcode iwubcode deleted the shader_asset branch July 3, 2023 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants