Conversation
|
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); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
const inst* const for the same reason.
That's actually a tough question. I recall that some engine actually runs the rendering and do image diff for simple static scenes. |
| const char** image_names; | ||
| /* @member Binding slot index of each image. */ | ||
| int* image_binding_slots; | ||
| const int* image_binding_slots; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
So the cast to free is not necessary if it wasn't const in the struct in the first place.
09e574b to
1eb21ef
Compare
|
It would be good to finish this up and get it in |
Closes #324