Add a Specular Glossiness to Metallic Roughness conversion step to workflows. #36
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 6 commits
June 25, 2018 13:48
Update driectxtex to 2018.6.1.2
Add abilibty to apply Draco mesh compression. Modify SerializeBinary to allow for BufferView's that we don't understand natively.
…rkflows. Refactored texture loading and packing utils into generic textures utils. Resolved packing, resizing and compressing texture issues with sRGB and alpha channels. Update draco build to allow for ARM.
robertos
reviewed
Jul 7, 2018
|
|
||
| // 2. Texture Compression | ||
| convertedDoc = GLTFTextureCompressionUtils::CompressAllTexturesForWindowsMR(streamReader, document, tempDirectoryA, maxTextureSize, false /* retainOriginalImages */); |
There was a problem hiding this comment.
Nice catch! This is a big bug.
robertos
reviewed
Jul 7, 2018
| @@ -68,5 +71,14 @@ | |||
| <ClCompile Include="src\GLBtoGLTF.cpp"> | |||
| <Filter>src</Filter> | |||
| </ClCompile> | |||
| <ClCompile Include="GLTFMeshCompressionUtils.cpp"> | |||
There was a problem hiding this comment.
This should be in the src folder
robertos
reviewed
Jul 7, 2018
| resultDocument = ConvertMaterial(streamReader, resultDocument, material, outputDirectory); | ||
| } | ||
|
|
||
| resultDocument.extensionsUsed.erase(KHR::Materials::PBRSPECULARGLOSSINESS_NAME); |
There was a problem hiding this comment.
does this fail if the extension is not there?
There was a problem hiding this comment.
No idempotent in that case.
robertos
reviewed
Jul 7, 2018
| @@ -261,7 +260,6 @@ void GLTFTextureCompressionUtils::CompressImage(DirectX::ScratchImage& image, Te | |||
| break; | |||
| case TextureCompression::BC7_SRGB: | |||
| compressionFormat = DXGI_FORMAT_BC7_UNORM_SRGB; | |||
| compressionFlags |= DirectX::TEX_COMPRESS_SRGB_IN; | |||
There was a problem hiding this comment.
Because I changed to handle the SRGB conversion during load. Math needs to be done on linear values so I just made it the default.
robertos
approved these changes
Jul 7, 2018
|
Did you have a collection of test assets that are typically used? |
|
I start with all the gltf samples, that's a quick and easy litmus test. I also use a few assets from actual apps, and more recently I've been using a few from Poly, Remix3D and some environments. If you need help I can send some assets on Monday.
Roberto
…________________________________
From: Jamie Marconi <notifications@github.com>
Sent: Saturday, July 7, 2018 1:09:28 PM
To: Microsoft/glTF-Toolkit
Cc: Roberto Sonnino; Comment
Subject: Re: [Microsoft/glTF-Toolkit] Add a Specular Glossiness to Metallic Roughness conversion step to workflows. (#36)
Did you have a collection of test assets that are typically used?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2FglTF-Toolkit%2Fpull%2F36%23issuecomment-403240082&data=02%7C01%7Crobertos%40microsoft.com%7C04f43b40f8ed4a00e7db08d5e4458c0c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636665909710895142&sdata=PeeD%2BaOCwIJaCpB2WnCyRdCgRwddH8BSGq3LaLCZ6eQ%3D&reserved=0>, or mute the thread<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAl8Brq4ALwcProISSai9VpgPwJWbr_qks5uERV4gaJpZM4VGK_0&data=02%7C01%7Crobertos%40microsoft.com%7C04f43b40f8ed4a00e7db08d5e4458c0c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636665909710895142&sdata=jmjt7PoZxr4rhKp1AevmfHgb4%2FSGoc5sggicEXdXK7w%3D&reserved=0>.
|
Switch draco compression to an option; it makes loading slower.
|
@najadojo Let me know if I can help you to complete this PR - lots of goodness here :) |
|
I haven't completed testing these changes yet and I've been moved on to other work... I'll try and finish that up in my free time. |
|
@Dharnidharka this last commit will fix #39, using mesh compressed assets as input. You can grab a release binary on the artifacts page. |
|
@robertos this PR is ready for merging. |
|
@najadojo Thank you for the fix :) |
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.
Add a Specular Glossiness to Metallic Roughness conversion step to workflows.
Refactored texture loading and packing utils into generic textures utils.
Resolved packing, resizing and compressing texture issues with sRGB and alpha channels.
Update draco build to allow for ARM.
Also includes #34 and #35.