Add PBR sample ported from Nadrin/PBR #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
December 17, 2021 16:21
* Implements missing field (de)serialization for ProcessedModel
* Adds basic support for passing extra data into AssetProcessor
- this allows the e.g. the AssimpProcessor to accept as input the
vertex data layout that it should use
* Adds StbImageSharp reference and support for loading .hdr images
* Removes direct Assimp interaction from AnimatedMesh, and replaces it
with use of ProcessedModel
|
OpenGL progress Added cubemap tests to veldrid that the cover the cubemap generation that was previously broken. Passing in OpenGL and Vulkan, failing in D3D11. There appears to be a bug in full texture copy for cubemaps in Vulkan. There's a workaround in the GraphicsDeviceTestBase.GetReadBack(Texture) to work around this for now, but the failing Vulkan test covers it so once that's fixed it can be reverted. specularBRDF_LUT generation is still broken. Looking into this next. |
|
OpenGL working now; was an issue with resource naming in the sample. |
|
Most of the d3d11 test failures were problems with the tests. I was ignoring the existence of row pitches, causing reads of memory outside of the intended bounds of the mapped resource.
Still investigating remaining failures. |
|
Tests passing now Been focusing on OSX. Added https://github.com/etinquis/veldrid-spirv/tree/feature/setshaderversion to the list of dependency changes (this adds the ability to set shader version when cross-compiling, and lets me up the MSL version to 2.1 for some necessary feature support). Metal is looking... metal... MoltenVK is slightly better There were some more veldrid changes necessary to get that far. Will continue to experiment with this. |
…h 6 layers) in places where we need to write to them in a compute shader so as to support d3d11
|
Direct3D11 is no longer completely unsupported, though it's looking stellar |
|
MoltenVK looking better now
|
swapping out shaders with hlsl shader; there's some badness happening between the vert and frag shaders when using the cross-compiled glsl versions
|
D3D11 kinda sorta working I'm swapping out for the hlsl shader version in case of d3d. I have yet to figure out whats causing the cross-compiled shaders to look like this |
|
Now works without the hack 👍 |
|
Metal looking better now, but still scuffed |
|
Well, I think I know what the problem is, but not quite sure where it's coming from yet. Metal is seeing this fragment shader (translated from pbr_fs.glsl) [snip]
fragment main0_out main0(main0_in in [[stage_in]], constant _164& _166 [[buffer(0)]], constant _200& _202 [[buffer(1)]], texture2d<float> _177 [[texture(0)]], texture2d<float> _185 [[texture(1)]], texture2d<float> _191 [[texture(2)]], texture2d<float> _215 [[texture(3)]], texturecube<float> _349 [[texture(4)]], texturecube<float> _373 [[texture(5)]], texture2d<float> _386 [[texture(6)]], sampler _61 [[sampler(0)]])
{
main0_out out = {};
float3x3 _226 = {};
_226[0] = in.m_226_0;
_226[1] = in.m_226_1;
_226[2] = in.m_226_2;
float4 _426 = _177.sample(_61, in.m_179);
float3 _183 = _426.xyz;
float4 _433 = _185.sample(_61, in.m_179);
float4 _440 = _191.sample(_61, in.m_179);
float _195 = _440.x;
float3 _212 = normalize(_202._m1.xyz - in.m_209);
float4 _447 = _215.sample(_61, in.m_179);
float3 _230 = normalize(_226 * normalize((_447.xyz * 2.0) - float3(1.0)));
[snip]This is translated from this: [snip]
void main()
{
float alpha = clamp(viewProjectionMatrix[0][0], 0.0f, 1.0f) + 1.0f; // access something in TransformUniforms so it isn't omitted in msl fragment shader
// Sample input textures to get shading model params.
vec3 albedo = texture(albedoTexture, vin_texcoord).rgb;
float metalness = texture(metalnessTexture, vin_texcoord).r;
float roughness = texture(roughnessTexture, vin_texcoord).r;
// Outgoing light direction (vector from world-space fragment position to the "eye").
vec3 Lo = normalize(eyePosition.xyz - vin_pos);
// Get current fragment's normal and transform to world space.
vec3 N = normalize(2.0 * texture(normalTexture, vin_texcoord).rgb - 1.0);
N = normalize(vin_tangentBasis * N);
[snip]The issue appears to be the texture ( float4 _447 = _215.sample(_61, in.m_179);
float3 _230 = normalize(_226 * normalize((_447.xyz * 2.0) - float3(1.0)));According to the MSL shader, So it seems something in the process is getting the textures confused. At the moment it appears to me to be something either in shaderc or spirv-cross. |
…generated; it appears that texture binding order isn't respected somewhere which causes the code-side bindings not to match the MSL shader-side order
|
Rearranging all the bindings to appease the cross compilation gods appears to work around the problem. It's ALIVE! D3D11 OpenGL Vulkan Metal |
3 tasks
etinquis
commented
Dec 23, 2021
| @@ -55,7 +55,7 @@ public void Run() | |||
| #if DEBUG | |||
| options.Debug = true; | |||
| #endif | |||
| _gd = VeldridStartup.CreateGraphicsDevice(_window, options, GraphicsBackend.Direct3D11); | |||
| _gd = VeldridStartup.CreateGraphicsDevice(_window, options, GraphicsBackend.OpenGL); | |||
etinquis
commented
Dec 23, 2021
| @@ -1,6 +1,6 @@ | |||
| Microsoft Visual Studio Solution File, Format Version 12.00 | |||
| # Visual Studio 15 | |||
| VisualStudioVersion = 15.0.27130.2010 | |||
| # Visual Studio Version 17 | |||
etinquis
commented
Dec 23, 2021
|
|
||
| void main() | ||
| { | ||
| float alpha = clamp(viewProjectionMatrix[0][0], 0.0f, 1.0f) + 1.0f; // access something in TransformUniforms so it isn't omitted in msl fragment shader |
There was a problem hiding this comment.
double-check if this is still necessary
etinquis
commented
Dec 23, 2021
| layout(location=3) in vec3 bitangent; | ||
| layout(location=4) in vec2 texcoord; | ||
|
|
||
| #if VULKAN |
etinquis
commented
Dec 23, 2021
| return textureSize(samplerCube(tex, textureSampler), lod); | ||
| } | ||
|
|
||
| #define PARAM_LEVEL pushConstants.level |
There was a problem hiding this comment.
Since the #define VULKAN that necessitated these has been removed, they can be inlined.
etinquis
commented
Dec 23, 2021
|
|
||
| if (GraphicsDevice.GetVulkanInfo(out var info)) | ||
| { | ||
| info.TransitionImageLayout(specularEnvironmentMapTexture, |
There was a problem hiding this comment.
I don't think these are supposed to be necessary. I don't think I've fixed anything yet that would make them redundant but there may be something yet to fix in the library re: transitioning states after compute shaders are run?
|
Now running on Android in GLES (Samsung Galaxy A5) Can't test on device with Vulkan. VulkanUtils.TryLoadVulkan |
|
Hmm I'm quite certain I've gotten a SIGABRT error with an very similar log but in net6-android. I have a big suspicion that NativeLibraryLoader is outdated for net6 and not finding the right things. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sample is based on https://github.com/Nadrin/PBR/.
Yet to do:
Original text:
I've focused on Vulkan mainly thus far. There were some cubemap-related fixes required in the veldrid repo, so it will need to be locally built off https://github.com/etinquis/veldrid/tree/fix/pbr-cubemap-fixes and updated here. I will submit a PR to the veldrid repo once I'm done.
It fails in Direct3D11 with
Compilation failed: RWTextureCube does not exist in HLSL, presumably due to use of a generated cube map.The compute shaders and cube maps are misbehaving in OpenGL at the moment:
