Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(689)

Unified Diff: gpu/command_buffer/service/gles2_cmd_decoder.cc

Issue 1736093002: Add a workaround for copyTexImage2D as it is sometimes broken on OSX. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Restrict workaround to Nvidia GPUs. Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | gpu/config/gpu_driver_bug_list_json.cc » ('j') | gpu/config/gpu_driver_bug_list_json.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index f95d2843c5c5c4d560ec6defc84585ddaa0f28c4..58f372f1b874fa5118846e27e12628e973dd440a 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1906,6 +1906,15 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
const SamplerState& GetSamplerStateForTextureUnit(GLenum target, GLuint unit);
+ // copyTexImage2D doesn't work on OSX under very specific conditions.
+ // Returns whether those conditions have been met. If this method returns
+ // true, |original_texture_service_id| and |original_texture_target| are also
+ // populated, since they are needed to implement the workaround.
+ bool NeedsCopyTextureImageWorkaround(GLenum internal_format,
+ int32_t channels_exist,
+ GLuint* original_texture_service_id,
+ GLenum* original_texture_target);
+
// Generate a member function prototype for each command in an automated and
// typesafe way.
#define GLES2_CMD_OP(name) \
@@ -11534,8 +11543,56 @@ void GLES2DecoderImpl::DoCopyTexImage2D(
copyWidth, copyHeight);
}
} else {
- glCopyTexImage2D(target, level, internal_format,
- copyX, copyY, copyWidth, copyHeight, border);
+ GLuint original_texture_service_id = 0;
+ GLenum original_texture_target = 0;
+ bool use_workaround = NeedsCopyTextureImageWorkaround(
+ internal_format, channels_exist, &original_texture_service_id,
+ &original_texture_target);
+ if (use_workaround) {
+ // Copy from the read framebuffer into |temp_texture|.
+ GLuint temp_texture;
+ GLenum temp_internal_format = 0;
+ if (channels_exist == GLES2Util::kRGBA) {
+ temp_internal_format = GL_RGBA;
+ } else if (channels_exist == GLES2Util::kRGB) {
+ temp_internal_format = GL_RGB;
+ } else {
+ NOTREACHED();
+ }
+ glGenTextures(1, &temp_texture);
+ glBindTexture(original_texture_target, temp_texture);
Ken Russell (switch to Gerrit) 2016/02/29 22:06:45 This breaks the texture binding to |original_textu
erikchen 2016/03/14 23:57:30 I believe it does. Do you have reason to believe t
Ken Russell (switch to Gerrit) 2016/03/15 00:47:38 Yes, because the code doesn't query the name of th
+ glTexParameteri(original_texture_target, GL_TEXTURE_MIN_FILTER,
Zhenyao Mo 2016/03/02 00:22:40 Why do you need to worry about tex parameters here
erikchen 2016/03/14 23:57:30 Removed.
+ GL_LINEAR);
+ glTexParameteri(original_texture_target, GL_TEXTURE_MAG_FILTER,
+ GL_LINEAR);
+ glTexParameteri(original_texture_target, GL_TEXTURE_WRAP_S,
+ GL_CLAMP_TO_EDGE);
+ glTexParameteri(original_texture_target, GL_TEXTURE_WRAP_T,
+ GL_CLAMP_TO_EDGE);
+ glCopyTexImage2D(original_texture_target, 0, temp_internal_format, copyX,
+ copyY, copyWidth, copyHeight, border);
+
+ // Attach the temp texture.
+ glFramebufferTexture2DEXT(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
Zhenyao Mo 2016/03/02 00:22:40 If this is on top of ES2, then READ_FRAMEBUFFER is
erikchen 2016/03/14 23:57:30 Done.
+ original_texture_target, temp_texture, 0);
+
+ // Copy to the final texture.
+ GLuint service_id = texture->service_id();
+ glBindTexture(target, service_id);
+ DCHECK_EQ(static_cast<GLuint>(GL_TEXTURE_2D), target);
+ glCopyTexImage2D(target, level, internal_format, 0, 0, copyWidth,
+ copyHeight, 0);
+
+ // Rebind initial texture.
+ glFramebufferTexture2DEXT(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
Zhenyao Mo 2016/03/02 00:22:40 and here.
erikchen 2016/03/14 23:57:30 Done.
+ original_texture_target,
+ original_texture_service_id, 0);
+
+ glDeleteTextures(1, &temp_texture);
+ } else {
+ glCopyTexImage2D(target, level, internal_format, copyX, copyY, copyWidth,
+ copyHeight, border);
+ }
}
GLenum error = LOCAL_PEEK_GL_ERROR("glCopyTexImage2D");
if (error == GL_NO_ERROR) {
@@ -15794,6 +15851,50 @@ const SamplerState& GLES2DecoderImpl::GetSamplerStateForTextureUnit(
return default_sampler_state_;
}
+bool GLES2DecoderImpl::NeedsCopyTextureImageWorkaround(
+ GLenum internal_format,
+ int32_t channels_exist,
+ GLuint* original_texture_service_id,
+ GLenum* original_texture_target) {
+ // On some OSX devices, copyTexImage2D will fail if all of these conditions
+ // are met:
+ // 1. The internal format of the new texture is not GL_RGB or GL_RGBA.
+ // 2. The image of the read FBO is backed by an IOSurface.
+ // See https://crbug.com/581777#c4 for more details.
+ if (!workarounds().use_intermediary_for_copy_texture_image)
+ return false;
+
+ if (internal_format == GL_RGB || internal_format == GL_RGBA)
+ return false;
+
+ Framebuffer* framebuffer =
+ GetFramebufferInfoForTarget(GL_READ_FRAMEBUFFER_EXT);
+ if (!framebuffer)
+ return false;
+
+ const Framebuffer::Attachment* attachment =
+ framebuffer->GetReadBufferAttachment();
+ if (!attachment)
+ return false;
+
+ if (!attachment->IsTextureAttachment())
+ return false;
+
+ TextureRef* texture =
+ texture_manager()->GetTexture(attachment->object_name());
+ if (!texture->texture()->HasImages())
+ return false;
+
+ // The workaround only works if the source texture consists of the channels
+ // kRGB or kRGBA.
+ if (channels_exist != GLES2Util::kRGBA && channels_exist != GLES2Util::kRGB)
+ return false;
+
+ *original_texture_target = texture->texture()->target();
+ *original_texture_service_id = texture->service_id();
+ return true;
+}
+
error::Error GLES2DecoderImpl::HandleBindFragmentInputLocationCHROMIUMBucket(
uint32_t immediate_data_size,
const void* cmd_data) {
« no previous file with comments | « no previous file | gpu/config/gpu_driver_bug_list_json.cc » ('j') | gpu/config/gpu_driver_bug_list_json.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698