Skip to content

Add RGBA format in GpuShaderCreator for better Metal support (Issue #1956) #1984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add RGBA format in GpuShaderCreator for better Metal support (Issue #1956) #1984

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5befeb7
Add RGBA format to TextureType
wRosie Jun 4, 2024
1f0fdb2
Fix { and address code review comments
wRosie Jul 27, 2024
0e66261
Merge branch 'main' into feature/add_rgba_for_metal
wRosie Aug 19, 2024
00237a1
Merge branch 'main' into feature/add_rgba_for_metal
wRosie Sep 12, 2024
69c7780
Fix RGB_to_RGBA
wRosie Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/api/python/frozen/pyopencolorio_gpushaderdesc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
:module: PyOpenColorIO


.. py:method:: GpuShaderDesc.add3DTexture(self: PyOpenColorIO.GpuShaderDesc, textureName: str, samplerName: str, edgeLen: int, interpolation: PyOpenColorIO.Interpolation, values: buffer) -> None
.. py:method:: GpuShaderDesc.add3DTexture(self: PyOpenColorIO.GpuShaderDesc, textureName: str, samplerName: str, edgeLen: int, channel: PyOpenColorIO.GpuShaderCreator.TextureType, interpolation: PyOpenColorIO.Interpolation, values: buffer) -> None
:module: PyOpenColorIO

Add a 3D texture with RGB channel type.
Expand Down Expand Up @@ -477,6 +477,8 @@
.. py:property:: Texture3D.interpolation
:module: PyOpenColorIO.GpuShaderDesc

.. py:property:: Texture3D.channel
:module: PyOpenColorIO.GpuShaderDesc

.. py:property:: Texture3D.samplerName
:module: PyOpenColorIO.GpuShaderDesc
Expand Down
5 changes: 4 additions & 1 deletion include/OpenColorIO/OpenColorIO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3267,7 +3267,8 @@ class OCIOEXPORT GpuShaderCreator
enum TextureType
{
TEXTURE_RED_CHANNEL, ///< Only need a red channel texture
TEXTURE_RGB_CHANNEL ///< Need a RGB texture
TEXTURE_RGB_CHANNEL, ///< Need a RGB texture
TEXTURE_RGBA_CHANNEL ///< Need a RGBA texture
};

/**
Expand Down Expand Up @@ -3304,6 +3305,7 @@ class OCIOEXPORT GpuShaderCreator
virtual void add3DTexture(const char * textureName,
const char * samplerName,
unsigned edgelen,
TextureType channel,
Interpolation interpolation,
const float * values) = 0;

Expand Down Expand Up @@ -3551,6 +3553,7 @@ class OCIOEXPORT GpuShaderDesc : public GpuShaderCreator
const char *& textureName,
const char *& samplerName,
unsigned & edgelen,
TextureType& channel,
Interpolation & interpolation) const = 0;
virtual void get3DTextureValues(unsigned index, const float *& values) const = 0;

Expand Down
11 changes: 8 additions & 3 deletions src/OpenColorIO/GpuShader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ class PrivateImpl
void add3DTexture(const char * textureName,
const char * samplerName,
unsigned edgelen,
GenericGpuShaderDesc::TextureType channel,
Interpolation interpolation,
const float * values)
{
Expand All @@ -258,7 +259,7 @@ class PrivateImpl
}

Texture t(textureName, samplerName, edgelen, edgelen, edgelen,
GpuShaderDesc::TEXTURE_RGB_CHANNEL, 3,
channel, 3,
interpolation, values);
m_textures3D.push_back(t);
}
Expand All @@ -267,6 +268,7 @@ class PrivateImpl
const char *& textureName,
const char *& samplerName,
unsigned & edgelen,
GenericGpuShaderDesc::TextureType& channel,
Interpolation & interpolation) const
{
if(index >= m_textures3D.size())
Expand All @@ -281,6 +283,7 @@ class PrivateImpl
textureName = t.m_textureName.c_str();
samplerName = t.m_samplerName.c_str();
edgelen = t.m_width;
channel = t.m_type;
interpolation = t.m_interp;
}

Expand Down Expand Up @@ -520,19 +523,21 @@ unsigned GenericGpuShaderDesc::getNum3DTextures() const noexcept
void GenericGpuShaderDesc::add3DTexture(const char * textureName,
const char * samplerName,
unsigned edgelen,
TextureType channel,
Interpolation interpolation,
const float * values)
{
getImplGeneric()->add3DTexture(textureName, samplerName, edgelen, interpolation, values);
getImplGeneric()->add3DTexture(textureName, samplerName, edgelen, channel, interpolation, values);
}

void GenericGpuShaderDesc::get3DTexture(unsigned index,
const char *& textureName,
const char *& samplerName,
unsigned & edgelen,
TextureType & channel,
Interpolation & interpolation) const
{
getImplGeneric()->get3DTexture(index, textureName, samplerName, edgelen, interpolation);
getImplGeneric()->get3DTexture(index, textureName, samplerName, edgelen, channel, interpolation);
}

void GenericGpuShaderDesc::get3DTextureValues(unsigned index, const float *& values) const
Expand Down
2 changes: 2 additions & 0 deletions src/OpenColorIO/GpuShader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,14 @@ class GenericGpuShaderDesc : public GpuShaderDesc
void add3DTexture(const char * textureName,
const char * samplerName,
unsigned edgelen,
TextureType channel,
Interpolation interpolation,
const float * values) override;
void get3DTexture(unsigned index,
const char *& textureName,
const char *& samplerName,
unsigned & edgelen,
TextureType& channel,
Interpolation & interpolation) const override;
void get3DTextureValues(unsigned index, const float *& value) const override;

Expand Down
26 changes: 26 additions & 0 deletions src/OpenColorIO/GpuShaderUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,32 @@ std::string getMatrixValues(const T * mtx, GpuLanguage lang, bool transpose)
return vals;
}


void RGBtoRGBATexture(const float* lutValues, int valueCount, std::vector<float>& float4AdaptedLutValues)
{
if(valueCount % 3 != 0)
{
throw Exception("Value count should be divisible by 3.");
}

valueCount = valueCount * 4 / 3;
if(lutValues != nullptr)
{
float4AdaptedLutValues.resize(valueCount);
const float *rgbLutValuesIt = lutValues;
float *rgbaLutValuesIt = float4AdaptedLutValues.data();
const float *end = rgbaLutValuesIt + valueCount;

while(rgbaLutValuesIt != end)
{
*rgbaLutValuesIt++ = *rgbLutValuesIt++;
*rgbaLutValuesIt++ = *rgbLutValuesIt++;
*rgbaLutValuesIt++ = *rgbLutValuesIt++;
*rgbaLutValuesIt++ = 1.0f;
}
}
}

GpuShaderText::GpuShaderLine::GpuShaderLine(GpuShaderText * text)
: m_text(text)
{
Expand Down
3 changes: 3 additions & 0 deletions src/OpenColorIO/GpuShaderUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ void AddLinToLogShader(GpuShaderCreatorRcPtr & shaderCreator, GpuShaderText & st
// Convert "grading log" values to scene-linear.
void AddLogToLinShader(GpuShaderCreatorRcPtr & shaderCreator, GpuShaderText & st);

// Texture converter from RGB to RGBA
void RGBtoRGBATexture(const float* lutValues, int valueCount, std::vector<float>& float4AdaptedLutValues);

} // namespace OCIO_NAMESPACE

#endif
40 changes: 32 additions & 8 deletions src/OpenColorIO/ops/lut1d/Lut1DOpGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,27 +155,52 @@ void GetLut1DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator,
const unsigned long length = lutData->getArray().getLength();
const unsigned long width = std::min(length, defaultMaxWidth);
const unsigned long height = (length / defaultMaxWidth) + 1;
const unsigned long numChannels = lutData->getArray().getNumColorComponents();
unsigned long numChannels = lutData->getArray().getNumColorComponents();

// Note: The 1D LUT needs a GPU texture for the Look-up table implementation.
// However, the texture type & content may vary based on the number of channels
// i.e. when all channels are identical a F32 Red GPU texture is enough.

const bool singleChannel = (numChannels == 1);

// When shader language is metal, we want to return a texture in
// RGBA format instead of RGB.
if (shaderCreator ->getLanguage() == GPU_LANGUAGE_MSL_2_0 && numChannels == 3)
{
numChannels = 4;
}

// Adjust LUT texture to allow for correct 2d linear interpolation, if needed.

std::vector<float> values;
values.reserve(width * height * numChannels);

if (singleChannel) // i.e. numChannels == 1.
GpuShaderCreator::TextureType channel = GpuShaderCreator::TEXTURE_RED_CHANNEL;
switch (numChannels)
{
case 1:
CreatePaddedRedChannel(width, height, lutData->getArray().getValues(), values);
}
else
{
channel = GpuShaderCreator::TEXTURE_RED_CHANNEL;
break;
case 3:
CreatePaddedLutChannels(width, height, lutData->getArray().getValues(), values);
channel = GpuShaderCreator::TEXTURE_RGB_CHANNEL;
break;
case 4:
{
std::vector<float> paddedChannels;
paddedChannels.reserve(width * height * 3);
CreatePaddedLutChannels(width, height, lutData->getArray().getValues(), paddedChannels);
// Insert a place holder alpha channel with value of 1. This is to support RGBA
// texture format for Metal shading language.
RGBtoRGBATexture(paddedChannels.data(), width * height * 3, values);
channel = GpuShaderCreator::TEXTURE_RGBA_CHANNEL;
break;
}
default:
throw Exception("Invalid number of texture channels.");
break;
}

// Register the RGB LUT.

Expand Down Expand Up @@ -203,8 +228,7 @@ void GetLut1DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator,
GpuShaderText::getSamplerName(name).c_str(),
width,
height,
singleChannel ? GpuShaderCreator::TEXTURE_RED_CHANNEL
: GpuShaderCreator::TEXTURE_RGB_CHANNEL,
channel,
dimensions,
lutData->getConcreteInterpolation(),
&values[0]);
Expand Down
27 changes: 22 additions & 5 deletions src/OpenColorIO/ops/lut3d/Lut3DOpGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "ops/lut3d/Lut3DOpGPU.h"
#include "utils/StringUtils.h"


namespace OCIO_NAMESPACE
{

Expand Down Expand Up @@ -38,12 +37,30 @@ void GetLut3DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator, ConstLut3DO
{
samplerInterpolation = INTERP_NEAREST;
}
// (Using CacheID here to potentially allow reuse of existing textures.)
shaderCreator->add3DTexture(name.c_str(),

if(shaderCreator->getLanguage() == GPU_LANGUAGE_MSL_2_0){
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Minor] Brace opening should be on the next line.

unsigned edgelen = lutData->getGridSize();
std::vector<float> float4AdaptedLutValues;
RGBtoRGBATexture(&lutData->getArray()[0], 3*edgelen*edgelen*edgelen, float4AdaptedLutValues);

shaderCreator->add3DTexture(name.c_str(),
GpuShaderText::getSamplerName(name).c_str(),
lutData->getGridSize(),
edgelen,
GpuShaderCreator::TEXTURE_RGBA_CHANNEL,
samplerInterpolation,
&lutData->getArray()[0]);
float4AdaptedLutValues.data());
}
else
{
// All other languages
// (Using CacheID here to potentially allow reuse of existing textures.)
shaderCreator->add3DTexture(name.c_str(),
GpuShaderText::getSamplerName(name).c_str(),
lutData->getGridSize(),
GpuShaderCreator::TEXTURE_RGB_CHANNEL,
samplerInterpolation,
&lutData->getArray()[0]);
}

{
GpuShaderText ss(shaderCreator->getLanguage());
Expand Down
22 changes: 17 additions & 5 deletions src/bindings/python/PyGpuShaderDesc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ struct Texture3D
std::string m_textureName;
std::string m_samplerName;
unsigned m_edgelen;
GpuShaderDesc::TextureType m_channel;
Interpolation m_interpolation;
GpuShaderDescRcPtr m_shaderDesc;
int m_index;
Expand Down Expand Up @@ -129,6 +130,9 @@ void bindPyGpuShaderDesc(py::module & m)
case GpuShaderDesc::TEXTURE_RGB_CHANNEL:
numChannels = 3;
break;
case GpuShaderDesc::TEXTURE_RGBA_CHANNEL:
numChannels = 4;
break;
default:
throw Exception("Error: Unsupported texture type");
}
Expand Down Expand Up @@ -160,6 +164,7 @@ void bindPyGpuShaderDesc(py::module & m)
const std::string & textureName,
const std::string & samplerName,
unsigned edgelen,
GpuShaderDesc::TextureType channel,
Interpolation interpolation,
const py::buffer & values)
{
Expand All @@ -172,10 +177,11 @@ void bindPyGpuShaderDesc(py::module & m)
self->add3DTexture(textureName.c_str(),
samplerName.c_str(),
edgelen,
channel,
interpolation,
static_cast<float *>(info.ptr));
},
"textureName"_a, "samplerName"_a, "edgeLen"_a, "interpolation"_a, "values"_a,
"textureName"_a, "samplerName"_a, "edgeLen"_a, "channel"_a, "interpolation"_a, "values"_a,
DOC(GpuShaderCreator, add3DTexture))
.def("get3DTextures", [](GpuShaderDescRcPtr & self)
{
Expand Down Expand Up @@ -266,6 +272,9 @@ void bindPyGpuShaderDesc(py::module & m)
case GpuShaderDesc::TEXTURE_RGB_CHANNEL:
numChannels = 3;
break;
case GpuShaderDesc::TEXTURE_RGBA_CHANNEL:
numChannels = 4;
break;
default:
throw Exception("Error: Unsupported texture type");
}
Expand Down Expand Up @@ -323,6 +332,7 @@ void bindPyGpuShaderDesc(py::module & m)
.def_readonly("textureName", &Texture3D::m_textureName)
.def_readonly("samplerName", &Texture3D::m_samplerName)
.def_readonly("edgeLen", &Texture3D::m_edgelen)
.def_readonly("channel", &Texture3D::m_channel)
.def_readonly("interpolation", &Texture3D::m_interpolation)
.def("getValues", [](Texture3D & self)
{
Expand Down Expand Up @@ -350,10 +360,11 @@ void bindPyGpuShaderDesc(py::module & m)
const char * textureName = nullptr;
const char * samplerName = nullptr;
unsigned edgelen;
GpuShaderDesc::TextureType channel;
Interpolation interpolation;
it.m_obj->get3DTexture(i, textureName, samplerName, edgelen, interpolation);
it.m_obj->get3DTexture(i, textureName, samplerName, edgelen, channel, interpolation);

return { textureName, samplerName, edgelen, interpolation, it.m_obj, i };
return { textureName, samplerName, edgelen, channel, interpolation, it.m_obj, i };
})
.def("__iter__", [](Texture3DIterator & it) -> Texture3DIterator &
{
Expand All @@ -366,10 +377,11 @@ void bindPyGpuShaderDesc(py::module & m)
const char * textureName = nullptr;
const char * samplerName = nullptr;
unsigned edgelen;
GpuShaderDesc::TextureType channel;
Interpolation interpolation;
it.m_obj->get3DTexture(i, textureName, samplerName, edgelen, interpolation);
it.m_obj->get3DTexture(i, textureName, samplerName, edgelen, channel, interpolation);

return { textureName, samplerName, edgelen, interpolation, it.m_obj, i };
return { textureName, samplerName, edgelen, channel, interpolation, it.m_obj, i };
});
}

Expand Down
3 changes: 2 additions & 1 deletion src/libutils/oglapphelpers/glsl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,9 @@ void OpenGLBuilder::allocateAllTextures(unsigned startIndex)
const char * textureName = nullptr;
const char * samplerName = nullptr;
unsigned edgelen = 0;
GpuShaderCreator:: TextureType channel = GpuShaderCreator::TEXTURE_RGB_CHANNEL;
Interpolation interpolation = INTERP_LINEAR;
m_shaderDesc->get3DTexture(idx, textureName, samplerName, edgelen, interpolation);
m_shaderDesc->get3DTexture(idx, textureName, samplerName, edgelen, channel, interpolation);

if(!textureName || !*textureName
|| !samplerName || !*samplerName
Expand Down
3 changes: 2 additions & 1 deletion src/libutils/oglapphelpers/metalapp.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,10 @@ vertex VertexOut ColorCorrectionVS(unsigned int vId [[ vertex_id ]])
const char* textureName;
const char* samplerName;
unsigned int edgeLen;
GpuShaderDesc::TextureType channel;
Interpolation interpolation;

shaderDesc->get3DTexture(i, textureName, samplerName, edgeLen, interpolation);
shaderDesc->get3DTexture(i, textureName, samplerName, edgeLen, channel, interpolation);

main << ", texture3d<float> "
<< textureName
Expand Down
3 changes: 2 additions & 1 deletion src/libutils/oglapphelpers/msl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,9 @@ void RGB_to_RGBA(const float* lutValues, int valueCount, std::vector<float>& flo
const char * textureName = nullptr;
const char * samplerName = nullptr;
unsigned edgelen = 0;
GpuShaderDesc::TextureType channel = GpuShaderDesc::TEXTURE_RGBA_CHANNEL;
Interpolation interpolation = INTERP_LINEAR;
m_shaderDesc->get3DTexture(idx, textureName, samplerName, edgelen, interpolation);
m_shaderDesc->get3DTexture(idx, textureName, samplerName, edgelen, channel, interpolation);

Copy link
Collaborator

@doug-walker doug-walker Aug 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Following up on Remi's comment above, GitHub won't let me leave a comment at that line, but on line 233, AllocateTexture3D seems to assume it is getting an RGB array. It calls RGB_to_RGBA on it. Isn't the idea that this should not be necessary anymore?

Are the Metal GPU unit tests passing on your local build?

When I run "ctest -v" on your branch, I get a seg-fault in the Metal tests in the RGB_to_RGBA function.

if(!textureName || !*textureName
|| !samplerName || !*samplerName
Expand Down
Loading
Loading