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

Improve and simplify shadercode #42

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

Improve and simplify shadercode #42

wants to merge 4 commits into from

Conversation

Vipitis
Copy link
Collaborator

@Vipitis Vipitis commented Jan 28, 2025

Next part of #30

  • The shader code construction (vertex, builtin, fragment) is now handled via a public method so it can be used externally.
  • Headers for inputs (bindings, samplers) are now dynamic via the input class. This will extend easier to more input channels like cubemaps and 3D samplers.
  • dropped the glsl aliases (i_time, etc) only supporting Shadertoy syntax now (iTime). might help with GLSL keywords used as identifiers #22
  • Uniform formats are also now documented in the README.

this removes some of the massive strings at the top of the file and might also reduce some overhead (which the compiler likely took care off anyway).

@Vipitis
Copy link
Collaborator Author

Vipitis commented Jan 28, 2025

found the diffs and there is indeed a difference. I will try to look at this closer tomorrow.
Since the one shader we test with screenshots is written in wgsl the only real culprit could be the simplified vertex stage or fragment stage.

@Vipitis
Copy link
Collaborator Author

Vipitis commented Jan 30, 2025

so after a few days distraction, I got back to looking at pixels and stepping through assembly. A lot of back and force and testing between different machines etc. I believe the changes introduced here remove one multiplication and replace it with a subtraction, hence causing tiny differences in the execution path and therefore floating point division. In some ways this is related to #38 as doing the flip in post also gives me a tiny diff.

so I see two options:

  • revert the simplification
  • accept this as more accurate and replace the screenshot (manually?)

in light of #31 there should be more screenshot testing, to include GLSL samples. But there are testing improvements from wgpu-py/pygfx that I wanted to look at too.

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.

1 participant