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

Fix socket indices of Principled BSDF node #2606

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

MoritzBrueckner
Copy link
Collaborator

@MoritzBrueckner MoritzBrueckner commented Sep 29, 2022

Apparently, Blender changed the Principled BSDF sockets a second time between 3.0-3.3 without a note in the release notes, at least I can't find it anywhere.

This PR fixes parsing normals and emission values. Emission is still pretty broken since we have no space left in the gbuffers to save any emission color (#1774), and I don't think that we can fix it without a 3rd render target. However, this would mean that either the 3rd render target would be bound all the time during mesh rendering, or we'd need to sort the meshes each frame to prevent to many context switches and maybe also bandwidth issues (I'm not sure what affects the bandwidth in what way, more specifically where it matters). @ luboslenco do you have an opinion on this? I think the current emission support is a problem, but I can also understand if you say that improving it would waste too much performance.

Here's a comparison between Blender and Armory, the cubes on the left use the Principled BSDF node, and the cubes on the right use the emission node. If you want I can add this file to the armory_calibration repo, but as you can see it doesn't completely work yet:

grafik


Off-topic: what exactly is rp_render_to_texture supposed to do/solve? There's a bug with transparency if this flag is disabled, I found two possible fixes and my decision about what fix to choose depends on what this flag does.

@MoritzBrueckner MoritzBrueckner added Release Notes: Fixes A pull request that fixes something. Used to generate release notes. Blender 3.3 LTS Issues and PRs linked to Blender 3.3 LTS support labels Sep 29, 2022
@e2002e
Copy link
Contributor

e2002e commented Oct 3, 2022

Hi, i just read that. I'm not familiar with GPU programming, yet having one more texture in the shader should cost nothing. I'm quite sure about that.

@luboslenco luboslenco merged commit 34bf363 into armory3d:main Oct 4, 2022
@luboslenco
Copy link
Member

and I don't think that we can fix it without a 3rd render target

If there is no way around it we can go with the 3rd render target. Ideally it would toggle itself on/off if emission use is detected in a project? For a start let's just attach it during mesh rendering.

If you want I can add this file to the armory_calibration repo

Yeah, that might come in handy.

what exactly is rp_render_to_texture supposed to do/solve?

When rp_render_to_texture is disabled it should render directly to the screen framebuffer, instead of our own allocated texture / render target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blender 3.3 LTS Issues and PRs linked to Blender 3.3 LTS support Release Notes: Fixes A pull request that fixes something. Used to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants