From 9d8f5869881e3effa3cb2bb7ac50b75f54fe2213 Mon Sep 17 00:00:00 2001 From: Serdar Kocdemir Date: Thu, 9 Apr 2026 14:42:13 +0100 Subject: [PATCH 1/3] Avoid GL related crashes on device pose window with Vulkan composition Ensure device pose window won't call fatal errors and avoid GL rendering instead when trying to get color buffer contents for rendering foldable devices. Bug: 477193075 Test: manual, use device pose extended window controls with -feature GuestAngle Change-Id: Ic1436647ad4fe4bdaaa2ff0c61a985d7513eb723 --- host/color_buffer.cpp | 113 ++++++++++++++++++++---------------- host/color_buffer.h | 12 ++-- host/frame_buffer.cpp | 10 ++++ host/gl/color_buffer_gl.cpp | 12 ++-- host/gl/color_buffer_gl.h | 5 +- 5 files changed, 87 insertions(+), 65 deletions(-) diff --git a/host/color_buffer.cpp b/host/color_buffer.cpp index 698721a76..3b482f373 100644 --- a/host/color_buffer.cpp +++ b/host/color_buffer.cpp @@ -87,21 +87,21 @@ class ColorBuffer::Impl : public LazySnapshotObj { std::optional exportBlob(); #if GFXSTREAM_ENABLE_HOST_GLES - GLuint glOpGetTexture(); + bool canUseGlOps(); bool glOpBlitFromCurrentReadBuffer(); bool glOpBindToTexture(); bool glOpBindToTexture2(); bool glOpBindToRenderbuffer(); - void glOpReadback(unsigned char* img, bool readbackBgra); - void glOpReadbackAsync(GLuint buffer, bool readbackBgra); + bool glOpReadback(unsigned char* img, bool readbackBgra); + bool glOpReadbackAsync(GLuint buffer, bool readbackBgra); bool glOpImportEglNativePixmap(void* pixmap, bool preserveContent); - void glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, GfxstreamFormat texturesFormat, + bool glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, GfxstreamFormat texturesFormat, GLuint* textures); bool glOpReadContents(size_t* outNumBytes, void* outContents); bool glOpIsFastBlitSupported() const; - void glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, + bool glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, const std::optional>& colorTransform); - void glOpPostViewportScaledWithOverlay( + bool glOpPostViewportScaledWithOverlay( float rotation, float dx, float dy, float scaleX, float scaleY, const std::optional>& colorTransform); #endif @@ -268,7 +268,7 @@ void ColorBuffer::Impl::readToBytes( return; } - GFXSTREAM_FATAL("No ColorBuffer impl"); + GFXSTREAM_FATAL("%s: No ColorBuffer impl", __func__); } void ColorBuffer::Impl::readToBytesScaled( @@ -289,7 +289,7 @@ void ColorBuffer::Impl::readToBytesScaled( return; } - GFXSTREAM_FATAL("%s: Unimplemented", __func__); + GFXSTREAM_FATAL("%s: No ColorBuffer impl", __func__); } void ColorBuffer::Impl::readYuvToBytes(int x, int y, int width, int height, void* outPixels, @@ -308,7 +308,7 @@ void ColorBuffer::Impl::readYuvToBytes(int x, int y, int width, int height, void return; } - GFXSTREAM_FATAL("No ColorBuffer impl"); + GFXSTREAM_FATAL("%s: No ColorBuffer impl", __func__); } bool ColorBuffer::Impl::updateFromBytes(int x, int y, int width, int height, GfxstreamFormat pixelsFormat, @@ -329,7 +329,7 @@ bool ColorBuffer::Impl::updateFromBytes(int x, int y, int width, int height, Gfx return mColorBufferVk->updateFromBytes(x, y, width, height, pixels); } - GFXSTREAM_FATAL("No ColorBuffer impl"); + GFXSTREAM_FATAL("%s: No ColorBuffer impl", __func__); return false; } @@ -351,19 +351,21 @@ std::unique_ptr ColorBuffer::Impl::borrowForComposition(UsedA case UsedApi::kGl: { #if GFXSTREAM_ENABLE_HOST_GLES if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return nullptr; } return mColorBufferGl->getBorrowedImageInfo(); #endif } case UsedApi::kVk: { if (!mColorBufferVk) { - GFXSTREAM_FATAL("ColorBufferVk not available"); + GFXSTREAM_ERROR("%s: ColorBufferVk not available", __func__); + return nullptr; } return mColorBufferVk->borrowForComposition(isTarget); } } - GFXSTREAM_FATAL("%s: Unimplemented", __func__); + GFXSTREAM_ERROR("%s: Unimplemented", __func__); return nullptr; } @@ -372,19 +374,21 @@ std::unique_ptr ColorBuffer::Impl::borrowForDisplay(UsedApi a case UsedApi::kGl: { #if GFXSTREAM_ENABLE_HOST_GLES if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return nullptr; } return mColorBufferGl->getBorrowedImageInfo(); #endif } case UsedApi::kVk: { if (!mColorBufferVk) { - GFXSTREAM_FATAL("ColorBufferVk not available"); + GFXSTREAM_ERROR("%s: ColorBufferVk not available", __func__); + return nullptr; } return mColorBufferVk->borrowForDisplay(); } } - GFXSTREAM_FATAL("%s: Unimplemented", __func__); + GFXSTREAM_ERROR("%s: Unimplemented", __func__); return nullptr; } @@ -511,9 +515,14 @@ std::optional ColorBuffer::Impl::exportBlob() { } #if GFXSTREAM_ENABLE_HOST_GLES +bool ColorBuffer::Impl::canUseGlOps() { + return (mColorBufferGl != nullptr); +} + bool ColorBuffer::Impl::glOpBlitFromCurrentReadBuffer() { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } touch(); @@ -523,7 +532,8 @@ bool ColorBuffer::Impl::glOpBlitFromCurrentReadBuffer() { bool ColorBuffer::Impl::glOpBindToTexture() { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } touch(); @@ -533,7 +543,8 @@ bool ColorBuffer::Impl::glOpBindToTexture() { bool ColorBuffer::Impl::glOpBindToTexture2() { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } return mColorBufferGl->bindToTexture2(); @@ -541,7 +552,8 @@ bool ColorBuffer::Impl::glOpBindToTexture2() { bool ColorBuffer::Impl::glOpBindToRenderbuffer() { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } touch(); @@ -549,19 +561,10 @@ bool ColorBuffer::Impl::glOpBindToRenderbuffer() { return mColorBufferGl->bindToRenderbuffer(); } -GLuint ColorBuffer::Impl::glOpGetTexture() { - if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); - } - - touch(); - - return mColorBufferGl->getTexture(); -} - -void ColorBuffer::Impl::glOpReadback(unsigned char* img, bool readbackBgra) { +bool ColorBuffer::Impl::glOpReadback(unsigned char* img, bool readbackBgra) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } touch(); @@ -569,29 +572,32 @@ void ColorBuffer::Impl::glOpReadback(unsigned char* img, bool readbackBgra) { return mColorBufferGl->readback(img, readbackBgra); } -void ColorBuffer::Impl::glOpReadbackAsync(GLuint buffer, bool readbackBgra) { +bool ColorBuffer::Impl::glOpReadbackAsync(GLuint buffer, bool readbackBgra) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } touch(); - mColorBufferGl->readbackAsync(buffer, readbackBgra); + return mColorBufferGl->readbackAsync(buffer, readbackBgra); } bool ColorBuffer::Impl::glOpImportEglNativePixmap(void* pixmap, bool preserveContent) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } return mColorBufferGl->importEglNativePixmap(pixmap, preserveContent); } -void ColorBuffer::Impl::glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, +bool ColorBuffer::Impl::glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, GfxstreamFormat texturesFormat, GLuint* textures) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } mColorBufferGl->swapYUVTextures(texturesFormat, textures); @@ -601,11 +607,13 @@ void ColorBuffer::Impl::glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, mColorBufferGl->subUpdate(0, 0, mWidth, mHeight, texturesFormat, /*pixels=*/nullptr); flushFromGl(); + return true; } bool ColorBuffer::Impl::glOpReadContents(size_t* outNumBytes, void* outContents) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } return mColorBufferGl->readContents(outNumBytes, outContents); @@ -613,29 +621,34 @@ bool ColorBuffer::Impl::glOpReadContents(size_t* outNumBytes, void* outContents) bool ColorBuffer::Impl::glOpIsFastBlitSupported() const { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } return mColorBufferGl->isFastBlitSupported(); } -void ColorBuffer::Impl::glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, +bool ColorBuffer::Impl::glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, const std::optional>& colorTransform) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } mColorBufferGl->postLayer(l, frameWidth, frameHeight, colorTransform); + return true; } -void ColorBuffer::Impl::glOpPostViewportScaledWithOverlay( +bool ColorBuffer::Impl::glOpPostViewportScaledWithOverlay( float rotation, float dx, float dy, float scaleX, float scaleY, const std::optional>& colorTransform) { if (!mColorBufferGl) { - GFXSTREAM_FATAL("ColorBufferGl not available"); + GFXSTREAM_ERROR("%s: ColorBufferGl not available", __func__); + return false; } mColorBufferGl->postViewportScaledWithOverlay(rotation, dx, dy, scaleX, scaleY, colorTransform); + return true; } #endif @@ -737,7 +750,7 @@ bool ColorBuffer::invalidateForVk() { return mImpl->invalidateForVk(); } std::optional ColorBuffer::exportBlob() { return mImpl->exportBlob(); } #if GFXSTREAM_ENABLE_HOST_GLES -GLuint ColorBuffer::glOpGetTexture() { return mImpl->glOpGetTexture(); } +bool ColorBuffer::canUseGlOps() { return mImpl->canUseGlOps(); } bool ColorBuffer::glOpBlitFromCurrentReadBuffer() { return mImpl->glOpBlitFromCurrentReadBuffer(); } @@ -747,11 +760,11 @@ bool ColorBuffer::glOpBindToTexture2() { return mImpl->glOpBindToTexture2(); } bool ColorBuffer::glOpBindToRenderbuffer() { return mImpl->glOpBindToRenderbuffer(); } -void ColorBuffer::glOpReadback(unsigned char* img, bool readbackBgra) { +bool ColorBuffer::glOpReadback(unsigned char* img, bool readbackBgra) { return mImpl->glOpReadback(img, readbackBgra); } -void ColorBuffer::glOpReadbackAsync(GLuint buffer, bool readbackBgra) { +bool ColorBuffer::glOpReadbackAsync(GLuint buffer, bool readbackBgra) { return mImpl->glOpReadbackAsync(buffer, readbackBgra); } @@ -759,7 +772,7 @@ bool ColorBuffer::glOpImportEglNativePixmap(void* pixmap, bool preserveContent) return mImpl->glOpImportEglNativePixmap(pixmap, preserveContent); } -void ColorBuffer::glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, +bool ColorBuffer::glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, GfxstreamFormat texturesFormat, GLuint* textures) { return mImpl->glOpSwapYuvTexturesAndUpdate(format, type, texturesFormat, textures); } @@ -770,12 +783,12 @@ bool ColorBuffer::glOpReadContents(size_t* outNumBytes, void* outContents) { bool ColorBuffer::glOpIsFastBlitSupported() const { return mImpl->glOpIsFastBlitSupported(); } -void ColorBuffer::glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, +bool ColorBuffer::glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, const std::optional>& colorTransform) { return mImpl->glOpPostLayer(l, frameWidth, frameHeight, colorTransform); } -void ColorBuffer::glOpPostViewportScaledWithOverlay( +bool ColorBuffer::glOpPostViewportScaledWithOverlay( float rotation, float dx, float dy, float scaleX, float scaleY, const std::optional>& colorTransform) { return mImpl->glOpPostViewportScaledWithOverlay(rotation, dx, dy, scaleX, scaleY, diff --git a/host/color_buffer.h b/host/color_buffer.h index 5e32e076e..4162c1045 100644 --- a/host/color_buffer.h +++ b/host/color_buffer.h @@ -105,21 +105,21 @@ class ColorBuffer : public LazySnapshotObj { std::optional exportBlob(); #if GFXSTREAM_ENABLE_HOST_GLES - GLuint glOpGetTexture(); + bool canUseGlOps(); bool glOpBlitFromCurrentReadBuffer(); bool glOpBindToTexture(); bool glOpBindToTexture2(); bool glOpBindToRenderbuffer(); - void glOpReadback(unsigned char* img, bool readbackBgra); - void glOpReadbackAsync(GLuint buffer, bool readbackBgra); + bool glOpReadback(unsigned char* img, bool readbackBgra); + bool glOpReadbackAsync(GLuint buffer, bool readbackBgra); bool glOpImportEglNativePixmap(void* pixmap, bool preserveContent); - void glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, GfxstreamFormat texturesFormat, + bool glOpSwapYuvTexturesAndUpdate(GLenum format, GLenum type, GfxstreamFormat texturesFormat, GLuint* textures); bool glOpReadContents(size_t* outNumBytes, void* outContents); bool glOpIsFastBlitSupported() const; - void glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, + bool glOpPostLayer(const ComposeLayer& l, int frameWidth, int frameHeight, const std::optional>& colorTransform); - void glOpPostViewportScaledWithOverlay( + bool glOpPostViewportScaledWithOverlay( float rotation, float dx, float dy, float scaleX, float scaleY, const std::optional>& colorTransform); #endif diff --git a/host/frame_buffer.cpp b/host/frame_buffer.cpp index 672ea4597..bb849ce9c 100644 --- a/host/frame_buffer.cpp +++ b/host/frame_buffer.cpp @@ -4742,6 +4742,16 @@ bool FrameBuffer::Impl::bindColorBufferToTexture2(HandleType p_colorbuffer) { return false; } + if (!colorBuffer->canUseGlOps()) { + // cannot call glOpBindToTexture2 without a valid gl colorbuffer + static bool errorReported = false; + if (!errorReported) { + GFXSTREAM_ERROR("%s: Cannot use GL colorbuffer operations", __func__); + errorReported = true; + } + return false; + } + return colorBuffer->glOpBindToTexture2(); } diff --git a/host/gl/color_buffer_gl.cpp b/host/gl/color_buffer_gl.cpp index 54d9a2e3c..c69a59002 100644 --- a/host/gl/color_buffer_gl.cpp +++ b/host/gl/color_buffer_gl.cpp @@ -1096,10 +1096,10 @@ bool ColorBufferGl::postViewportScaledWithOverlay( scaleY, colorTransform); } -void ColorBufferGl::readback(unsigned char* img, bool readbackBgra) { +bool ColorBufferGl::readback(unsigned char* img, bool readbackBgra) { RecursiveScopedContextBind context(m_helper); if (!context.isOk()) { - return; + return false; } waitSync(); @@ -1113,12 +1113,13 @@ void ColorBufferGl::readback(unsigned char* img, bool readbackBgra) { s_gles2.glReadPixels(0, 0, m_width, m_height, format, GL_UNSIGNED_BYTE, img); unbindFbo(); } + return true; } -void ColorBufferGl::readbackAsync(GLuint buffer, bool readbackBgra) { +bool ColorBufferGl::readbackAsync(GLuint buffer, bool readbackBgra) { RecursiveScopedContextBind context(m_helper); if (!context.isOk()) { - return; + return false; } waitSync(); @@ -1132,6 +1133,7 @@ void ColorBufferGl::readbackAsync(GLuint buffer, bool readbackBgra) { s_gles2.glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); unbindFbo(); } + return true; } HandleType ColorBufferGl::getHndl() const { return mHndl; } @@ -1205,8 +1207,6 @@ void ColorBufferGl::restore() { } } -GLuint ColorBufferGl::getTexture() { return m_tex; } - void ColorBufferGl::postLayer(const ComposeLayer& l, int frameWidth, int frameHeight, const std::optional>& colorTransform) { waitSync(); diff --git a/host/gl/color_buffer_gl.h b/host/gl/color_buffer_gl.h index 8b09ddc37..2e5ff6322 100644 --- a/host/gl/color_buffer_gl.h +++ b/host/gl/color_buffer_gl.h @@ -189,9 +189,9 @@ class ColorBufferGl { // Read the content of the whole ColorBufferGl as 32-bit RGBA pixels. // |img| must be a buffer large enough (i.e. width * height * 4). - void readback(unsigned char* img, bool readbackBgra = false); + bool readback(unsigned char* img, bool readbackBgra = false); // readback() but async (to the specified |buffer|) - void readbackAsync(GLuint buffer, bool readbackBgra = false); + bool readbackAsync(GLuint buffer, bool readbackBgra = false); void onSave(gfxstream::Stream* stream); static std::unique_ptr onLoad(gfxstream::Stream* stream, EGLDisplay p_display, @@ -205,7 +205,6 @@ class ColorBufferGl { bool isFastBlitSupported() const { return m_fastBlitSupported; } void postLayer(const ComposeLayer& l, int frameWidth, int frameHeight, const std::optional>& colorTransform); - GLuint getTexture(); std::unique_ptr getBorrowedImageInfo(); From 8a3ea125fb8900aa4d8ebc1a3560f0b49a806c44 Mon Sep 17 00:00:00 2001 From: Venkatesh Srinivasan Date: Tue, 12 Aug 2025 19:30:18 +0000 Subject: [PATCH 2/3] Fix -Wrange-loop-construct warning Fixing these warnings either by adding a reference to the loop index's type or removing the reference from it. hardware/google/gfxstream/tests/end2end/TestDataUtils.cpp:65:29: error: loop variable 'possiblePath' of type 'const std::string &' (aka 'const basic_string &') binds to a temporary constructed from type 'reference' (aka 'const std::filesystem::path &') [-Werror,-Wrange-loop-construct] 65 | for (const std::string& possiblePath : possiblePaths) { | ^ hardware/google/gfxstream/tests/end2end/TestDataUtils.cpp:65:10: note: use non-reference type 'std::string' (aka 'basic_string') to make construction explicit or type 'const std::filesystem::path &' to prevent copying 65 | for (const std::string& possiblePath : possiblePaths) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Bug: 153747076 Test: m & presubmits Flag: EXEMPT refactor Change-Id: I457af4828122e9942f5f8508f7cbaf24bc596e22 --- tests/end2end/test_data_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/test_data_utils.cpp b/tests/end2end/test_data_utils.cpp index 96f86751c..de53792d7 100644 --- a/tests/end2end/test_data_utils.cpp +++ b/tests/end2end/test_data_utils.cpp @@ -62,7 +62,7 @@ std::filesystem::path GetTestDataPath(const std::string& basename) { currentPath / basename, currentPath / "testdata" / basename, }; - for (const std::string& possiblePath : possiblePaths) { + for (const std::string possiblePath : possiblePaths) { if (std::filesystem::exists(possiblePath)) { return possiblePath; } From 6bd51545d29e9d501439c91741d5e0cb2ac73af0 Mon Sep 17 00:00:00 2001 From: Jason Macnak Date: Mon, 18 May 2026 15:54:33 -0700 Subject: [PATCH 3/3] Remove unused function Deleted in ag/39370045 as part of merge conflict resolution? --- host/virtio_gpu_resource.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/host/virtio_gpu_resource.cpp b/host/virtio_gpu_resource.cpp index dc21e5c5a..18155c376 100644 --- a/host/virtio_gpu_resource.cpp +++ b/host/virtio_gpu_resource.cpp @@ -81,15 +81,6 @@ static std::unordered_map virglFormatInfoMap = { {VIRGL_FORMAT_R8_UNORM, {DRM_FORMAT_R8, 1}}, }; -static std::optional DrmFourccToVirglFormat(uint32_t drm_fourcc) { - for (auto it : virglFormatInfoMap) { - if (it.second.drm_fourcc == drm_fourcc) { - return it.first; - } - } - return -1; -} - static std::optional VirglFormatInfo(uint32_t virglFormat) { auto it = virglFormatInfoMap.find(virglFormat); if (virglFormatInfoMap.end() != it) {