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

Inconsistent layout in aomap_fragment include, compared to i.e. roughnessmap_fragment #28617

Closed
kristiker opened this issue Jun 11, 2024 · 3 comments
Milestone

Comments

@kristiker
Copy link

kristiker commented Jun 11, 2024

Description

I want to modify he occlusion value using my own custom logic. But the ambientOcclusion variable is defined very late on the shader.

I think the aomap_fragment.js file could be simplified like below:

Solution

image

In case of USE_AOMAP being false, the multiplication by 1.0 for the extra operations should be insignificant.

As for computeSpecularOcclusion, I don't know where that would go.

Alternatives

I store temp variable early on, and modify the aomap_fragment chunk in a messy way.

Additional context

I am using this tool: https://ycw.github.io/three-shaderlib-skim/dist/#/latest/standard/fragment

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 11, 2024

I'm afraid it is not clear what you are suggesting. Please do never use screenshots to explain code changes. It's better to implement them in a branch and then share the commit. In this way, we can easier see the diff.

@kristiker
Copy link
Author

kristiker commented Jun 11, 2024

Sure, I might get to it soon. Edit: There you go @Mugen87 5fae13b

kristiker added a commit to kristiker/three.js that referenced this issue Jun 11, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 12, 2024

TBH, I don't see how this commit is an improvement. It makes the code actually harder to read since the AO-related logic now leaks into a chunk where it does not belong.

Sorry, but you need to find a solution for this on app level.

@Mugen87 Mugen87 closed this as completed Jun 12, 2024
@Mugen87 Mugen87 added this to the r166 milestone Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants