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

Support for vrm. #199

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 65 additions & 5 deletions gltf/gltfpack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ static void process(cgltf_data* data, const char* input_path, const char* output
std::string json_accessors;
std::string json_meshes;
std::string json_nodes;
std::string json_extensions;
std::string json_skins;
std::vector<std::string> json_roots(data->scenes_count);
std::string json_animations;
Expand Down Expand Up @@ -440,7 +441,15 @@ static void process(cgltf_data* data, const char* input_path, const char* output
const Mesh& mesh = meshes[i];

comma(json_meshes);
append(json_meshes, "{\"primitives\":[");
append(json_meshes, "{");
if (!mesh.name.empty())
{
append(json_meshes, "\"name\":\"");
append(json_meshes, mesh.name);
append(json_meshes, "\"");
comma(json_meshes);
}
append(json_meshes, "\"primitives\":[");

size_t pi = i;
for (; pi < meshes.size(); ++pi)
Expand All @@ -462,7 +471,8 @@ static void process(cgltf_data* data, const char* input_path, const char* output
const QuantizationTexture& qt = prim.material ? qt_materials[prim.material - data->materials] : qt_dummy;

comma(json_meshes);
append(json_meshes, "{\"attributes\":{");
append(json_meshes, "{");
append(json_meshes, "\"attributes\":{");
writeMeshAttributes(json_meshes, views, json_accessors, accr_offset, prim, 0, qp, qt, settings);
append(json_meshes, "}");
append(json_meshes, ",\"mode\":");
Expand All @@ -488,6 +498,11 @@ static void process(cgltf_data* data, const char* input_path, const char* output
append(json_meshes, ",\"indices\":");
append(json_meshes, index_accr);
}
if (settings.keep_extras && !prim.extras.empty())
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this necessary? AFAICT the only "extras" node in the .VRM file from the issue is on the mesh, not on primitive, and that contains targetNames which should already be preserved by gltfpack.

{
append(json_meshes, ",\"extras\":");
append(json_meshes, prim.extras);
}

if (prim.material)
{
Expand Down Expand Up @@ -592,7 +607,9 @@ static void process(cgltf_data* data, const char* input_path, const char* output
append(json_nodes, "{");
writeNode(json_nodes, node, nodes, data);
if (settings.keep_extras)
{
writeExtras(json_nodes, extras, node.extras);
}
append(json_nodes, "}");
}

Expand Down Expand Up @@ -650,7 +667,7 @@ static void process(cgltf_data* data, const char* input_path, const char* output
const ExtensionInfo extensions[] = {
{"KHR_mesh_quantization", settings.quantize, true},
{"EXT_meshopt_compression", settings.compress, !settings.fallback},
{"KHR_texture_transform", settings.quantize && !json_textures.empty(), false},
{"KHR_texture_transform", (settings.quantize && !json_textures.empty()) || (settings.vrm && !json_textures.empty()), false},
{"KHR_materials_pbrSpecularGlossiness", ext_pbr_specular_glossiness, false},
{"KHR_materials_clearcoat", ext_clearcoat, false},
{"KHR_materials_transmission", ext_transmission, false},
Expand All @@ -661,10 +678,39 @@ static void process(cgltf_data* data, const char* input_path, const char* output
{"KHR_lights_punctual", data->lights_count > 0, false},
{"KHR_texture_basisu", !json_textures.empty() && settings.texture_ktx2, true},
{"EXT_mesh_gpu_instancing", ext_instancing, true},
{"VRM", settings.vrm, false},
};

writeExtensions(json, extensions, sizeof(extensions) / sizeof(extensions[0]));

comma(json_extensions);
append(json_extensions, "\"extensions\":{");
if (settings.vrm)
{

std::string extension_vrm;
extension_vrm.assign("VRM");
for (size_t i = 0; i < data->data_extensions_count; ++i)
{
cgltf_extension extension = data->data_extensions[i];
if (!extension.name)
Copy link
Owner

Choose a reason for hiding this comment

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

This & above & below can be replaced with if (strcmp(extension.name, "VRM") == 0) (extension.name is guaranteed to be non-null)

{
continue;
}
std::string extension_tested;
extension_tested.assign(extension.name);
if (extension_vrm != extension_tested)
{
break;
}
append(json_extensions, "\"");
append(json_extensions, extension.name);
append(json_extensions, "\":");
append(json_extensions, extension.data);
Copy link
Owner

Choose a reason for hiding this comment

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

This should use appendJson to minify the output

}
}
append(json_extensions, "}");

std::string json_views;
finalizeBufferViews(json_views, views, bin, settings.fallback ? &fallback : NULL, fallback_size);

Expand All @@ -676,6 +722,8 @@ static void process(cgltf_data* data, const char* input_path, const char* output
writeArray(json, "meshes", json_meshes);
writeArray(json, "skins", json_skins);
writeArray(json, "animations", json_animations);
comma(json);
append(json, json_extensions);
Copy link
Owner

Choose a reason for hiding this comment

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

This will always emit "extensions" and also causes a conflict with top-level KHR_lights_punctual extension since there's now going to be two extension blocks for files that use that.

Instead what should happen is:

  • json_extensions should be initially empty
  • json_lights should be appended to it, if necessary
  • VRM data should be appended to it, if necessary
  • if json_extensions isn't empty when forming the final JSON blob, we should write out "extensions":{ followed by json_extensions contents followed by }

writeArray(json, "nodes", json_nodes);

if (!json_roots.empty())
Expand Down Expand Up @@ -791,7 +839,12 @@ int gltfpack(const char* input, const char* output, const char* report, Settings
const char* iext = strrchr(input, '.');
const char* oext = output ? strrchr(output, '.') : NULL;

if (iext && (strcmp(iext, ".gltf") == 0 || strcmp(iext, ".GLTF") == 0 || strcmp(iext, ".glb") == 0 || strcmp(iext, ".GLB") == 0))
if (iext && (strcmp(iext, ".vrm") == 0 || strcmp(iext, ".VRM") == 0))
{
settings.vrm = true;
}

if (iext && (strcmp(iext, ".vrm") == 0 || strcmp(iext, ".VRM") == 0 || strcmp(iext, ".gltf") == 0 || strcmp(iext, ".GLTF") == 0 || strcmp(iext, ".glb") == 0 || strcmp(iext, ".GLB") == 0))
{
const char* error = 0;
data = parseGltf(input, meshes, animations, extras, &error);
Expand Down Expand Up @@ -843,6 +896,13 @@ int gltfpack(const char* input, const char* output, const char* report, Settings
settings.texture_embed = true;
}

if (oext && (strcmp(oext, ".vrm") == 0 || strcmp(oext, ".VRM") == 0))
{
settings.vrm = true;
settings.keep_extras = true;
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer to omit these, esp. quantize but preferably others. It's not clear to me why these need to be present, and if a user wants to apply these optimizations to a VRM file there's not going to be a way to do this without adding more kinds of command-line options, if vrm extension acts as an override.

settings.quantize = false;
}

std::string json, bin, fallback;
size_t fallback_size = 0;
process(data, input, output, report, meshes, animations, extras, settings, json, bin, fallback, fallback_size);
Expand Down Expand Up @@ -896,7 +956,7 @@ int gltfpack(const char* input, const char* output, const char* report, Settings
return 4;
}
}
else if (oext && (strcmp(oext, ".glb") == 0 || strcmp(oext, ".GLB") == 0))
else if (oext && (strcmp(oext, ".vrm") == 0 || strcmp(oext, ".VRM") == 0 || strcmp(oext, ".glb") == 0 || strcmp(oext, ".GLB") == 0))
Copy link
Owner

Choose a reason for hiding this comment

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

Should this also be added to the condition above that sets texture_embed? Unclear what the de-facto rules are for VRM files, e.g. whether they are supposed to keep textures separately or not.

{
std::string fbpath = output;
fbpath.replace(fbpath.size() - 4, 4, ".fallback.bin");
Expand Down
5 changes: 5 additions & 0 deletions gltf/gltfpack.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct Transform

struct Mesh
{
std::string name;
int scene;
std::vector<cgltf_node*> nodes;
std::vector<Transform> instances;
Expand All @@ -54,6 +55,8 @@ struct Mesh
size_t targets;
std::vector<float> target_weights;
std::vector<const char*> target_names;

std::string extras;
};

struct Track
Expand Down Expand Up @@ -125,6 +128,8 @@ struct Settings
bool fallback;

int verbose;

bool vrm;
};

struct QuantizationPosition
Expand Down
6 changes: 3 additions & 3 deletions gltf/mesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -725,8 +725,8 @@ static void sortPointMesh(Mesh& mesh)
}

void processMesh(Mesh& mesh, const Settings& settings)
{
filterStreams(mesh);
{
filterStreams(mesh);

switch (mesh.type)
{
Expand Down Expand Up @@ -765,7 +765,7 @@ void debugSimplify(const Mesh& source, Mesh& kinds, Mesh& loops, float ratio)

// note: it's important to follow the same pipeline as processMesh
// otherwise the result won't match
filterStreams(mesh);
filterStreams(mesh);
filterBones(mesh);
reindexMesh(mesh);
filterTriangles(mesh);
Expand Down
25 changes: 14 additions & 11 deletions gltf/parsegltf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,16 @@ static void fixupIndices(std::vector<unsigned int>& indices, cgltf_primitive_typ
}
}

static void evacuateExtras(cgltf_data* data, std::string& extras, cgltf_extras& item)
{
size_t offset = extras.size();

extras.append(data->json + item.start_offset, item.end_offset - item.start_offset);

item.start_offset = offset;
item.end_offset = extras.size();
}

static void parseMeshesGltf(cgltf_data* data, std::vector<Mesh>& meshes, std::vector<std::pair<size_t, size_t> >& mesh_remap)
{
size_t total_primitives = 0;
Expand All @@ -154,7 +164,7 @@ static void parseMeshesGltf(cgltf_data* data, std::vector<Mesh>& meshes, std::ve

for (size_t pi = 0; pi < mesh.primitives_count; ++pi)
{
const cgltf_primitive& primitive = mesh.primitives[pi];
cgltf_primitive& primitive = mesh.primitives[pi];

if (primitive.type == cgltf_primitive_type_points && primitive.indices)
{
Expand All @@ -165,6 +175,8 @@ static void parseMeshesGltf(cgltf_data* data, std::vector<Mesh>& meshes, std::ve
meshes.push_back(Mesh());
Mesh& result = meshes.back();

result.name = mesh.name;
Copy link
Author

Choose a reason for hiding this comment

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

Might be a problem with variable lifetime


result.scene = -1;

result.material = primitive.material;
Expand Down Expand Up @@ -244,6 +256,7 @@ static void parseMeshesGltf(cgltf_data* data, std::vector<Mesh>& meshes, std::ve
result.targets = primitive.targets_count;
result.target_weights.assign(mesh.weights, mesh.weights + mesh.weights_count);
result.target_names.assign(mesh.target_names, mesh.target_names + mesh.target_names_count);
evacuateExtras(data, result.extras, primitive.extras);
}

mesh_remap[mi] = std::make_pair(remap_offset, meshes.size());
Expand Down Expand Up @@ -378,16 +391,6 @@ static bool needsDummyBuffers(cgltf_data* data)
return false;
}

static void evacuateExtras(cgltf_data* data, std::string& extras, cgltf_extras& item)
{
size_t offset = extras.size();

extras.append(data->json + item.start_offset, item.end_offset - item.start_offset);

item.start_offset = offset;
item.end_offset = extras.size();
}

static void evacuateExtras(cgltf_data* data, std::string& extras)
{
size_t size = 0;
Expand Down