Metal: Fix uniform upload for small types

This patch adds special cases to Shader::uniform_int routine
to allow writing of small types (1 bytes, 2 bytes) to the push
constant buffer.

This previously interpreted all incoming push constant data as
integer components only, resulting in rendering artifacts such as
bad SRGB mode selection and shader editor not rendering due
to mis-aligned overlay parameter, as the uniform assignment
would overflow consecutive small types.

Authored by Apple: Michael Parkin-White

Pull Request: https://projects.blender.org/blender/blender/pulls/119285
This commit is contained in:
Jason Fielder
2024-03-10 19:36:30 +01:00
committed by Clément Foucault
parent 2c5f51e683
commit 703353b5da
3 changed files with 32 additions and 5 deletions

View File

@@ -658,15 +658,43 @@ void MTLShader::uniform_int(int location, int comp_len, int array_size, const in
uint8_t *ptr = (uint8_t *)push_constant_data_;
ptr += uniform.byte_offset;
/** Determine size of data to copy. */
const char *data_to_copy = (char *)data;
uint data_size_to_copy = sizeof(int) * comp_len * array_size;
/* Special cases for small types support where storage is shader push constant buffer is smaller
* than the incoming data. */
ushort us;
uchar uc;
if (uniform.size_in_bytes == 1) {
/* Convert integer storage value down to uchar. */
data_size_to_copy = uniform.size_in_bytes;
uc = *data;
data_to_copy = (char *)&uc;
}
else if (uniform.size_in_bytes == 2) {
/* Convert integer storage value down to ushort. */
data_size_to_copy = uniform.size_in_bytes;
us = *data;
data_to_copy = (char *)&us;
}
else {
BLI_assert_msg(
(mtl_get_data_type_alignment(uniform.type) % sizeof(int)) == 0,
"When uniform inputs are provided as integers, the underlying type must adhere "
"to alignment per-component. If this test fails, the input data cannot be directly copied "
"to the buffer. e.g. Array of small types uchar/bool/ushort etc; are currently not "
"handled.");
}
/* Copy data into local block. Only flag UBO as modified if data is different
* This can avoid re-binding of unmodified local uniform data, reducing
* the total number of copy operations needed and data transfers between
* CPU and GPU. */
bool data_changed = (memcmp((void *)ptr, (void *)data, sizeof(int) * comp_len * array_size) !=
0);
bool data_changed = (memcmp((void *)ptr, (void *)data_to_copy, data_size_to_copy) != 0);
if (data_changed) {
this->push_constant_bindstate_mark_dirty(true);
memcpy((void *)ptr, (void *)data, sizeof(int) * comp_len * array_size);
memcpy((void *)ptr, (void *)data_to_copy, data_size_to_copy);
}
}

View File

@@ -191,7 +191,7 @@ void MTLShaderInterface::add_uniform(uint32_t name_offset, eMTLDataType type, in
/* Determine size and offset alignment -- C++ struct alignment rules: Base address of value must
* match alignment of type. GLSL follows minimum type alignment of 4. */
int data_type_size = mtl_get_data_type_size(type) * array_len;
int data_type_alignment = max_ii(mtl_get_data_type_alignment(type), 4);
int data_type_alignment = mtl_get_data_type_alignment(type);
int current_offset = push_constant_block_.current_offset;
if ((current_offset % data_type_alignment) != 0) {
current_offset += data_type_alignment - (current_offset % data_type_alignment);

View File

@@ -189,7 +189,6 @@ inline uint mtl_get_data_type_alignment(eMTLDataType type)
case MTL_DATATYPE_CHAR3:
case MTL_DATATYPE_UCHAR3:
case MTL_DATATYPE_BOOL3:
return 3;
case MTL_DATATYPE_CHAR4:
case MTL_DATATYPE_UCHAR4:
case MTL_DATATYPE_INT: