Multiple cgltf Nodes with base_color_texture Import, Skysphere matrix function

TimeTravelerFromNow commented 3 months ago

Sky matrix and cgltf material mapping

This PR expands cgltf loading; pbr basic color texture image url is used to load a texture and is saved in a pbr_textures array. The cgltf node's material pointer is used to map to save a material id on the mesh primitive in gunslinger.

Materials are initialized and a handle to the corresponding material is stored on the pbr_node for rendering the correct material. Currently this only supports the base_color_texture.

sky matrix

The sky matrix functions are matrix constructors that ignore position calculations, and can be used for skyboxes, skyspheres etc.

Todos copied from the example Pull Request

Matrix transforms for child- parent nodes.
More pbr textures and flag handling as in the gltf spec
the black mesh actually has a vec4 base color that isn't rendered yet.
Expand upon the pbr_basic.sf pipeline

Samdal commented 3 months ago

gs.h

What's the incentive behind the sky matrix functions? Just set the position to zero before getting the projection matrix.

    gs_camera_t tmp_cam = *cam;
    tmp_cam.transform.position = gs_v3s(0);
    gs_mat4 svp = gs_camera_get_view_projection(&tmp_cam, fbs.x, fbs.y);

All of those 60 lines don't do anything useful unless I'm misunderstanding something.

gs_gfxt

`gs_gfxt_load_gltf_all_data_from_file`

With this PR there will be two functions to load gltf data, why not update the existing one?

`gs_gfxt_scene_new`

As you mentioned in the other PR, this function uses hardcoded paths. Which isn't acceptable. The function also doesn't do anything. If I rewrite the function it becomes easier to see why

GS_API_DECL 
gs_gfxt_scene_t gs_gfxt_scene_new()
{
    return (gs_gfxt_scene_t){
        .pbr_pip = gs_gfxt_pipeline_load_from_file("path/to/pipeline")
    };
}

The user should just call gs_gfxt_pipeline_load_from_file themselves, no need to create a new function. Even if the pbr pipeline was to be vendored with gfxt using gs_gfxt_pipeline_load_from_memory, having a function like this would be better.

GS_API_DECL gs_gfxt_pipeline_t gs_gfxt_default_pbr_pipeline(void);

`gs_gfxt_scene_pbr_draw`

This function is mostly alright. but going with the idea of simply providing a default pipeline, renaming it to something like gs_gfxt_default_pbr_pipeline_draw would show clearly that it's specific to that pipeline.

`gs_gfxt_scene_free`

Your free function should also destroy the items contained within the slot arrays.

TimeTravelerFromNow commented 3 months ago

Glad to start this discussion on the items brought up.

gs.h

sky matrix functions

Why not just set the position column zero? This discards the last column of a standard view_projection matrix operation; for sure the easiest solution. The usefulness of sky-specific matrix, parts of the operation never need to be done in the first place. I can argue at least unlike the regular perspective matrix maker: gs.h line 3958, that we can omit 3 dot products. Meaning gs_gfxt_sky_perspective doesn't need these:

    m_res.elements[3 * 4 + 0] = -gs_vec3_dot(s, position);;
    m_res.elements[3 * 4 + 1] = -gs_vec3_dot(u, position);
    m_res.elements[3 * 4 + 2] = gs_vec3_dot(f, position);

and gs_gfxt_sky_perspective doesn't need this f32 c = (2.0f * n * f) / (n - f); I agree less lines of code is definitely better. What do you think @Samdal ?

gs_gfxt

gs_gfxt_load_gltf_all_data_from_file

started with a copy of the original. Didn't know if it was going to deviate a lot from the other gs_gfxt_load_gltf_data_from_file. Can for sure be merged with the original function.

gs_gfxt_scene_new

Your comment was the idea I needed input on this. If I understand right, scene_new will allow an optional user pipeline file, else it loads_from_memory the vendored default pbr pipeline. And the function example you wrote I'll add that.

Can you give me some help on how I would accomplish vendoring the pipeline from memory?

gs_gfxt_scene_pbr_draw

gs_gfxt_default_pbr_pipeline_draw. sounds good

gs_gfxt_scene_free

Definetely. Got some free functions to write

Future ideas after current work is done

Imagining what sort of physically-based rendering meshes we can draw will be so sick. Also I did some digging into gltf collision physics extensions, KHR_collision_shapes and KHR_physics_rigid_bodies, OMI_physics_body all super cool. KHR_physics_rigid_bodies shapes has a working blender extension.

Much appreciated for the review Samdal.

Samdal commented 3 months ago

gs_gfxt_scene_new

Your comment was the idea I needed input on this. If I understand right, scene_new will allow an optional user pipeline file, else it loads_from_memory the vendored default pbr pipeline. And the function example you wrote I'll add that.

Can you give me some help on how I would accomplish vendoring the pipeline from memory?

