From f293b2ff78ce4adac28962d020a0313fdb509cd2 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 30 Jul 2009 12:19:10 -0700 Subject: [PATCH 1/2] NPOT EGLimage without GL_ARB_texture_non_power_of_two would be improperly scalled The current gralloc allocates buffer memory for render targets that will typically have NPOT dimensions. Assuming that the vendor driver supports converting the resulting NPOT android_native_buffer_t to a NPOT EGLImage, SurfaceFlinger calls glEGLImageTargetTexture2DOES(), and uses glGetError() to test whether the GL can support creating an EGL target texture with the specified NPOT EGLImage. If it is supported, the DIRECT_TEXTURE flag remains set, otherwise it is cleared. Tangentially, if the driver advertises the GL_ARB_texture_non_power_of_two extension, the NPOT_EXTENSION flag is set, otherwise it is cleared. If the driver supported creating an EGL target texture from a NPOT source EGLImage, it implicitly creates a NPOT texture. This does not need any glScalef() texture coordinate correction in LayerBase::drawWithOpenGL(). However, the same driver may not advertise the GL_ARB_texture_non_power_of_two extension nor generally support NPOT textures that were not derived from EGLImages. So SurfaceFlinger may flag only DIRECT_TEXTURE, not NPOT_EXTENSION. Therefore, the test in LayerBase::drawWithOpenGL() should only perform the glScalef() if neither NPOT_EXTENSION or DIRECT_TEXTURE are flagged. Otherwise scaling is applied to NPOT EGL target textures when none is required. --- libs/surfaceflinger/LayerBase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp index a841ab3348d10..fbce73ddd3be7 100644 --- a/libs/surfaceflinger/LayerBase.cpp +++ b/libs/surfaceflinger/LayerBase.cpp @@ -461,7 +461,8 @@ void LayerBase::drawWithOpenGL(const Region& clip, const Texture& texture) const glRotatef(-90, 0, 0, 1); } - if (!(mFlags & DisplayHardware::NPOT_EXTENSION)) { + if (!(mFlags & (DisplayHardware::NPOT_EXTENSION | + DisplayHardware::DIRECT_TEXTURE))) { // find the smallest power-of-two that will accommodate our surface GLuint tw = 1 << (31 - clz(width)); GLuint th = 1 << (31 - clz(height)); From 5e631892fb9ac4da83b70ba129ceb6a3f501bad9 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 30 Jul 2009 12:24:41 -0700 Subject: [PATCH 2/2] Fix a debug statement in BufferMapper --- libs/ui/BufferMapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/ui/BufferMapper.cpp b/libs/ui/BufferMapper.cpp index 92a9a86bb0f29..4add8f975a863 100644 --- a/libs/ui/BufferMapper.cpp +++ b/libs/ui/BufferMapper.cpp @@ -65,7 +65,7 @@ status_t BufferMapper::lock(buffer_handle_t handle, { status_t err = mAllocMod->lock(mAllocMod, handle, usage, bounds.left, bounds.top, bounds.width(), bounds.height(), vaddr); - LOGW_IF(err, "unlock(...) failed %d (%s)", err, strerror(-err)); + LOGW_IF(err, "lock(...) failed %d (%s)", err, strerror(-err)); return err; }