Skip to content
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.

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

Autodesk: [hdx] Refactor screen space task to remove index buffer #3284

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 3 additions & 71 deletions pxr/imaging/hdx/colorCorrectionTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,6 @@ HdxColorCorrectionTask::~HdxColorCorrectionTask()
}
_textureLUTs.clear();

if (_vertexBuffer) {
_GetHgi()->DestroyBuffer(&_vertexBuffer);
}

if (_indexBuffer) {
_GetHgi()->DestroyBuffer(&_indexBuffer);
}

if (_shaderProgram) {
_DestroyShaderProgram();
}
Expand Down Expand Up @@ -770,9 +762,8 @@ HdxColorCorrectionTask::_CreateShaderResources()
vertDesc.debugName = _tokens->colorCorrectionVertex.GetString();
vertDesc.shaderStage = HgiShaderStageVertex;
HgiShaderFunctionAddStageInput(
&vertDesc, "position", "vec4");
HgiShaderFunctionAddStageInput(
&vertDesc, "uvIn", "vec2");
&vertDesc, "hd_VertexID", "uint",
HgiShaderKeywordTokens->hdVertexID);
HgiShaderFunctionAddStageOutput(
&vertDesc, "gl_Position", "vec4", "position");
HgiShaderFunctionAddStageOutput(
Expand Down Expand Up @@ -880,38 +871,6 @@ HdxColorCorrectionTask::_CreateShaderResources()
return true;
}

bool
HdxColorCorrectionTask::_CreateBufferResources()
{
if (_vertexBuffer) {
return true;
}

// A larger-than screen triangle made to fit the screen.
constexpr float vertData[][6] =
{ { -1, 3, 0, 1, 0, 2 },
{ -1, -1, 0, 1, 0, 0 },
{ 3, -1, 0, 1, 2, 0 } };

HgiBufferDesc vboDesc;
vboDesc.debugName = "HdxColorCorrectionTask VertexBuffer";
vboDesc.usage = HgiBufferUsageVertex;
vboDesc.initialData = vertData;
vboDesc.byteSize = sizeof(vertData);
vboDesc.vertexStride = sizeof(vertData[0]);
_vertexBuffer = _GetHgi()->CreateBuffer(vboDesc);

static const int32_t indices[3] = {0,1,2};

HgiBufferDesc iboDesc;
iboDesc.debugName = "HdxColorCorrectionTask IndexBuffer";
iboDesc.usage = HgiBufferUsageIndex32;
iboDesc.initialData = indices;
iboDesc.byteSize = sizeof(indices);
_indexBuffer = _GetHgi()->CreateBuffer(iboDesc);
return true;
}

bool
HdxColorCorrectionTask::_CreateResourceBindings(
HgiTextureHandle const &aovTexture)
Expand Down Expand Up @@ -966,29 +925,6 @@ HdxColorCorrectionTask::_CreatePipeline(HgiTextureHandle const& aovTexture)
desc.debugName = "ColorCorrection Pipeline";
desc.shaderProgram = _shaderProgram;

// Describe the vertex buffer
HgiVertexAttributeDesc posAttr;
posAttr.format = HgiFormatFloat32Vec3;
posAttr.offset = 0;
posAttr.shaderBindLocation = 0;

HgiVertexAttributeDesc uvAttr;
uvAttr.format = HgiFormatFloat32Vec2;
uvAttr.offset = sizeof(float) * 4; // after posAttr
uvAttr.shaderBindLocation = 1;

size_t bindSlots = 0;

HgiVertexBufferDesc vboDesc;

vboDesc.bindingIndex = bindSlots++;
vboDesc.vertexStride = sizeof(float) * 6; // pos, uv
vboDesc.vertexAttributes.clear();
vboDesc.vertexAttributes.push_back(posAttr);
vboDesc.vertexAttributes.push_back(uvAttr);

desc.vertexBuffers.push_back(std::move(vboDesc));

