Commit 935fe307 authored by Vasiliy Telezhnikov's avatar Vasiliy Telezhnikov Committed by Commit Bot

Always use source texture in EGL SharedImage.

On some Mali gpus glTexSubImage fails for egl siblings with R8 format.
Before we experienced that on NVidia gpu we get memory corruption after
deleting source texture.

This CL modifies NVidia workaround to not only keep source texture
alive but also use it if we're on the same context and enables it on
all Android devices to be closer to MailboxManagerSync behavior be had
before threaded shared images.

Bug: 1117370
Change-Id: Ia93badb27bbb23146ec348598821f66ad9b9f15d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390861Reviewed-by: default avatarPeng Huang <penghuang@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803936}
parent cb55886e
......@@ -25,14 +25,16 @@ class SharedImageRepresentationEglImageGLTexture
SharedImageRepresentationEglImageGLTexture(SharedImageManager* manager,
SharedImageBacking* backing,
MemoryTypeTracker* tracker,
gles2::Texture* texture)
gles2::Texture* texture,
bool should_delete_texture)
: SharedImageRepresentationGLTexture(manager, backing, tracker),
texture_(texture) {}
texture_(texture),
should_delete_texture_(should_delete_texture) {}
~SharedImageRepresentationEglImageGLTexture() override {
EndAccess();
if (texture_)
if (texture_ && should_delete_texture_)
texture_->RemoveLightweightRef(has_context());
}
......@@ -77,6 +79,7 @@ class SharedImageRepresentationEglImageGLTexture
}
gles2::Texture* texture_;
const bool should_delete_texture_;
RepresentationAccessMode mode_ = RepresentationAccessMode::kNone;
DISALLOW_COPY_AND_ASSIGN(SharedImageRepresentationEglImageGLTexture);
};
......@@ -107,9 +110,7 @@ SharedImageBackingEglImage::SharedImageBackingEglImage(
gl_type_(gl_type),
batch_access_manager_(batch_access_manager) {
DCHECK(batch_access_manager_);
#if DCHECK_IS_ON()
created_on_context_ = gl::g_current_gl_context;
#endif
// On some GPUs (NVidia) keeping reference to egl image itself is not enough,
// we must keep reference to at least one sibling.
if (workarounds.dont_delete_source_texture_for_egl_image) {
......@@ -137,11 +138,20 @@ bool SharedImageBackingEglImage::ProduceLegacyMailbox(
std::unique_ptr<SharedImageRepresentationGLTexture>
SharedImageBackingEglImage::ProduceGLTexture(SharedImageManager* manager,
MemoryTypeTracker* tracker) {
// On some GPUs (Mali, mostly Android 9, like J7) glTexSubImage fails on egl
// image sibling. So we use the original texture if we're on the same gl
// context. see https://crbug.com/1117370
if (source_texture_ && created_on_context_ == gl::g_current_gl_context) {
return std::make_unique<SharedImageRepresentationEglImageGLTexture>(
manager, this, tracker, source_texture_,
/*should_delete_texture=*/false);
}
auto* texture = GenEGLImageSibling();
if (!texture)
return nullptr;
return std::make_unique<SharedImageRepresentationEglImageGLTexture>(
manager, this, tracker, texture);
manager, this, tracker, texture, /*should_delete_texture=*/true);
}
std::unique_ptr<SharedImageRepresentationSkia>
......@@ -149,13 +159,9 @@ SharedImageBackingEglImage::ProduceSkia(
SharedImageManager* manager,
MemoryTypeTracker* tracker,
scoped_refptr<SharedContextState> context_state) {
auto* texture = GenEGLImageSibling();
if (!texture)
auto gl_representation = ProduceGLTexture(manager, tracker);
if (!gl_representation)
return nullptr;
auto gl_representation =
std::make_unique<SharedImageRepresentationEglImageGLTexture>(
manager, this, tracker, std::move(texture));
return SharedImageRepresentationSkiaGL::Create(std::move(gl_representation),
std::move(context_state),
manager, this, tracker);
......@@ -324,9 +330,8 @@ void SharedImageBackingEglImage::SetEndReadFence(
void SharedImageBackingEglImage::MarkForDestruction() {
AutoLock auto_lock(this);
#if DCHECK_IS_ON()
DCHECK(!have_context() || created_on_context_ == gl::g_current_gl_context);
#endif
if (source_texture_) {
source_texture_->RemoveLightweightRef(have_context());
source_texture_ = nullptr;
......
......@@ -82,10 +82,7 @@ class SharedImageBackingEglImage : public ClearTrackingSharedImageBacking {
const GLuint gl_format_;
const GLuint gl_type_;
gles2::Texture* source_texture_ = nullptr;
#if DCHECK_IS_ON()
gl::GLApi* created_on_context_;
#endif
// This class encapsulates the EGLImage object for android.
scoped_refptr<gles2::NativeImageBuffer> egl_image_buffer_ GUARDED_BY(lock_);
......
......@@ -3277,12 +3277,11 @@
},
{
"id": 334,
"description": "NVidia drivers seem corrupt memory when all siblings of eglImage are destroyed",
"cr_bugs": [1052114],
"description": "Some drivers seem to require as to use original texture whenever possible",
"cr_bugs": [1052114, 1117370],
"os": {
"type": "android"
},
"gl_vendor": "NVIDIA.*",
"features": [
"dont_delete_source_texture_for_egl_image"
]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment