Repository navigation
Conversation
…buffer ConfigureFBO in the GLES blit commands treated every wrapped texture as the default framebuffer. A texture created with TextureGLES::WrapTexture wraps a texture name, not a framebuffer, so copying it to a buffer read whatever the default framebuffer held. A texture made by WrapFBO also got framebuffer 0 bound instead of the framebuffer it wraps. TextureGLES::IsWrappedFBO now tells the two apart. A wrapped framebuffer is bound as is and never deleted; a wrapped texture name is attached to a temporary framebuffer and can be read. Writing into a wrapped texture name stays unsupported, as it already was for uploads and SetContents: a blit into one and a render pass with one as its color attachment are refused with a validation error instead of drawing into whatever framebuffer happens to be bound. The mock GL proc table now forwards glBlitFramebuffer so the tests can check it.
github-actions
Bot
added
engine
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the GLES backend
TextureGLES::IsWrapped()is true for two different things: a texture made byWrapFBO, which stands for a framebuffer, and one made byWrapTexture, which is an ordinary GL texture name owned by someone else.ConfigureFBOin the blit commands andRenderPassGLEStreated both as a wrapped framebuffer and bound framebuffer 0. For a wrapped texture name that means a texture-to-buffer copy runsglReadPixelson the default framebuffer and hands back the screen's pixels, with no error; a texture-to-texture blit reads or writes the wrong framebuffer the same way, and a wrapped FBO other than 0 is read from 0 too.TextureGLES::IsWrappedFBO()now tells the two apart, and both call sites use it. A wrapped framebuffer binds the FBO it wraps and is still never deleted. A wrapped texture name is read through a temporary FBO, like any texture the engine made.I kept this read-only. Uploads and
SetContentsalready refuse wrapped textures, so a blit into a wrapped texture name, or a render pass that uses one as its color attachment, is refused here with a validation error instead of writing into whatever framebuffer is bound. Writing into textures the caller owns is what #192059 is about, and this change stays out of it.The mock GL proc table now forwards
glBlitFramebuffer, so the tests can check that a refused blit never reaches GL. That is the test infrastructure this needs.Where it was verified. macOS arm64,
host_debug_unopt_arm64, rebased on current main. Five new tests inblit_command_gles_unittests.ccandrender_pass_gles_unittests.ccfail without the change (framebuffer 0 bound instead of 5, nothing attached for the texture name, the destination blit and the render pass not refused) and pass with it:impeller_unittests --gtest_filter='BlitCommandGLESTest.*:RenderPassGLES*'29/29, and the wider--gtest_filter='*GLES*:*/OpenGLES:*/OpenGLESSDF'1585/1585. I found the bug while reading back imported GL textures for #194142.Fixes #194150
Pre-launch Checklist
///).