NervLand DevLog #31: Adding support to display the DamagedHelmet PBR model

Hi everyone! Having a short break from ProLand this time: here we will rather get back to our PBR investigations and try to display the DamagegHelmet “standard GLTF” model in NervLand. As usual, we will hit some road blocks, but that never stopped us before, right 😉? So let's get to it and see how this goes!

Youtube videos (2 parts) for this article available at:

References:

• https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html
• https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0

The first initial attempt I made was to download the DamagedHelmet model and then create a new sample demo app based on our previous simple_scene demo, replacing the cerberus model with this new one.

Unfortunately, the first thing you notice, is that the roughness and metallic infos are both stored in the same texture on the green and blue channels respectively.

So my idea was to provide that file twice in the PBR setup:

        .draw_pbr({
            .ibl = _ibl,
            .transform = Mat4f::scale(2.0, 2.0, 2.0) *
                         Mat4f::rotate(toRad(90.0), VEC3F_YAXIS) *
                         Mat4f::rotate(toRad(-90.0), VEC3F_XAXIS),
            .modelName = "tests/DamagedHelmet",
            .albedoFile = "tests/damaged_helmet/Default_albedo.jpg",
            .roughnessFile = "tests/damaged_helmet/Default_metalRoughness.jpg",
            .metallicFile = "tests/damaged_helmet/Default_metalRoughness.jpg",
            .normalFile = "tests/damaged_helmet/Default_normal.jpg",
            .aoFile = "tests/damaged_helmet/Default_AO.jpg",
        });

⇒ The good thing is, this will only create a single shared copy of the source texture. But then in the shader itself, we need to be able to select which channel we will use for retrieval of the data, something like that:

	// var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv).x;
	// var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv).x;
	var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv)[2];
	var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv)[1];

But obviously, we are breaking the initial implementation here, so we need something more flexible, and I think this is where proper support for preprocessor defines could become handly, allowing something like that:

#ifndef NV_PBR_ROUGHNESS_CHANNEL
#define NV_PBR_ROUGHNESS_CHANNEL 1
#endif

#ifndef NV_PBR_METALLIC_CHANNEL
#define NV_PBR_METALLIC_CHANNEL 1
#endif

    // Later in function:
    var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv)[NV_PBR_METALLIC_CHANNEL];
	var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv)[NV_PBR_ROUGHNESS_CHANNEL];

⇒ Setting up a unit test to check this:

BOOST_AUTO_TEST_CASE(test_define_support) {
    // Test the support for define values

    String source = R"(
#ifndef NV_PBR_ROUGHNESS_CHANNEL
#define NV_PBR_ROUGHNESS_CHANNEL 1
#endif

#ifndef NV_PBR_METALLIC_CHANNEL
#define NV_PBR_METALLIC_CHANNEL 1
#endif

// Later in function:
var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv)[NV_PBR_METALLIC_CHANNEL];
var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv)[NV_PBR_ROUGHNESS_CHANNEL];
)";

    String expected = R"(


// Later in function:
var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv)[1];
var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv)[1];
)";

    String expected2 = R"(


// Later in function:
var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv)[3];
var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv)[1];
)";

    Preprocessor pproc;

    String res =
        pproc.preprocess_content(source, WGPUEngine::cat_shader, nullptr);

    BOOST_CHECK_EQUAL(res, expected);

    Preprocessor::DefMap defs;
    Preprocessor::add_defs(defs, "NV_PBR_METALLIC_CHANNEL=3,SOMETHING=0");

    res = pproc.preprocess_content(source, WGPUEngine::cat_shader, &defs);

    BOOST_CHECK_EQUAL(res, expected2);
}

⇒ in the end this was pretty easy to implement 😉! Most significant change was this function below:

void Preprocessor::process_define(ProcessingContext& ctx, const String& line) {

    // Retrieve the name of the definition of interest here:
    std::size_t pos = define_prefix.size();
    String defLine = strip(remove_comments(line.substr(pos)));
    defLine = normalize_spaces(defLine);

    // The defLine should contain 2 elements separated by space:
    // But it might also be that we try to define something without a value,
    //  in which case, we will not have that space:

    // Find the first space char:
    auto idx = defLine.find(' ');
    // NVCHK(idx != String::npos, "Unexpected #define statement: {}",
    // defLine.c_str());

    String defValue;
    String defName = defLine;
    if (idx != String::npos) {
        defName = defLine.substr(0, idx);
        defValue = defLine.substr(idx + 1);
    }

    // Check that this key is not already defined:
    NVCHK(!ctx.defines.contains(defName), "Name {} already defined.",
          defValue.c_str());

    // Add this define to the list:
    ctx.defines.insert(std::make_pair(std::move(defName), std::move(defValue)));

    // Erase the current line:
    ctx.lines.erase(ctx.lines.begin() + ctx.lineIdx);
}