You didn't exactly understand what I meant.

Your function:

GS_API_DECL 
gs_gfxt_scene_t gs_gfxt_scene_new()
{
  // maybe we can ship this standard pipeline with the gs framework
  gs_println("looking for pbr_basic in ./assets/pipelines/pbr_basic.sf");
  const char* pbr_basic_path = "./assets/pipelines/pbr_basic.sf"; 
  gs_gfxt_pipeline_t pbr_pip = gs_gfxt_pipeline_load_from_file(pbr_basic_path);
  gs_gfxt_scene_t scene = {0};
  scene.pbr_pip = pbr_pip;
  return scene;
};

usage of your function:

gs_gfxt_scene_t scene = gs_gfxt_scene_new()

my function:

// it doesn't exist

usage of my function:

gs_gfxt_scene_t scene = {.pbr_pip = gs_gfxt_pipeline_load_from_file("path/to/pipeline")};

If you were to vendor a pipeline in together with gfxt, you should do this

GS_API_DECL 
gs_gfxt_pipeline_t gs_gfxt_default_pbr_pipeline(void)
{
  const char* pip = "/*\n"
"    TODO(john): Need a description of the .sf format here\n"
"*/\n"
"\n"
"pipeline {\n"
"\n"
"    raster\n"
"    {\n"
"        primitive: TRIANGLES\n"
"        index_buffer_element_size: UINT32\n"
"    },\n"
"\n"
"    depth\n"
"    {\n"
"        func: LESS\n"
"    },\n"
"\n"
"\n"
"    shader {\n"
"\n"
"        vertex {\n"
"\n"
"            attributes {\n"
"\n"
"                /*\n"
"                    Vertex layout required for this pipeline (for input assembler)\n"
"\n"
"                    fields:\n"
"\n"
"                    POSITION:          float3\n"
"                    TEXCOORD:          float2\n"
"                    TEXCOORD[0 - 12]:  float2\n"
"                    COLOR:         uint8[4]\n"
"                    NORMAL:        float3\n"
"                    TANGENT:       float4\n"
"                    JOINT:         float4\n"
"                    WEIGHT:        float\n"
"                    FLOAT:         float\n"
"                    FLOAT2:        float2\n"
"                    FLOAT3:        float3\n"
"                    FLOAT4:        float4\n"
"                */\n"
"\n"
"                POSITION : a_position\n"
"                TEXCOORD : a_uv\n"
"                COLOR    : a_color\n"
"            },\n"
"\n"
"            uniforms {\n"
"                mat4 u_mvp;\n"
"            },\n"
"\n"
"            out {\n"
"                vec2 uv;\n"
"                vec3 position;\n"
"            },\n"
"\n"
"            code {\n"
"                void main() {\n"
"                    gl_Position = u_mvp * vec4(a_position, 1.0);\n"
"                    uv = a_uv;\n"
"                    position = a_position;\n"
"                }\n"
"            }\n"
"        },\n"
"\n"
"        fragment {\n"
"        \n"
"            uniforms {\n"
"                sampler2D u_base_col_tex;\n"
"                vec4 u_base_col_fact;\n"
"            },\n"
"            \n"
"            out {\n"
"                vec4 frag_color;\n"
"            },\n"
"\n"
"            code {\n"
"                void main() {\n"
"                    frag_color = texture(u_base_col_tex, uv); // //\n"
"                }\n"
"            }\n"
"        }\n"
"    }\n"
"}\n";

  return gs_gfxt_pipeline_load_from_memory(pip, sizeof(pip)-1);
};

and then usage of it

gs_gfxt_scene_t scene = {.pbr_pip = gs_gfxt_default_pbr_pipeline()};

Samdal commented 3 months ago

Future ideas after current work is done

Imagining what sort of physically-based rendering meshes we can draw will be so sick.

The current PBR pipeline here only does color texture mapping. So it can't really do much in the current state, calling it PBR is a bit misleading to be honest. You'll have to do quite a bit more work for it to be called that, but I'm guessing that's the end goal so it doesn't bother me.

TimeTravelerFromNow commented 3 months ago

Added the free of arrays and merged the material loading logic into the existing gs_gfxt_mesh_load_from_file function, which had to be changed to accept the mesh directory, so we can load image URIs from the containing folder.

I wanted to set fragment uniforms to pass in has_base_color_factor, however gs_gfxt_uniform_block_create line 596 has the existing types, no custom structs yet. Thinking of adding a struct for the pbr description.

Samdal commented 3 months ago

I would've probably not had util/pbr_default.h as it's own file, and rather have it be inside util/gs_gfxt.h, but that's mostly personal preference. At this point it's mostly up to what @MrFrenik thinks about the PR.

TimeTravelerFromNow commented 3 months ago

Great, lmk if I should squash the commits

MrFrenik / gunslinger