tmaq
/
tornavis
1
0
Fork 0
Code Issues 2 Pull Requests 29 Packages Projects 24 Releases Wiki Activity

GPU: Assert framebuffer operations match attachment layout 

Ensure attachment states and load/store configs don't get out of sync
with the framebuffer layout.
In theory, a Framebuffer could have empty attachments interleaved with
valid ones so checking just the attachments "length" is not enough.
What this does instead is to ensure that valid attachments have a valid
config and that null attachments either don't have a matching config or
have an IGNORE/DONT_CARE one.

Pull Request: https://projects.blender.org/blender/blender/pulls/117073
This commit is contained in:
Miguel Pozo 2024-01-15 13:25:20 +01:00
parent a4331be6c4
commit 333a5b513b
8 changed files with 71 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
source/blender/gpu/dummy/dummy_framebuffer.hh
View File

@ -35,8 +35,8 @@ class DummyFrameBuffer : public FrameBuffer {
void attachment_set_loadstore_op(GPUAttachmentType /*type*/, GPULoadStore /*ls*/) override {}
void subpass_transition(const GPUAttachmentState /*depth_attachment_state*/,
Span<GPUAttachmentState> /*color_attachment_states*/) override{};
void subpass_transition_impl(const GPUAttachmentState /*depth_attachment_state*/,
Span<GPUAttachmentState> /*color_attachment_states*/) override{};
void read(eGPUFrameBufferBits /*planes*/,
eGPUDataFormat /*format*/,

50
source/blender/gpu/intern/gpu_framebuffer.cc
View File

@ -127,25 +127,67 @@ void FrameBuffer::attachment_remove(GPUAttachmentType type)
dirty_attachments_ = true;
}
void FrameBuffer::subpass_transition(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states)
{
/* NOTE: Depth is not supported as input attachment because the Metal API doesn't support it and
* because depth is not compatible with the framebuffer fetch implementation. */
BLI_assert(depth_attachment_state != GPU_ATTACHEMENT_READ);
if (!attachments_[GPU_FB_DEPTH_ATTACHMENT].tex &&
!attachments_[GPU_FB_DEPTH_STENCIL_ATTACHMENT].tex)
{
BLI_assert(depth_attachment_state == GPU_ATTACHEMENT_IGNORE);
}
BLI_assert(color_attachment_states.size() <= GPU_FB_MAX_COLOR_ATTACHMENT);
for (int i : IndexRange(GPU_FB_MAX_COLOR_ATTACHMENT)) {
GPUAttachmentType type = GPU_FB_COLOR_ATTACHMENT0 + i;
if (this->attachments_[type].tex) {
BLI_assert(i < color_attachment_states.size());
}
else {
BLI_assert(i >= color_attachment_states.size() ||
color_attachment_states[i] == GPU_ATTACHEMENT_IGNORE);
}
}
subpass_transition_impl(depth_attachment_state, color_attachment_states);
}
void FrameBuffer::load_store_config_array(const GPULoadStore *load_store_actions, uint actions_len)
{
/* Follows attachment structure of GPU_framebuffer_config_array/GPU_framebuffer_ensure_config */
const GPULoadStore &depth_action = load_store_actions[0];
Span<GPULoadStore> color_attachment_actions(load_store_actions + 1, actions_len - 1);
BLI_assert(color_attachment_actions.size() <= GPU_FB_MAX_COLOR_ATTACHMENT);
if (!attachments_[GPU_FB_DEPTH_ATTACHMENT].tex &&
!attachments_[GPU_FB_DEPTH_STENCIL_ATTACHMENT].tex)
{
BLI_assert(depth_action.load_action == GPU_LOADACTION_DONT_CARE &&
depth_action.store_action == GPU_STOREACTION_DONT_CARE);
}
if (this->attachments_[GPU_FB_DEPTH_STENCIL_ATTACHMENT].tex) {
this->attachment_set_loadstore_op(GPU_FB_DEPTH_STENCIL_ATTACHMENT, depth_action);
}
if (this->attachments_[GPU_FB_DEPTH_ATTACHMENT].tex) {
this->attachment_set_loadstore_op(GPU_FB_DEPTH_ATTACHMENT, depth_action);
}
GPUAttachmentType type = GPU_FB_COLOR_ATTACHMENT0;
for (const GPULoadStore &action : color_attachment_actions) {
for (int i : IndexRange(GPU_FB_MAX_COLOR_ATTACHMENT)) {
GPUAttachmentType type = GPU_FB_COLOR_ATTACHMENT0 + i;
if (this->attachments_[type].tex) {
this->attachment_set_loadstore_op(type, action);
BLI_assert(i < color_attachment_actions.size());
this->attachment_set_loadstore_op(type, color_attachment_actions[i]);
}
else {
BLI_assert(i >= color_attachment_actions.size() ||
(color_attachment_actions[i].load_action == GPU_LOADACTION_DONT_CARE &&
color_attachment_actions[i].store_action == GPU_STOREACTION_DONT_CARE));
}
++type;
}
}

9
source/blender/gpu/intern/gpu_framebuffer_private.hh
View File

@ -129,8 +129,13 @@ class FrameBuffer {
int dst_offset_x,
int dst_offset_y) = 0;
virtual void subpass_transition(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states) = 0;
protected:
virtual void subpass_transition_impl(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states) = 0;
public:
void subpass_transition(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states);
void load_store_config_array(const GPULoadStore *load_store_actions, uint actions_len);

6
source/blender/gpu/metal/mtl_framebuffer.hh
View File

@ -153,9 +153,11 @@ class MTLFrameBuffer : public FrameBuffer {
int dst_offset_x,
int dst_offset_y) override;
void subpass_transition(const GPUAttachmentState /*depth_attachment_state*/,
Span<GPUAttachmentState> /*color_attachment_states*/) override{};
protected:
void subpass_transition_impl(const GPUAttachmentState /*depth_attachment_state*/,
Span<GPUAttachmentState> /*color_attachment_states*/) override{};
public:
void apply_state();
/* State. */

7
source/blender/gpu/opengl/gl_framebuffer.cc
View File

@ -226,12 +226,9 @@ void GLFrameBuffer::update_attachments()
}
}
void GLFrameBuffer::subpass_transition(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states)
void GLFrameBuffer::subpass_transition_impl(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states)
{
/* NOTE: Depth is not supported as input attachment because the Metal API doesn't support it and
* because depth is not compatible with the framebuffer fetch implementation. */
BLI_assert(depth_attachment_state != GPU_ATTACHEMENT_READ);
GPU_depth_mask(depth_attachment_state == GPU_ATTACHEMENT_WRITE);
bool any_read = false;

6
source/blender/gpu/opengl/gl_framebuffer.hh
View File

@ -81,9 +81,11 @@ class GLFrameBuffer : public FrameBuffer {
/* Attachment load-stores are currently no-op's in OpenGL. */
void attachment_set_loadstore_op(GPUAttachmentType type, GPULoadStore ls) override;
void subpass_transition(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states) override;
protected:
void subpass_transition_impl(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states) override;
public:
void read(eGPUFrameBufferBits planes,
eGPUDataFormat format,
const int area[4],

4
source/blender/gpu/vulkan/vk_framebuffer.cc
View File

@ -229,8 +229,8 @@ void VKFrameBuffer::attachment_set_loadstore_op(GPUAttachmentType /*type*/, GPUL
/** \name Sub-pass transition
* \{ */
void VKFrameBuffer::subpass_transition(const GPUAttachmentState /*depth_attachment_state*/,
Span<GPUAttachmentState> /*color_attachment_states*/)
void VKFrameBuffer::subpass_transition_impl(const GPUAttachmentState /*depth_attachment_state*/,
Span<GPUAttachmentState> /*color_attachment_states*/)
{
NOT_YET_IMPLEMENTED;
}

6
source/blender/gpu/vulkan/vk_framebuffer.hh
View File

@ -59,9 +59,11 @@ class VKFrameBuffer : public FrameBuffer {
void attachment_set_loadstore_op(GPUAttachmentType type, GPULoadStore /*ls*/) override;
void subpass_transition(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states) override;
protected:
void subpass_transition_impl(const GPUAttachmentState depth_attachment_state,
Span<GPUAttachmentState> color_attachment_states) override;
public:
void read(eGPUFrameBufferBits planes,
eGPUDataFormat format,
const int area[4],