From 41857178b00f3185a0522aa330ea6ddfe3640cde Mon Sep 17 00:00:00 2001 From: Einar Forselv Date: Fri, 6 Oct 2023 20:40:01 +0200 Subject: [PATCH 1/3] Bug: leaving random glReadBuffer() state This can cause other read and blit operations to fail leaving a cryptic error message. --- arcade/gl/framebuffer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/arcade/gl/framebuffer.py b/arcade/gl/framebuffer.py index 6a4744e92..775df50e1 100644 --- a/arcade/gl/framebuffer.py +++ b/arcade/gl/framebuffer.py @@ -433,6 +433,7 @@ def read( x, y, width, height = 0, 0, self._width, self._height data = (gl.GLubyte * (components * component_size * width * height))(0) gl.glReadPixels(x, y, width, height, base_format, pixel_type, data) + gl.glReadBuffer(gl.GL_COLOR_ATTACHMENT0) # Reset to default return string_at(data, len(data)) From 183e9e96fa34c9967b729032d2365cc1698719b4 Mon Sep 17 00:00:00 2001 From: Einar Forselv Date: Fri, 6 Oct 2023 21:07:33 +0200 Subject: [PATCH 2/3] copy_framebuffer: Support basic parameters * Select what color layer to read from * Make depth buffer optional --- arcade/gl/context.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/arcade/gl/context.py b/arcade/gl/context.py index a20979007..fac3aea0c 100644 --- a/arcade/gl/context.py +++ b/arcade/gl/context.py @@ -777,9 +777,17 @@ def flush(self) -> None: # Various utility methods - def copy_framebuffer(self, src: Framebuffer, dst: Framebuffer): + def copy_framebuffer( + self, + src: Framebuffer, + dst: Framebuffer, + src_layer_id: int = 0, + depth: bool = True, + ): """ Copies/blits a framebuffer to another one. + We can select one color attachment to copy plus + an optional depth attachment. This operation has many restrictions to ensure it works across different platforms and drivers: @@ -791,15 +799,20 @@ def copy_framebuffer(self, src: Framebuffer, dst: Framebuffer): :param src: The framebuffer to copy from :param dst: The framebuffer we copy to + :param glBlitNamedFramebuffer: The color attachment layer to copy from + :param depth: Also copy depth attachment if present """ + # TODO: Make make this more configurable (target) + gl.glReadBuffer(gl.GL_COLOR_ATTACHMENT0 + src_layer_id) gl.glBindFramebuffer(gl.GL_READ_FRAMEBUFFER, src._glo) gl.glBindFramebuffer(gl.GL_DRAW_FRAMEBUFFER, dst._glo) gl.glBlitFramebuffer( 0, 0, src.width, src.height, # Make source and dest size the same 0, 0, src.width, src.height, - gl.GL_COLOR_BUFFER_BIT | gl.GL_DEPTH_BUFFER_BIT, + gl.GL_COLOR_BUFFER_BIT | gl.GL_DEPTH_BUFFER_BIT if depth else 0, gl.GL_NEAREST, ) + gl.glReadBuffer(gl.GL_COLOR_ATTACHMENT0) # Reset read buffer self.active_framebuffer.use(force=True) # --- Resource methods --- From 1cc590d19c0406f4a9bea417ddaab88a751fe158 Mon Sep 17 00:00:00 2001 From: Einar Forselv Date: Fri, 6 Oct 2023 21:17:53 +0200 Subject: [PATCH 3/3] Improve parameter name --- arcade/gl/context.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arcade/gl/context.py b/arcade/gl/context.py index fac3aea0c..c5c07cc65 100644 --- a/arcade/gl/context.py +++ b/arcade/gl/context.py @@ -781,7 +781,7 @@ def copy_framebuffer( self, src: Framebuffer, dst: Framebuffer, - src_layer_id: int = 0, + src_attachment_index: int = 0, depth: bool = True, ): """ @@ -799,12 +799,12 @@ def copy_framebuffer( :param src: The framebuffer to copy from :param dst: The framebuffer we copy to - :param glBlitNamedFramebuffer: The color attachment layer to copy from + :param src_attachment_index: The color attachment to copy from :param depth: Also copy depth attachment if present """ # TODO: Make make this more configurable (target) - gl.glReadBuffer(gl.GL_COLOR_ATTACHMENT0 + src_layer_id) gl.glBindFramebuffer(gl.GL_READ_FRAMEBUFFER, src._glo) + gl.glReadBuffer(gl.GL_COLOR_ATTACHMENT0 + src_attachment_index) gl.glBindFramebuffer(gl.GL_DRAW_FRAMEBUFFER, dst._glo) gl.glBlitFramebuffer( 0, 0, src.width, src.height, # Make source and dest size the same