// Depth test and write can be off since we only colorcorrect the color aov.
desc.depthState.depthTestEnabled = false;
desc.depthState.depthWriteEnabled = false;
Expand Down Expand Up @@ -1058,7 +994,6 @@ HdxColorCorrectionTask::_ApplyColorCorrection(
gfxCmds->PushDebugGroup("ColorCorrection");
gfxCmds->BindResources(_resourceBindings);
gfxCmds->BindPipeline(_pipeline);
gfxCmds->BindVertexBuffers({{_vertexBuffer, 0, 0}});

// Update viewport/screen size
const GfVec4i vp(0, 0, dimensions[0], dimensions[1]);
Expand All @@ -1067,7 +1002,7 @@ HdxColorCorrectionTask::_ApplyColorCorrection(
_SetConstants(gfxCmds.get());
gfxCmds->SetViewport(vp);

gfxCmds->DrawIndexed(_indexBuffer, 3, 0, 0, 1, 0);
gfxCmds->Draw(3, 0, 1, 0);
gfxCmds->PopDebugGroup();

// Done recording commands, submit work.
Expand Down Expand Up @@ -1153,9 +1088,6 @@ HdxColorCorrectionTask::Execute(HdTaskContext* ctx)
// So, no layout transition there.
aovTexture->SubmitLayoutChange(HgiTextureUsageBitsShaderRead);

if (!TF_VERIFY(_CreateBufferResources())) {
return;
}
if (!TF_VERIFY(_CreateAovSampler())) {
return;
}
Expand Down
5 changes: 0 additions & 5 deletions pxr/imaging/hdx/colorCorrectionTask.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ class HdxColorCorrectionTask : public HdxTask
std::string _CreateOpenColorIOShaderCode(std::string &ocioGpuShaderText,
HgiShaderFunctionDesc &fragDesc);

// Utility function to create buffer resources.
bool _CreateBufferResources();

// Utility to create resource bindings
bool _CreateResourceBindings(HgiTextureHandle const& aovTexture);

Expand Down Expand Up @@ -178,8 +175,6 @@ class HdxColorCorrectionTask : public HdxTask
_OCIOResources _ocioResources;

HgiAttachmentDesc _attachment0;
HgiBufferHandle _indexBuffer;
HgiBufferHandle _vertexBuffer;
HgiSamplerHandle _aovSampler;

struct TextureSamplerInfo
Expand Down
13 changes: 11 additions & 2 deletions pxr/imaging/hdx/effectsShader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,17 @@ HdxEffectsShader::_CreateAndSubmitGraphicsCmds(
_gfxCmds.reset();
}


void
HdxEffectsShader::_DrawWithoutVertexBuffer(
uint32_t vertexCount,
uint32_t baseVertex,
uint32_t instanceCount,
uint32_t baseInstance)
{
_gfxCmds->Draw(vertexCount, baseVertex, instanceCount, baseInstance);
}

void
HdxEffectsShader::_DrawNonIndexed(
const HgiBufferHandle& vertexBuffer,
Expand All @@ -345,7 +356,6 @@ HdxEffectsShader::_DrawNonIndexed(
uint32_t baseInstance)
{
_gfxCmds->BindVertexBuffers({{vertexBuffer, 0, 0}});

_gfxCmds->Draw(vertexCount, baseVertex, instanceCount, baseInstance);
}

Expand All @@ -360,7 +370,6 @@ HdxEffectsShader::_DrawIndexed(
uint32_t baseInstance)
{
_gfxCmds->BindVertexBuffers({{vertexBuffer, 0, 0}});

_gfxCmds->DrawIndexed(indexBuffer, indexCount, indexBufferByteOffset,
baseVertex, instanceCount, baseInstance);
}
Expand Down
8 changes: 8 additions & 0 deletions pxr/imaging/hdx/effectsShader.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ class HdxEffectsShader
HDX_API
virtual void _RecordDrawCmds() = 0;

// Invokes HgiGraphicsCmds::Draw.
HDX_API
void _DrawWithoutVertexBuffer(
uint32_t vertexCount,
uint32_t baseVertex,
uint32_t instanceCount,
uint32_t baseInstance);

// Sets the vertex buffer and invokes HgiGraphicsCmds::Draw.
HDX_API
void _DrawNonIndexed(
Expand Down
105 changes: 5 additions & 100 deletions pxr/imaging/hdx/fullscreenShader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ HdxFullscreenShader::HdxFullscreenShader(
const std::string& debugName)
: HdxEffectsShader(hgi, debugName.empty() ? "HdxFullscreenShader" : debugName)
{
// Create descriptor for vertex pos and uvs
_SetVertexBufferDescriptor();

// Depth test and write must be on since we may want to transfer depth.
// Depth test must be on because when off it also disables depth writes.
// Instead we set the compare function to always.
Expand Down Expand Up @@ -75,14 +72,6 @@ HdxFullscreenShader::~HdxFullscreenShader()
return;
}

if (_vertexBuffer) {
hgi->DestroyBuffer(&_vertexBuffer);
}

if (_indexBuffer) {
hgi->DestroyBuffer(&_indexBuffer);
}

if (_shaderProgram) {
_DestroyShaderProgram(&_shaderProgram);
}
Expand Down Expand Up @@ -141,11 +130,9 @@ HdxFullscreenShader::SetProgram(
HgiShaderFunctionDesc vertDesc;
vertDesc.debugName = _tokens->fullscreenVertex;
vertDesc.shaderStage = HgiShaderStageVertex;

HgiShaderFunctionAddStageInput(
&vertDesc, "position", "vec4", "position");
HgiShaderFunctionAddStageInput(
&vertDesc, "uvIn", "vec2");
&vertDesc, "hd_VertexID", "uint",
HgiShaderKeywordTokens->hdVertexID);
HgiShaderFunctionAddStageOutput(
&vertDesc, "gl_Position", "vec4", "position");
HgiShaderFunctionAddStageOutput(
Expand Down Expand Up @@ -225,64 +212,6 @@ HdxFullscreenShader::SetShaderConstants(
_SetShaderConstants(byteSize, data, HgiShaderStageFragment);
}

void
HdxFullscreenShader::_CreateBufferResources()
{
if (_vertexBuffer) {
return;
}

/* For the fullscreen pass, we draw a triangle:
*
* |\
* |_\
* | |\
* |_|_\
*
* The vertices are at (-1, 3) [top left]; (-1, -1) [bottom left];
* and (3, -1) [bottom right]; UVs are assigned so that the bottom left
* is (0,0) and the clipped vertices are 2 on their axis, so that:
* x=-1 => s = 0; x = 3 => s = 2, which means x = 1 => s = 1.
*
* This maps the texture space [0,1]^2 to the clip space XY [-1,1]^2.
* The parts of the triangle extending past NDC space are clipped before
* rasterization.
*
* This has the advantage (over rendering a quad) that we don't render
* the diagonal twice.
*
* Note that we're passing in NDC positions, and we don't expect the vertex
* shader to transform them. Also note: the fragment shader can optionally
* read depth from a texture, but otherwise the depth is -1, meaning near
* plane.
*/
constexpr size_t elementsPerVertex = 6;
constexpr size_t vertDataCount = elementsPerVertex * 3;
constexpr float vertData[vertDataCount] =
{ -1, 3, 0, 1, 0, 2,
-1, -1, 0, 1, 0, 0,
3, -1, 0, 1, 2, 0};

HgiBufferDesc vboDesc;
vboDesc.debugName = "HdxFullscreenShader VertexBuffer";
vboDesc.usage = HgiBufferUsageVertex;
vboDesc.initialData = vertData;
vboDesc.byteSize = sizeof(vertData);
vboDesc.vertexStride = elementsPerVertex * sizeof(vertData[0]);
_vertexBuffer = _GetHgi()->CreateBuffer(vboDesc);

constexpr int32_t indices[3] = {0,1,2};

HgiBufferDesc iboDesc;
iboDesc.debugName = "HdxFullscreenShader IndexBuffer";
iboDesc.usage = HgiBufferUsageIndex32;
iboDesc.initialData = indices;
iboDesc.byteSize = sizeof(indices);
_indexBuffer = _GetHgi()->CreateBuffer(iboDesc);

_SetPrimitiveType(HgiPrimitiveTypeTriangleList);
}

void
HdxFullscreenShader::BindTextures(
HgiTextureHandleVector const& textures,
Expand Down Expand Up @@ -346,28 +275,6 @@ HdxFullscreenShader::_SetResourceBindings()
_SetBufferBindings(bufferBindings);
}

void
HdxFullscreenShader::_SetVertexBufferDescriptor()
{
// Describe the vertex buffer
HgiVertexAttributeDesc posAttr;
posAttr.format = HgiFormatFloat32Vec4;
posAttr.offset = 0;
posAttr.shaderBindLocation = 0;

HgiVertexAttributeDesc uvAttr;
uvAttr.format = HgiFormatFloat32Vec2;
uvAttr.offset = sizeof(float) * 4; // after posAttr
uvAttr.shaderBindLocation = 1;

HgiVertexBufferDescVector vboDescs(1);
vboDescs[0].bindingIndex = 0;
vboDescs[0].vertexStride = sizeof(float) * 6; // pos, uv
vboDescs[0].vertexAttributes = { posAttr, uvAttr };

_SetVertexBufferDescs(vboDescs);
}

HgiSamplerHandle
HdxFullscreenShader::_GetDefaultSampler()
{
Expand Down Expand Up @@ -427,6 +334,7 @@ HdxFullscreenShader::_SetDefaultProgram(
HgiShaderFunctionAddTexture(
&fragDesc, "colorIn", /*bindIndex = */0);


if (writeDepth) {
HgiShaderFunctionAddStageOutput(
&fragDesc, "gl_FragDepth", "float", "depth(any)");
Expand All @@ -452,10 +360,7 @@ HdxFullscreenShader::_Draw(
_SetDefaultProgram(writeDepth);
}

// Create draw buffers if they haven't been created yet.
if (!_vertexBuffer) {
_CreateBufferResources();
}
_SetPrimitiveType(HgiPrimitiveTypeTriangleList);

// Set or update the resource bindings (textures may have changed)
_SetResourceBindings();
Expand Down Expand Up @@ -485,7 +390,7 @@ HdxFullscreenShader::_Draw(
void
HdxFullscreenShader::_RecordDrawCmds()
{
_DrawIndexed(_vertexBuffer, _indexBuffer, 3, 0, 0, 1, 0);
_DrawWithoutVertexBuffer(3, 0, 1, 0);
}

PXR_NAMESPACE_CLOSE_SCOPE
7 changes: 0 additions & 7 deletions pxr/imaging/hdx/fullscreenShader.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,9 @@ class HdxFullscreenShader : public HdxEffectsShader
HdxFullscreenShader(const HdxFullscreenShader&) = delete;
HdxFullscreenShader& operator=(const HdxFullscreenShader&) = delete;

// Utility function to create buffer resources.
void _CreateBufferResources();

// Utility to set resource bindings.
void _SetResourceBindings();

// Utility to create default vertex buffer descriptor.
void _SetVertexBufferDescriptor();

// Utility to create and get the default texture sampler.
HgiSamplerHandle _GetDefaultSampler();

Expand All @@ -182,7 +176,6 @@ class HdxFullscreenShader : public HdxEffectsShader
TfToken _glslfxPath;
TfToken _shaderName;

HgiBufferHandle _indexBuffer;
HgiBufferHandle _vertexBuffer;
HgiShaderProgramHandle _shaderProgram;
HgiSamplerHandle _defaultSampler;
Expand Down
4 changes: 2 additions & 2 deletions pxr/imaging/hdx/shaders/colorCorrection.glslfx
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@

void main(void)
{
gl_Position = position;
uvOut = uvIn;
uvOut = vec2((hd_VertexID << 1) & 2, hd_VertexID & 2);
gl_Position = vec4(uvOut * 2.0f + -1.0f, 0.0f, 1.0f);
}

-- glsl ColorCorrection.Fragment
Expand Down
4 changes: 2 additions & 2 deletions pxr/imaging/hdx/shaders/fullscreen.glslfx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@

void main(void)
{
gl_Position = position;
uvOut = uvIn;
uvOut = vec2((hd_VertexID << 1) & 2, hd_VertexID & 2);
gl_Position = vec4(uvOut * 2.0f + -1.0f, 0.0f, 1.0f);
}

-- glsl Composite.FragmentNoDepth
Expand Down
Loading