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.

Sign up for GitHub

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

Jump to bottom

gltfio: use tangent space mesh #7566

Closed
wants to merge 2 commits into from
Closed

gltfio: use tangent space mesh #7566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
872d243
gltfio: use tangent space mesh
poweifeng Jan 24, 2024
e2db352
Merge branch 'main' into pf/gltfio-tangent-space-mesh
poweifeng Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEW_RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ for next branch cut* header.
appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md).

## Release notes for next branch cut

- gltfio: support for fallback flat-shading and mikktspace tangents
8 changes: 8 additions & 0 deletions android/gltfio-android/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,17 @@ set(GLTFIO_SRCS
${GLTFIO_DIR}/src/TangentsJob.cpp
${GLTFIO_DIR}/src/TangentsJob.h
${GLTFIO_DIR}/src/UbershaderProvider.cpp
${GLTFIO_DIR}/src/Utility.cpp
${GLTFIO_DIR}/src/Utility.h
${GLTFIO_DIR}/src/Wireframe.cpp
${GLTFIO_DIR}/src/Wireframe.h
${GLTFIO_DIR}/src/downcast.h
${GLTFIO_DIR}/src/extended/AssetLoaderExtended.cpp
${GLTFIO_DIR}/src/extended/AssetLoaderExtended.h
${GLTFIO_DIR}/src/extended/ResourceLoaderExtended.cpp
${GLTFIO_DIR}/src/extended/ResourceLoaderExtended.h
${GLTFIO_DIR}/src/extended/TangentsJobExtended.cpp
${GLTFIO_DIR}/src/extended/TangentsJobExtended.h

src/main/cpp/Animator.cpp
src/main/cpp/AssetLoader.cpp
Expand Down
2 changes: 1 addition & 1 deletion libs/geometry/include/geometry/TangentSpaceMesh.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class TangentSpaceMesh {
* Destroy the mesh object
* @param mesh A pointer to a TangentSpaceMesh ready to be destroyed
*/
static void destroy(TangentSpaceMesh* mesh) noexcept;
static void destroy(TangentSpaceMesh* mesh) noexcept;

/**
* Move constructor
Expand Down
78 changes: 61 additions & 17 deletions libs/geometry/src/MikktspaceImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <math/mat3.h>
#include <math/norm.h>
#include <utils/Panic.h>

#include <meshoptimizer.h>
#include <mikktspace/mikktspace.h>
Expand Down Expand Up @@ -82,7 +83,7 @@ void MikktspaceImpl::setTSpaceBasic(SMikkTSpaceContext const* context, float con
// TODO: packTangentFrame actually changes the orientation of b.
quatf const quat = mat3f::packTangentFrame({t, b, n}, sizeof(int32_t));

auto output = wrapper->mOutputData;
auto& output = wrapper->mOutputData;
auto const& EMPTY_ELEMENT = wrapper->EMPTY_ELEMENT;

size_t const outputCurSize = output.size();
Expand All @@ -92,15 +93,15 @@ void MikktspaceImpl::setTSpaceBasic(SMikkTSpaceContext const* context, float con

uint8_t* cursor = output.data() + outputCurSize;

*((float3*) (cursor)) = pos;
*((float2*) (cursor + 12)) = uv;
*((quatf*) (cursor + 20)) = quat;
*((float3*) (cursor + POS_OFFSET)) = pos;
*((float2*) (cursor + UV_OFFSET)) = uv;
*((quatf*) (cursor + TBN_OFFSET)) = quat;

cursor += 36;
for (auto [attribArray, attribStride, attribSize]: wrapper->mInputAttribArrays) {
uint8_t const* input = pointerAdd(attribArray, vertInd, attribStride);
memcpy(cursor, input, attribSize);
cursor += attribSize;
cursor += BASE_OUTPUT_SIZE;
for (auto const& inputAttrib: wrapper->mInputAttribArrays) {
uint8_t const* input = pointerAdd(inputAttrib.data, vertInd, inputAttrib.stride);
memcpy(cursor, input, inputAttrib.size);
cursor += inputAttrib.size;
}
}

Expand All @@ -125,7 +126,12 @@ MikktspaceImpl::MikktspaceImpl(const TangentSpaceMeshInput* input) noexcept
for (auto attrib: input->getAuxAttributes()) {
size_t const attribSize =input->attributeSize(attrib);
mOutputElementSize += attribSize;
mInputAttribArrays.push_back({input->raw(attrib), input->stride(attrib), attribSize});
mInputAttribArrays.push_back({
.attrib = attrib,
.data = input->raw(attrib),
.stride = input->stride(attrib),
.size = attribSize,
});
}
mOutputData.reserve(mFaceCount * 3 * mOutputElementSize);

Expand Down Expand Up @@ -156,12 +162,12 @@ void MikktspaceImpl::run(TangentSpaceMeshOutput* output) noexcept {
genTangSpaceDefault(&context);

size_t oVertexCount = mOutputData.size() / mOutputElementSize;

std::vector<unsigned int> remap(oVertexCount);
std::vector<unsigned int> remap;
remap.resize(oVertexCount);
size_t vertexCount = meshopt_generateVertexRemap(remap.data(), NULL, remap.size(),
mOutputData.data(), oVertexCount, mOutputElementSize);

std::vector<IOVertex> newVertices(vertexCount);
std::vector<uint8_t> newVertices(vertexCount * mOutputElementSize);
meshopt_remapVertexBuffer((void*) newVertices.data(), mOutputData.data(), oVertexCount,
mOutputElementSize, remap.data());

Expand All @@ -172,10 +178,48 @@ void MikktspaceImpl::run(TangentSpaceMeshOutput* output) noexcept {
float2* outUVs = output->uvs().allocate(vertexCount);
quatf* outQuats = output->tspace().allocate(vertexCount);

for (size_t i = 0; i < vertexCount; ++i) {
outPositions[i] = newVertices[i].position;
outUVs[i] = newVertices[i].uv;
outQuats[i] = newVertices[i].tangentSpace;
uint8_t* const verts = newVertices.data();

std::vector<std::tuple<AttributeImpl, void*, size_t>> attributes;

for (auto const& inputAttrib: mInputAttribArrays) {
auto const attrib = inputAttrib.attrib;
switch(attrib) {
case AttributeImpl::UV1:
attributes.push_back(
{attrib, output->data<DATA_TYPE_UV1>(attrib).allocate(vertexCount),
inputAttrib.size});
break;
case AttributeImpl::COLORS:
attributes.push_back(
{attrib, output->data<DATA_TYPE_COLORS>(attrib).allocate(vertexCount),
inputAttrib.size});
break;
case AttributeImpl::JOINTS:
attributes.push_back(
{attrib, output->data<DATA_TYPE_JOINTS>(attrib).allocate(vertexCount),
inputAttrib.size});
break;
case AttributeImpl::WEIGHTS:
attributes.push_back(
{attrib, output->data<DATA_TYPE_WEIGHTS>(attrib).allocate(vertexCount),
inputAttrib.size});
break;
default:
PANIC_POSTCONDITION("Unexpected attribute=%d", (int) inputAttrib.attrib);
}
}

for (size_t i = 0, vi=0; i < vertexCount; ++i, vi+=mOutputElementSize) {
outPositions[i] = *((float3*) (verts + vi + POS_OFFSET));
outUVs[i] = *((float2*) (verts + vi + UV_OFFSET));
outQuats[i] = *((quatf*) (verts + vi + TBN_OFFSET));

uint8_t* cursor = verts + vi + BASE_OUTPUT_SIZE;
for (auto const [attrib, outdata, size] : attributes) {
memcpy((uint8_t*) outdata + (i * size), cursor, size);
cursor += size;
}
}

output->vertexCount = vertexCount;
Expand Down
18 changes: 16 additions & 2 deletions libs/geometry/src/MikktspaceImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,14 @@ class MikktspaceImpl {

private:
// sizeof(float3 + float2 + quatf) (pos, uv, tangent)
static constexpr size_t const BASE_OUTPUT_SIZE = 36;
static constexpr size_t const FLOAT3_SIZE = sizeof(float3);
static constexpr size_t const FLOAT2_SIZE = sizeof(float2);
static constexpr size_t const QUATF_SIZE = sizeof(quatf);

static constexpr size_t const POS_OFFSET = 0;
static constexpr size_t const UV_OFFSET = FLOAT3_SIZE;
static constexpr size_t const TBN_OFFSET = FLOAT3_SIZE + FLOAT2_SIZE;
static constexpr size_t const BASE_OUTPUT_SIZE = FLOAT3_SIZE + FLOAT2_SIZE + QUATF_SIZE;

static int getNumFaces(SMikkTSpaceContext const* context) noexcept;
static int getNumVerticesOfFace(SMikkTSpaceContext const* context, int const iFace) noexcept;
Expand All @@ -74,7 +81,14 @@ class MikktspaceImpl {
size_t const mUVStride;
uint8_t const* mTriangles;
bool mIsTriangle16;
std::vector<std::tuple<uint8_t const*, size_t, size_t>> mInputAttribArrays;

struct InputAttribute {
AttributeImpl attrib;
uint8_t const* data;
size_t stride;
size_t size;
};
std::vector<InputAttribute> mInputAttribArrays;

size_t mOutputElementSize;
std::vector<uint8_t> mOutputData;
Expand Down
19 changes: 13 additions & 6 deletions libs/geometry/src/TangentSpaceMesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,14 @@ void lengyelMethod(TangentSpaceMeshInput const* input, TangentSpaceMeshOutput* o
output->triangles16.borrow(triangles16);
}

void auxImpl(TangentSpaceMeshInput::AttributeMap& attributeData, AttributeImpl attribute,
InData data, size_t stride) noexcept {
attributeData[attribute] = {
data,
stride ? stride : TangentSpaceMeshInput::attributeSize(attribute),
};
}

} // anonymous namespace

Builder::Builder() noexcept
Expand All @@ -527,27 +535,27 @@ Builder& Builder::vertexCount(size_t vertexCount) noexcept {
}

Builder& Builder::normals(float3 const* normals, size_t stride) noexcept {
mMesh->mInput->attributeData[AttributeImpl::NORMALS] = { normals, stride };
auxImpl(mMesh->mInput->attributeData, AttributeImpl::NORMALS, normals, stride);
return *this;
}

Builder& Builder::uvs(float2 const* uvs, size_t stride) noexcept {
mMesh->mInput->attributeData[AttributeImpl::UV0] = { uvs, stride };
auxImpl(mMesh->mInput->attributeData, AttributeImpl::UV0, uvs, stride);
return *this;
}

Builder& Builder::positions(float3 const* positions, size_t stride) noexcept {
mMesh->mInput->attributeData[AttributeImpl::POSITIONS] = { positions, stride };
auxImpl(mMesh->mInput->attributeData, AttributeImpl::POSITIONS, positions, stride);
return *this;
}

Builder& Builder::tangents(float4 const* tangents, size_t stride) noexcept {
mMesh->mInput->attributeData[AttributeImpl::TANGENTS] = { tangents, stride };
auxImpl(mMesh->mInput->attributeData, AttributeImpl::TANGENTS, tangents, stride);
return *this;
}

Builder& Builder::aux(AuxAttribute attribute, InData data, size_t stride) noexcept {
mMesh->mInput->attributeData[static_cast<AttributeImpl>(attribute)] = { data, stride };
auxImpl(mMesh->mInput->attributeData, static_cast<AttributeImpl>(attribute), data, stride);
return *this;
}

Expand Down Expand Up @@ -646,7 +654,6 @@ size_t TangentSpaceMesh::getVertexCount() const noexcept {

void TangentSpaceMesh::getPositions(float3* positions, size_t stride) const {
auto inPositions = mInput->positions();

ASSERT_PRECONDITION(inPositions, "Must provide input positions");
stride = stride ? stride : sizeof(decltype(*positions));
auto const& outPositions = mOutput->positions();
Expand Down
6 changes: 3 additions & 3 deletions libs/geometry/src/TangentSpaceMeshInternal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,6 @@ class InternalArray {
using ArrayType = std::variant<InternalArray<float2>, InternalArray<float3>, InternalArray<float4>,
InternalArray<ushort3>, InternalArray<ushort4>, InternalArray<quatf>>;

using AttributeMap = std::unordered_map<AttributeImpl, AttributeDataStride>;

ArrayType toArray(AttributeImpl attribute) {
switch (attribute) {
case AttributeImpl::UV1:
Expand All @@ -178,6 +176,8 @@ ArrayType toArray(AttributeImpl attribute) {
} // namespace

struct TangentSpaceMeshInput {
using AttributeMap = std::unordered_map<AttributeImpl, AttributeDataStride>;

size_t vertexCount = 0;
ushort3 const* triangles16 = nullptr;
uint3 const* triangles32 = nullptr;
Expand Down Expand Up @@ -357,7 +357,7 @@ struct TangentSpaceMeshOutput {
return std::get<InternalArray<DataType>>(attributeData[attrib]);
}

void passthrough(AttributeMap const& inAttributeMap,
void passthrough(TangentSpaceMeshInput::AttributeMap const& inAttributeMap,
std::vector<AttributeImpl> const& attributes) {
auto const borrow = [&inAttributeMap, this](AttributeImpl attrib) {
auto ref = inAttributeMap.find(attrib);
Expand Down
8 changes: 8 additions & 0 deletions libs/gltfio/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,17 @@ set(SRCS
src/TangentsJob.cpp
src/TangentsJob.h
src/UbershaderProvider.cpp
src/Utility.cpp
src/Utility.h
src/Wireframe.cpp
src/Wireframe.h
src/downcast.h
src/extended/AssetLoaderExtended.cpp
src/extended/AssetLoaderExtended.h
src/extended/ResourceLoaderExtended.cpp
src/extended/ResourceLoaderExtended.h
src/extended/TangentsJobExtended.cpp
src/extended/TangentsJobExtended.h
)

# ==================================================================================================
Expand Down
15 changes: 15 additions & 0 deletions libs/gltfio/include/gltfio/AssetLoader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ namespace filament::gltfio {

class NodeManager;

// Use this struct to enable mikktspace-based tangent-space computation.
/**
* \struct AssetConfigurationExtended AssetLoader.h gltfio/AssetLoader.h
* \brief extends struct AssetConfiguration
* Useful if client needs mikktspace tangents.
*/
struct AssetConfigurationExtended {
//! Optional The same parameter as provided to \struct ResourceConfiguration ResourceLoader.h
//! gltfio/ResourceLoader.h
char const* gltfPath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a significant philosophical shift in API. With this change, now the AssetLoader is locked to a specific glTF path. Before, this was limited to ResourceLoader. I don't have a good solution off the top of my head, but I would be in favor of removing the concept of the "file system" from gltfio. Instead, we could require the client to search the file system and provide all the resource data through addResourceData. In fact, this is what iOS, Android, and Web already do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting point. I didn't really understand the need to separate AssetLoader and ResourceLoader, and your explanation makes that much clearer.

I think, longterm, if I'm able to, I'd like to move toward the future you described where filesystem is abstracted out. And then we wouldn't have to have two API entrance points for loading gltf. For now, this is kind of just hacking to make things work on top of the existing two classes.

Still, I'd like to introduce this change, which will admittedly only benefit desktops and only when all the data are on disk, but it'll definitely make the desktop gtlf_viewer nicer.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to add on to this that the ability to load data from anywhere, not just the filesystem, is really quite important, whether it's loading from a network, or a compressed archive of some kind. Only being able to load from regular filesystem files isn't flexible enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. I'll likely add to this with a more general loading mechanism in a follow-up. Thank you for the comment.

};

/**
* \struct AssetConfiguration AssetLoader.h gltfio/AssetLoader.h
* \brief Construction parameters for AssetLoader.
Expand All @@ -62,6 +74,9 @@ struct AssetConfiguration {

//! Optional default node name for anonymous nodes
char* defaultNodeName = nullptr;

//! Optional to enable mikktspace tangents.
AssetConfigurationExtended* ext = nullptr;
};

/**
Expand Down
1 change: 0 additions & 1 deletion libs/gltfio/include/gltfio/ResourceLoader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ class UTILS_PUBLIC ResourceLoader {

private:
bool loadResources(FFilamentAsset* asset, bool async);
void normalizeSkinningWeights(FFilamentAsset* asset) const;
struct Impl;
Impl* pImpl;
};
Expand Down
Loading
Loading