Skip to content

Commit

Permalink
Revert "Add a glFlush to GLContextCGL's Make/ReleaseVirtuallyCurrent"
Browse files Browse the repository at this point in the history
This reverts https://crrev.com/580656 because it re-introduced driver
crashes on macOS.

This don't re-introduce the logging that had been added (yet) because
this patch is to target merging for M69.

TBR=kbr

Bug: 863817
Change-Id: I5912f25416d11606fa335d3955a70478a5edf6fb
Reviewed-on: https://chromium-review.googlesource.com/1163180
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580787}
  • Loading branch information
ccameron-chromium authored and Commit Bot committed Aug 5, 2018
1 parent 5d832c4 commit 09ae405
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 33 deletions.
4 changes: 4 additions & 0 deletions gpu/command_buffer/service/gl_context_virtual.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ uint64_t GLContextVirtual::BackpressureFenceCreate() {
void GLContextVirtual::BackpressureFenceWait(uint64_t fence) {
shared_context_->BackpressureFenceWait(fence);
}

void GLContextVirtual::FlushForDriverCrashWorkaround() {
shared_context_->FlushForDriverCrashWorkaround();
}
#endif

GLContextVirtual::~GLContextVirtual() {
Expand Down
1 change: 1 addition & 0 deletions gpu/command_buffer/service/gl_context_virtual.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class GPU_GLES2_EXPORT GLContextVirtual : public gl::GLContext {
#if defined(OS_MACOSX)
uint64_t BackpressureFenceCreate() override;
void BackpressureFenceWait(uint64_t fence) override;
void FlushForDriverCrashWorkaround() override;
#endif

protected:
Expand Down
7 changes: 7 additions & 0 deletions gpu/command_buffer/service/gles2_cmd_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5690,6 +5690,13 @@ error::Error GLES2DecoderImpl::DoCommandsImpl(unsigned int num_commands,
}
}

#if defined(OS_MACOSX)
// Aggressively call glFlush on macOS. This is the only fix that has been
// found so far to avoid crashes on Intel drivers.
// https://crbug.com/863817
context_->FlushForDriverCrashWorkaround();
#endif

*entries_processed = process_pos;

if (error::IsError(result)) {
Expand Down
2 changes: 2 additions & 0 deletions ui/gl/gl_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ uint64_t GLContext::BackpressureFenceCreate() {
}

void GLContext::BackpressureFenceWait(uint64_t fence) {}

void GLContext::FlushForDriverCrashWorkaround() {}
#endif

bool GLContext::HasExtension(const char* name) {
Expand Down
8 changes: 5 additions & 3 deletions ui/gl/gl_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,11 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
virtual bool WasAllocatedUsingRobustnessExtension();

// Make this context current when used for context virtualization.
virtual bool MakeVirtuallyCurrent(GLContext* virtual_context,
GLSurface* surface);
bool MakeVirtuallyCurrent(GLContext* virtual_context, GLSurface* surface);

// Notify this context that |virtual_context|, that was using us, is
// being released or destroyed.
virtual void OnReleaseVirtuallyCurrent(GLContext* virtual_context);
void OnReleaseVirtuallyCurrent(GLContext* virtual_context);

// Returns the GL version string. The context must be current.
virtual std::string GetGLVersion();
Expand Down Expand Up @@ -203,6 +202,9 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
virtual uint64_t BackpressureFenceCreate();
// Perform a client-side wait on a previously-created fence.
virtual void BackpressureFenceWait(uint64_t fence);
// Flush the underlying context to avoid crashes due to driver bugs on macOS.
// https://crbug.com/863817
virtual void FlushForDriverCrashWorkaround();
#endif

protected:
Expand Down
37 changes: 10 additions & 27 deletions ui/gl/gl_context_cgl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,33 +214,6 @@ bool GLContextCGL::ForceGpuSwitchIfNeeded() {
return true;
}

void GLContextCGL::OnReleaseVirtuallyCurrent(GLContext* virtual_context) {
TRACE_EVENT0("gpu", "GLContextCGL::OnReleaseVirtuallyCurrent");
// Flush before switching contexts, to avoid driver crashes.
// https://crbug.com/863817
if (IsCurrent(nullptr))
glFlush();
GLContext::OnReleaseVirtuallyCurrent(virtual_context);
}

bool GLContextCGL::MakeVirtuallyCurrent(GLContext* virtual_context,
GLSurface* surface) {
TRACE_EVENT0("gpu", "GLContextCGL::MakeVirtuallyCurrent");
// Flush before restoring the new context's state, to avoid driver crashes.
// https://crbug.com/863817
if (IsCurrent(nullptr))
glFlush();

// Restore the state of the new context.
if (!GLContext::MakeVirtuallyCurrent(virtual_context, surface))
return false;

// Flush after having restored this context's state, to avoid driver crashes.
// https://crbug.com/863817
glFlush();
return true;
}

YUVToRGBConverter* GLContextCGL::GetYUVToRGBConverter(
const gfx::ColorSpace& color_space) {
std::unique_ptr<YUVToRGBConverter>& yuv_to_rgb_converter =
Expand All @@ -256,6 +229,10 @@ constexpr uint64_t kInvalidFenceId = 0;
uint64_t GLContextCGL::BackpressureFenceCreate() {
TRACE_EVENT0("gpu", "GLContextCGL::BackpressureFenceCreate");

// This flush will trigger a crash if FlushForDriverCrashWorkaround is not
// called sufficiently frequently.
glFlush();

if (gl::GLFence::IsSupported()) {
next_backpressure_fence_ += 1;
backpressure_fences_[next_backpressure_fence_] = GLFence::Create();
Expand Down Expand Up @@ -310,6 +287,12 @@ void GLContextCGL::BackpressureFenceWait(uint64_t fence_id) {
backpressure_fences_.erase(backpressure_fences_.begin());
}

void GLContextCGL::FlushForDriverCrashWorkaround() {
if (!context_ || CGLGetCurrentContext() != context_)
return;
glFlush();
}

bool GLContextCGL::MakeCurrent(GLSurface* surface) {
DCHECK(context_);

Expand Down
4 changes: 1 addition & 3 deletions ui/gl/gl_context_cgl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,11 @@ class GL_EXPORT GLContextCGL : public GLContextReal {
void* GetHandle() override;
void SetSafeToForceGpuSwitch() override;
bool ForceGpuSwitchIfNeeded() override;
void OnReleaseVirtuallyCurrent(GLContext* virtual_context) override;
bool MakeVirtuallyCurrent(GLContext* virtual_context,
GLSurface* surface) override;
YUVToRGBConverter* GetYUVToRGBConverter(
const gfx::ColorSpace& color_space) override;
uint64_t BackpressureFenceCreate() override;
void BackpressureFenceWait(uint64_t fence) override;
void FlushForDriverCrashWorkaround() override;

protected:
~GLContextCGL() override;
Expand Down

0 comments on commit 09ae405

Please sign in to comment.