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

Decode normal maps on export instead of trying to access pixels. #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hybridherbst
Copy link

Reading raw texture pixels from disk is, in general, not a good idea given the wealth of texture formats Unity supports internally. A Texture2D could come from anywhere -

  • embedded in an FBX file
  • PSD file
  • RenderTexture
  • stored as .asset file on disk
  • procedurally generated, only living in memory

One case where that is a problem in the sketchfab exporter is NormalMap exporting - their pixels would only be correct if the normal on disk is already in tangent space (sometimes, but rather rarely the case).

A solution (for the normal maps, but a similar approach would work in general) is to decode the actual texture and convert the necessary data to a PNG or JPEG file which is then stored for glTF usage.
A shader is used to convert into the right encoded normals format (tangent space).

This works for all normal maps, no matter how they were created (greyscale/bumpmap/RenderTex)...

…s for all normal maps, no matter how they were created (greyscale/bumpmap/tangent space...)
@AurL
Copy link

AurL commented Sep 11, 2018

Hi @soraryu,

Thanks a lot for this PR and the explanations 👍

Please note that we are not active anymore on this code, as we switched to https://github.com/sketchfab/UnityGLTF.
I'll take a look at this PR ASAP and probably report and adapt these changes to the new repository.

@hybridherbst
Copy link
Author

hybridherbst commented Sep 11, 2018

I see. Would be great if you could mention that on this repo's Readme – otherwise I would have spent time on that other code directly ...

From a quick look at the code, looks like the other repo has similar problems with requiring all textures to be living in the AssetDatabase for export.

Edit: the "new" repo is way behind the official Khronos Group repo, right?

@AurL
Copy link

AurL commented Sep 12, 2018

Yes, I would make sense to mention this in the Readme (I'll add a note), and you're right, we have the same problems with the new code.

We actually diverged from the official Khronos Group repo since our only goal was to build a Sketchfab to Unity plugin around glTF. The structure of UnityGLTF was much more interesting and maintainable so we moved both our importer and our exporter.

The official repo has been updated since we worked on our version so they probably went further that us (as we stopped when we had enough features to support for our usecase).

@hybridherbst
Copy link
Author

hybridherbst commented Jan 22, 2019

@AurL Note that both versions linked at https://sketchfab.com/exporters/unity, the AssetStore version and the github version, are this one (the one you say is deprecated).

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.

2 participants