During my first test I actually got an error on the loading of the default sampler (eg. “{}”) from the gltf file. I traced this to this location:

static auto get_wgpu_filter_mode(I32 filterMode) -> FilterMode {
    switch (filterMode) {
    case 9728:
        return FilterMode::Nearest;
    case 9729:
        return FilterMode::Linear;
    case 9984:
        return FilterMode::Nearest;
    case 9985:
        return FilterMode::Nearest;
    case 9986:
        return FilterMode::Linear;
    case 9987:
        return FilterMode::Linear;
    default:
        THROW_MSG("Unsupported filter mode value: {}", filterMode);
        return FilterMode::Linear;
    }
}

Which was producing this kind of error message:

2023-09-16 22:29:47.796509 [DEBUG] Loading GLTF model from file assets/models/tests/DamagedHelmet.gltf
2023-09-16 22:29:47.796641 [DEBUG] Loading CGLTF buffers...
2023-09-16 22:29:47.796895 [DEBUG] Loading 1 GLTF samplers...
2023-09-16 22:29:47.796897 [FATAL] Unsupported filter mode value: 0Ù7ƒ▼°

And then eventually I reached the cgltf code to create the sampler object:

static int cgltf_parse_json_sampler(cgltf_options* options, jsmntok_t const* tokens, int i, const uint8_t* json_chunk, cgltf_sampler* out_sampler)
{
	(void)options;
	CGLTF_CHECK_TOKTYPE(tokens[i], JSMN_OBJECT);

	out_sampler->wrap_s = 10497;
	out_sampler->wrap_t = 10497;

	int size = tokens[i].size;
	++i;

And because of the error message above I was first thinking that maybe some values in the sampler class were not properly initialized and some default were missing. But then I also realized that this sampler struct was created with a call to calloc() which will set all the memory to zero in the process. And thinking again about the error message I got, something started to feel wrong: in the THROW_MSG call is supposed to display an I32 value, so even if that value was not properly initialized to 0, it should still display as a number!

⇒ So this eventually led me to realize I actually had a bug in my method used to throw an error:

template <typename... Args>
inline void throw_msg(fmt::format_string<Args...> msg_format, Args&&... args) {
    // logFATAL(fmt, std::forward<Args>(args)...);
    auto out = fmt::memory_buffer();
    fmt::format_to(std::back_inserter(out), msg_format,
                   std::forward<Args>(args)...);
    auto* data = out.data(); // pointer to the formatted data
    logFATAL((const char*)data);
    // auto size = out.size();  // size of the formatted data
    // nv::String msg(data, size);
    // THROW(std::runtime_error(msg.c_str()));
#ifdef __cpp_exceptions
    throw std::runtime_error(data);
#else
    logFATAL(
        "Terminating program (Cannot throw when exceptions are disabled).");
    std::terminate();
#endif
}

⇒ In the code above I really need to take into consideration the size of the output string:

template <typename... Args>
inline void throw_msg(fmt::format_string<Args...> msg_format, Args&&... args) {
    // logFATAL(fmt, std::forward<Args>(args)...);
    auto out = fmt::memory_buffer();
    fmt::format_to(std::back_inserter(out), msg_format,
                   std::forward<Args>(args)...);
    auto* data = out.data(); // pointer to the formatted data
    std::string str(data, out.size());
    logFATAL(str.c_str());
    // auto size = out.size();  // size of the formatted data
    // nv::String msg(data, size);
    // THROW(std::runtime_error(msg.c_str()));
#ifdef __cpp_exceptions
    throw std::runtime_error(str.c_str());
#else
    logFATAL(
        "Terminating program (Cannot throw when exceptions are disabled).");
    std::terminate();
#endif
}

With that fix I now get the expected error message:

2023-09-16 22:40:16.744184 [DEBUG] Loading GLTF model from file assets/models/tests/DamagedHelmet.gltf
2023-09-16 22:40:16.744297 [DEBUG] Loading CGLTF buffers...
2023-09-16 22:40:16.744518 [DEBUG] Loading 1 GLTF samplers...
2023-09-16 22:40:16.744520 [FATAL] Unsupported filter mode value: 0

According to the gltf 2.0 specs:

Samplers are stored in the samplers array of the asset. Each sampler specifies filtering and wrapping modes.

The sampler properties use integer enums defined in the Properties Reference.

Client implementations SHOULD follow specified filtering modes. When the latter are undefined, client implementations MAY set their own default texture filtering settings.

Client implementations MUST follow specified wrapping modes.

⇒ So for now, when we read “0”, let's just initialize the filtering to Linear (an arbitrary decision really).

After the resolution of the sampler loading in the gltf file I got another error, this time on the image name:

2023-09-16 22:41:13.456285 [DEBUG] Loading GLTF model from file assets/models/tests/DamagedHelmet.gltf
2023-09-16 22:41:13.456415 [DEBUG] Loading CGLTF buffers...
2023-09-16 22:41:13.456657 [DEBUG] Loading 1 GLTF samplers...
2023-09-16 22:41:13.456666 [DEBUG] Loading 5 GLTF images...
2023-09-16 22:41:13.456668 [FATAL] Invalid name for gltf image 0
2023-09-16 22:41:13.456669 [FATAL] A fatal error occured

Checking the specs, it seems that the “name” is indeed not mandatory: https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#reference-image

So let's fix that, this is trivial really:

        cgltf_image* image = &data->images[i];
        // NVCHK(image->name != nullptr, "Invalid name for gltf image {}", i);
        if (image->name != nullptr) {
            logDEBUG("Loading GLTF image {}", image->name);
        }

But my next issue on this point is that I'm actually trying to load the images from the gltf file then:

2023-09-16 22:51:43.240276 [DEBUG] glTF: Loading image from file assets/models/tests/Default_albedo.jpg
2023-09-16 22:51:43.240291 [WARN] => Ignoring address mode 'repeat' from original implementation.
2023-09-16 22:51:43.240307 [FATAL] Invalid filename assets/models/tests/Default_albedo.jpg
2023-09-16 22:51:43.240309 [FATAL] A fatal error occured

⇒ I could disable that, but thinking about it: maybe I should rather just load the images in the model here and then use them to build the PBR rendering ?

Arrghh… but then this also means I should create/use the “materials” in the gltf model 😂

Note: Eventually I should link the materials to the corresponding mesh primitive in the file, but for now, let's just assume we will only have 1 material per file.

So we should introduce a simple material class:

class NVCORE_EXPORT Material : public RefObject {
    NV_DECLARE_CLASS(nv::Material)
    NV_DECLARE_NO_COPY(Material)
    NV_DECLARE_NO_MOVE(Material)

  public:
    // Constructor:
    Material();

    // Destructor
    ~Material() override;

    /** Set emissive factor */
    void set_emissive_factor(F32 x, F32 y, F32 z) {
        _emissiveFactor.set(x, y, z);
    }

    /** set emissive texture */
    void set_emissive_texture(wgpu::Texture tex) {
        _emissiveTexture = std::move(tex);
    }

    /** set normal texture */
    void set_normal_texture(wgpu::Texture tex) {
        _normalTexture = std::move(tex);
    }

    /** set occlusion texture */
    void set_occlusion_texture(wgpu::Texture tex) {
        _occlusionTexture = std::move(tex);
    }

    /** set baseColor texture */
    void set_base_color_texture(wgpu::Texture tex) {
        _baseColorTexture = std::move(tex);
    }

    /** set metalRoughness texture */
    void set_metal_roughness_texture(wgpu::Texture tex) {
        _metalRoughnessTexture = std::move(tex);
    }

  protected:
    /** Emissive factor */
    Vec3f _emissiveFactor{};

    /** Emissive texture */
    wgpu::Texture _emissiveTexture{};

    /** Normal texture */
    wgpu::Texture _normalTexture{};

    /** Occlusion texture */
    wgpu::Texture _occlusionTexture{};

    /** Base color texture */
    wgpu::Texture _baseColorTexture{};

    /** Metal roughness texture */
    wgpu::Texture _metalRoughnessTexture{};
};

I have then updated the Scene::draw_pbr() method to also support retrieving some material details directly from the gltf model being loaded.

But then, naturally, the next issue I get is due to the fact that the tangent attribute is not present in the model 😵‍💫, so we really need to generate that now.

To prepare the support to generate the “tangent attribute” I have introduced some “Creation flags” in my GLTFModel class: then I can add the flag “GENERATE_TANGENT” to this and proceed accordingly.

⇒ Allright, this was pretty tricky, but I have now implemented some kind of mechanism to generate the tangent attribute (still to be validated).

And actually, from that point, I can now display the model in my test app.. ta ta!:

Unfortunately, as you can see above, this is completed messed up for the moment 🤣 So let's try to fix that progressively.

First I fixed the transformation for the model:

            .transform = Mat4f::scale(2.0, 2.0, 2.0) *
                         Mat4f::rotate(toRad(160.0), VEC3F_YAXIS) *
                         Mat4f::rotate(toRad(90.0), VEC3F_XAXIS),

Next, what I find a bit strange is this green color display which seems to come from the albedo… hmmm 🤔.

The original model definitely doesn't look like that 😲: https://sketchfab.com/3d-models/battle-damaged-sci-fi-helmet-pbr-b81008d513954189a063ff901f7abfe4

Okay, so i realized that if is flip the v texcoord, I could move from this:

to this:

Still pretty far from perfect but that seems like an appropriate step anyway: so to me this indicates I probably need to flip the images vertically on load.

but even with that change it really seems there is something completely messed up with the UVs here.

Hmmm, checking the range of the texture coords, it seems we are well above 1.0 on one dimension:

        {
            "bufferView" : 3,
            "componentType" : 5126,
            "count" : 14556,
            "max" : [
                0.9999759793281555,
                1.998665988445282
            ],
            "min" : [
                0.002448640065267682,
                1.0005531199858524
            ],
            "type" : "VEC2"
        }

⇒ So this could indicated we really need to repeat for the wrap_s/wrap_t dims ? (and indeed value 10497 = GL_REPEAT!)

And indeed, much better with the repeat sampler setup:

But now it seems it will be better if I don't flip the V coord 😆 Bingo! Now we are really moving forward:

But I still need to enable the normal mapping part… And whhaaoo, this actually seems to work fine 😲🤣!

Next I also fixed the preprocessor defines usage in the pbr_texture shader:

#ifdef NV_PBR_SHARED_METAL_ROUGH_TEX
	var mr = textureSample(metallicTex, linearSampler, in.uv).xyz;
	var metallic: f32 = mr.z;
	var roughness: f32 = mr.y;
#else
	var metallic: f32 = textureSample(metallicTex, linearSampler, in.uv)[0];
	var roughness: f32 = textureSample(roughnessTex, linearSampler, in.uv)[0];
#endif

Another strange issue I noticed was on the PBR BRDF sampling process itself:

var brdf: vec2<f32> = textureSample(brdfTexture, linearSampler, vec2(max(dot(N, V), 0.0), roughness)).xy;

If I use the default sampler I'm setting up here for the damaged helmet resources access (eg. linear + repeat wrap_st), I get some inappropriate display:

But then if I change that to a dedicated sampler with (linear + clamp_to_edge + maxLod=1.0), then I get the expected display again:

⇒ Okay: investigating this further, it seems that just using a small minimal clamp on the roughness value is enough to be able to simple with the defaul (linear + repeat wrap_st) sampler:

var brdf: vec2<f32> = textureSample(brdfTexture, linearSampler, vec2(min(max(dot(N, V), 0.0), 1.0), clamp(roughness,0.001,1.0))).xy;

Note: If I use clamp(roughness,0.0,1.0) this will still not display correctly. I'm still not quite sure to understand that part, but let's say this is not critical for now.

Also, I realized that eventually I would need support to keep “empty locations” in a BindGroup in case some of the optional features are not available: otherwise I would quickly get into a mess trying the manage the binding location indices. So I have just defined the BindNone simple target. And now I can build a bind group as follow for instance:

    Bool sharedMetalRoughTex = metallic.Get() == roughness.Get();

    BindEntryList entries;
    append_bind_entries(entries, cam.as_ubo(), makeUBO(infos.transform),
                        sceneU.as_frag_ubo(), BindRepeatLinearSampler, BindNone,
                        tex2d(infos.ibl.brdfLut), texCube(infos.ibl.irradiance),
                        texCube(infos.ibl.filteredEnv), tex2d(albedo),
                        tex2d(normal), tex2d(ao), tex2d(roughness),
                        sharedMetalRoughTex ? BindNone : tex2d(metallic));

    if (sharedMetalRoughTex) {
        rnode.add_def("NV_PBR_SHARED_METAL_ROUGH_TEX");
    }

    rnode.add(mname, std::move(entries));

    rpass->render_node(rnode);

Or even directly as:

    Bool sharedMetalRoughTex = metallic.Get() == roughness.Get();

    if (sharedMetalRoughTex) {
        rnode.add_def("NV_PBR_SHARED_METAL_ROUGH_TEX");
    }

    rnode.add(mname, cam.as_ubo(), makeUBO(infos.transform),
              sceneU.as_frag_ubo(), BindRepeatLinearSampler, BindNone,
              tex2d(infos.ibl.brdfLut), texCube(infos.ibl.irradiance),
              texCube(infos.ibl.filteredEnv), tex2d(albedo), tex2d(normal),
              tex2d(ao), tex2d(roughness),
              sharedMetalRoughTex ? BindNone : tex2d(metallic));

    rpass->render_node(rnode);

Side Note: Oh and by the way, remember that weird UV mapping issue I had on one of the elements of the Cerberus model ? Well, it seems that using a (linear + repeat) sampler by default in our PBR pipeline now resolved that issue (this was basically the same problem as described above on the initial step with the damaged helmet display):

Finally I added support to access the emissve texture of the material if available, again using a custom preprocessor definition to integrated this in our unified PBR shader. And here is the result of this:

And here is a short video of the results I got with that 🥰: https://x.com/magik_engineer/status/1703839864049844291?s=20

Trying to load the model with embedded images, I eventually realized that the image data could in fact be base64 encoded directly in the image uri, for instance:

    "images" : [
        {
            "uri" : "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAUDBAQEAwUEBAQFBQUGBwwIBwcHBw8LCwkMEQ8SEhEPERETFhwXExQaFRERGCEYGh0dHx8fExciJCIeJBweHx7/2wBDAQUFBQcGBw4ICA4eFBEUHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh7/wAARCAgACAADASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAAA [...]
        }
]

And we currently don't seem to handle that correctly ?

OK: Fixed it: in the end for the images in a GLTF file we need to perform the base64 decoding ourself, so I had to update the code as follow:

        const char* uri = image->uri;
        if (uri != nullptr) {
            // We still need to check if this is a base64 encoded image:
            if (strncmp(uri, "data:", 5) == 0) {

                const char* comma = strchr(uri, ',');
                NVCHK(comma != nullptr, "Invalid comma.");
                auto diff = (size_t)(comma - uri);
                // logDEBUG("comma - uri = {}", diff);
                NVCHK(diff >= 7, "WTF on the comma diff.");
                size_t cmp = strncmp(comma - 7, ";base64", 7);
                // logDEBUG("cmp == {}", cmp);
                NVCHK(cmp == 0, "WTF on the base64 compare.");
                // String(uri).substr(0, 40)

                char* imgdata = nullptr;

                const char* b64str = comma + 1;
                U64 imgdata_size = get_original_length_from_b64(b64str);
                // logDEBUG("original image size: {}", imgdata_size);

                cgltf_result res = cgltf_load_buffer_base64(
                    &cgltf_opts, imgdata_size, b64str, (void**)&imgdata);

                NVCHK(res == cgltf_result_success, "Cannot decode image data.");

                //   Read the texture:
                texture =
                    eng->load_texture_from_memory(imgdata, imgdata_size, &opts);

                // logDEBUG("Releasing image data {}", i);
                CGLTF_FREE(imgdata);
                // logDEBUG("Done releasing {}", i);

                // texture =
                //     eng->create_texture_2d(256, 256,
                //     TextureFormat::BGRA8Unorm,
                //                            TextureUsage::TextureBinding);
            } else {
                // this is a regular file:
                // We load the image data from file.
                String img_path = get_path(model_dir, image->uri);
                logDEBUG("glTF: Loading image from file {}", img_path);
                texture = eng->load_texture_from_file(img_path.c_str(), &opts);
            }
        }

And now it's working fine 👍!

For the binary format version, I'm now automatically checking if a “.glb” file is available before checking the corresponding “.gltf” file, and it seems this is still working just fine too 😎:

auto WGPUEngine::get_model(const char* filename,
                           const WGPUGLTFModel::CreateInfo& infos)
    -> WGPUGLTFModel* {

    String fullpath = filename;
    if (get_path_extension(fullpath).empty()) {
        // Try to select the best extension automatically:
        fullpath += ".glb";
        if (!search_resource_path(cat_model, fullpath)) {
            fullpath = filename;
            fullpath += ".gltf";
        }
    }
    validate_resource_path(cat_model, fullpath);