Skip to content

Conversation

@pusewicz
Copy link
Contributor

Closes #324

@RandyGaul
Copy link
Owner

Awesome! So how can we know if the implementation here works as expected?

if (num_images > 0) {
image_names = (const char**)malloc(sizeof(char*) * num_images);
memcpy(image_names, image_names_vec.data(), sizeof(char*) * num_images);
image_binding_slots = (int*)malloc(sizeof(int) * num_images);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be freed in cute_shader_free_result

fprintf(file, " \"%s\",", shader_info->image_names[i]);
}
fprintf(file, "\n};\n");
fprintf(file, "static int %s%s_image_binding_slots[%d] = {\n ", var_name, suffix, shader_info->num_images);
Copy link
Contributor

Choose a reason for hiding this comment

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

const int for consistency with the rest.

Also some C compilers (e.g: Clang) are strict about what is considered "constant expression" for static initializer used in CF_ShaderBytecode later

fprintf(file, "\n};\n");
} else {
fprintf(file, "static const char** const %s%s_image_names = NULL;\n", var_name, suffix);
fprintf(file, "static int* const %s%s_image_binding_slots = NULL;\n", var_name, suffix);
Copy link
Contributor

Choose a reason for hiding this comment

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

const inst* const for the same reason.

@bullno1
Copy link
Contributor

bullno1 commented Aug 28, 2025

Awesome! So how can we know if the implementation here works as expected?

That's actually a tough question.

I recall that some engine actually runs the rendering and do image diff for simple static scenes.
It is probably possible to do something like that if it's just triangle and static sprites.

const char** image_names;
/* @member Binding slot index of each image. */
int* image_binding_slots;
const int* image_binding_slots;
Copy link
Contributor

@bullno1 bullno1 Aug 28, 2025

Choose a reason for hiding this comment

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

Oh I only meant the global variable generated by the shader compiler.
Because they get used by the static initializer later.
This doesn't have to be const.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually has to, as otherwise the generated values cannot be assigned to the struct member:

static const char** const s_backbuffer_fs_bytecode_image_names = NULL;
static const int* const s_backbuffer_fs_bytecode_image_binding_slots = NULL;
static CF_ShaderUniformInfo* const s_backbuffer_fs_bytecode_uniforms = NULL;
static CF_ShaderUniformMemberInfo* const s_backbuffer_fs_bytecode_uniform_members = NULL;
static CF_ShaderInputInfo* const s_backbuffer_fs_bytecode_inputs = NULL;
static const CF_ShaderBytecode s_backbuffer_fs_bytecode = {
    .content = s_backbuffer_fs_bytecode_content,
    .size = 408,
    .shader_info = {
        .num_samplers = 0,
        .num_storage_textures = 0,
        .num_storage_buffers = 0,
        .num_images = 0,
        .image_names = s_backbuffer_fs_bytecode_image_names,
        .image_binding_slots = s_backbuffer_fs_bytecode_image_binding_slots,
        .num_uniforms = 0,
        .uniforms = s_backbuffer_fs_bytecode_uniforms,
        .num_uniform_members = 0,
        .uniform_members = s_backbuffer_fs_bytecode_uniform_members,
        .num_inputs = 0,
        .inputs = s_backbuffer_fs_bytecode_inputs,
    },
};

This is what it's generated as: static const int* const s_backbuffer_fs_bytecode_image_binding_slots = NULL;

And here it's being assigned:

.image_binding_slots = s_backbuffer_fs_bytecode_image_binding_slots,

So unless there's a better way of doing it, it has to stay as is?

Copy link
Contributor

@bullno1 bullno1 Aug 31, 2025

Choose a reason for hiding this comment

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

I mean the problem is not in the generated code.

The declaration in the struct member does not have to be const int*.
It can just be int* image_binding_slots.

It's only const int* const in the generated code because some compilers in C mode complains that static initializer has to be constant expression.

free((char*)shader_info->image_names[i]);
}
free(shader_info->image_names);
free((int*)shader_info->image_binding_slots);
Copy link
Contributor

Choose a reason for hiding this comment

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

So the cast to free is not necessary if it wasn't const in the struct in the first place.

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.

Texture binding slots issue

3 participants