Skip to content

Commit 3303eaa

Browse files
strukturedclaude
andcommitted
Address review feedback: simplify destructor and merge SetSize loops
- Destructor: delete FBOs first instead of manually detaching (FBO deletion releases driver references to attached textures) - SetSize: merge three separate loops into single detach→resize→reattach per attachment - Remove C++17 structured bindings for C++14 compatibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e4e20b8 commit 3303eaa

1 file changed

Lines changed: 4 additions & 26 deletions

File tree

src/libprojectM/Renderer/Framebuffer.cpp

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,11 @@ Framebuffer::~Framebuffer()
2424
{
2525
if (!m_framebufferIds.empty())
2626
{
27-
// Detach all textures from each framebuffer before destroying them.
28-
// The GL driver keeps internal references to attached textures, so we must
29-
// detach before deleting to avoid use-after-free in driver memory.
30-
for (auto& [index, attachments] : m_attachments)
31-
{
32-
glBindFramebuffer(GL_FRAMEBUFFER, m_framebufferIds.at(index));
33-
for (auto& [type, _] : attachments)
34-
{
35-
glFramebufferTexture2D(GL_FRAMEBUFFER, type, GL_TEXTURE_2D, 0, 0);
36-
}
37-
}
38-
glBindFramebuffer(GL_FRAMEBUFFER, 0);
39-
40-
m_attachments.clear();
41-
27+
// Delete FBOs first — this also releases driver references to attached textures.
4228
glDeleteFramebuffers(static_cast<int>(m_framebufferIds.size()), m_framebufferIds.data());
4329
m_framebufferIds.clear();
30+
31+
m_attachments.clear();
4432
}
4533
}
4634

@@ -104,21 +92,11 @@ bool Framebuffer::SetSize(int width, int height)
10492
for (auto& attachments : m_attachments)
10593
{
10694
Bind(attachments.first);
107-
// First detach all textures from framebuffer before destroying them.
108-
// The GL driver keeps internal references to attached textures, so we must
109-
// detach before deleting to avoid use-after-free in driver memory.
11095
for (auto& texture : attachments.second)
11196
{
97+
// Detach old texture, resize (destroys old and creates new), reattach new.
11298
glFramebufferTexture2D(GL_FRAMEBUFFER, texture.first, GL_TEXTURE_2D, 0, 0);
113-
}
114-
// Now safe to resize (which destroys old textures and creates new ones)
115-
for (auto& texture : attachments.second)
116-
{
11799
texture.second->SetSize(width, height);
118-
}
119-
// Reattach the new textures
120-
for (auto& texture : attachments.second)
121-
{
122100
glFramebufferTexture2D(GL_FRAMEBUFFER, texture.first, GL_TEXTURE_2D, texture.second->Texture()->TextureID(), 0);
123101
}
124102
}

0 commit comments

Comments
 (0)