diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp index 16a7d65a1..6915706e5 100644 --- a/src/libANGLE/Context.cpp +++ b/src/libANGLE/Context.cpp @@ -174,6 +174,8 @@ Context::~Context() void Context::makeCurrent(egl::Surface *surface) { + ASSERT(surface != nullptr); + if (!mHasBeenCurrent) { initRendererString(); @@ -187,12 +189,16 @@ void Context::makeCurrent(egl::Surface *surface) // TODO(jmadill): do not allocate new pointers here Framebuffer *framebufferZero = new DefaultFramebuffer(mCaps, mRenderer, surface); - setFramebufferZero(framebufferZero); - mRenderBuffer = surface->getRenderBuffer(); } +void Context::releaseSurface() +{ + setFramebufferZero(nullptr); + mRenderBuffer = EGL_NONE; +} + // NOTE: this function should not assume that this context is current! void Context::markContextLost() { @@ -666,17 +672,19 @@ void Context::setFramebufferZero(Framebuffer *buffer) // First, check to see if the old default framebuffer // was set for draw or read framebuffer, and change // the bindings to point to the new one before deleting it. - if (mState.getDrawFramebuffer()->id() == 0) + if (mState.getDrawFramebuffer() == nullptr || + mState.getDrawFramebuffer()->id() == 0) { mState.setDrawFramebufferBinding(buffer); } - if (mState.getReadFramebuffer()->id() == 0) + if (mState.getReadFramebuffer() == nullptr || + mState.getReadFramebuffer()->id() == 0) { mState.setReadFramebufferBinding(buffer); } - delete mFramebufferMap[0]; + SafeDelete(mFramebufferMap[0]); mFramebufferMap[0] = buffer; } @@ -884,7 +892,7 @@ bool Context::getIndexedIntegerv(GLenum target, GLuint index, GLint *data) { // Queries about context capabilities and maximums are answered by Context. // Queries about current GL state values are answered by State. - // Indexed integer queries all refer to current state, so this function is a + // Indexed integer queries all refer to current state, so this function is a // mere passthrough. return mState.getIndexedIntegerv(target, index, data); } @@ -893,7 +901,7 @@ bool Context::getIndexedInteger64v(GLenum target, GLuint index, GLint64 *data) { // Queries about context capabilities and maximums are answered by Context. // Queries about current GL state values are answered by State. - // Indexed integer queries all refer to current state, so this function is a + // Indexed integer queries all refer to current state, so this function is a // mere passthrough. return mState.getIndexedInteger64v(target, index, data); } @@ -1321,7 +1329,7 @@ void Context::detachTexture(GLuint texture) void Context::detachBuffer(GLuint buffer) { - // Buffer detachment is handled by Context, because the buffer must also be + // Buffer detachment is handled by Context, because the buffer must also be // attached from any VAOs in existence, and Context holds the VAO map. // [OpenGL ES 2.0.24] section 2.9 page 22: @@ -1365,8 +1373,8 @@ void Context::detachRenderbuffer(GLuint renderbuffer) void Context::detachVertexArray(GLuint vertexArray) { - // Vertex array detachment is handled by Context, because 0 is a valid - // VAO, and a pointer to it must be passed from Context to State at + // Vertex array detachment is handled by Context, because 0 is a valid + // VAO, and a pointer to it must be passed from Context to State at // binding time. // [OpenGL ES 3.0.2] section 2.10 page 43: diff --git a/src/libANGLE/Context.h b/src/libANGLE/Context.h index eeada4335..32bfa08c4 100644 --- a/src/libANGLE/Context.h +++ b/src/libANGLE/Context.h @@ -63,11 +63,12 @@ class Context final : angle::NonCopyable virtual ~Context(); void makeCurrent(egl::Surface *surface); + void releaseSurface(); virtual void markContextLost(); bool isContextLost(); - // These create and destroy methods are merely pass-throughs to + // These create and destroy methods are merely pass-throughs to // ResourceManager, which owns these object types GLuint createBuffer(); GLuint createShader(GLenum type); @@ -94,7 +95,7 @@ class Context final : angle::NonCopyable // NV Fences are owned by the Context. GLuint createFenceNV(); void deleteFenceNV(GLuint fence); - + // Queries are owned by the Context; GLuint createQuery(); void deleteQuery(GLuint query); diff --git a/src/libANGLE/Display.cpp b/src/libANGLE/Display.cpp index a217b7337..4af5a448e 100644 --- a/src/libANGLE/Display.cpp +++ b/src/libANGLE/Display.cpp @@ -496,8 +496,9 @@ Error Display::makeCurrent(egl::Surface *drawSurface, egl::Surface *readSurface, return error; } - if (context && drawSurface) + if (context != nullptr && drawSurface != nullptr) { + ASSERT(readSurface == drawSurface); context->makeCurrent(drawSurface); } diff --git a/src/libANGLE/State.cpp b/src/libANGLE/State.cpp index f75c2303a..1e34c633c 100644 --- a/src/libANGLE/State.cpp +++ b/src/libANGLE/State.cpp @@ -765,7 +765,8 @@ const Framebuffer *State::getDrawFramebuffer() const bool State::removeReadFramebufferBinding(GLuint framebuffer) { - if (mReadFramebuffer->id() == framebuffer) + if (mReadFramebuffer != nullptr && + mReadFramebuffer->id() == framebuffer) { mReadFramebuffer = NULL; return true; @@ -776,7 +777,8 @@ bool State::removeReadFramebufferBinding(GLuint framebuffer) bool State::removeDrawFramebufferBinding(GLuint framebuffer) { - if (mDrawFramebuffer->id() == framebuffer) + if (mReadFramebuffer != nullptr && + mDrawFramebuffer->id() == framebuffer) { mDrawFramebuffer = NULL; return true; diff --git a/src/libGLESv2/entry_points_egl.cpp b/src/libGLESv2/entry_points_egl.cpp index 801240ad2..b6c08946e 100644 --- a/src/libGLESv2/entry_points_egl.cpp +++ b/src/libGLESv2/entry_points_egl.cpp @@ -512,6 +512,20 @@ EGLBoolean EGLAPIENTRY MakeCurrent(EGLDisplay dpy, EGLSurface draw, EGLSurface r return EGL_FALSE; } + if (dpy == EGL_NO_DISPLAY) + { + SetGlobalError(Error(EGL_BAD_DISPLAY, "'dpy' not a valid EGLDisplay handle")); + return EGL_FALSE; + } + + // EGL 1.5 spec: dpy can be uninitialized if all other parameters are null + if (dpy != EGL_NO_DISPLAY && !display->isInitialized() && + (ctx != EGL_NO_CONTEXT || draw != EGL_NO_SURFACE || read != EGL_NO_SURFACE)) + { + SetGlobalError(Error(EGL_NOT_INITIALIZED, "'dpy' not initialized")); + return EGL_FALSE; + } + if (ctx != EGL_NO_CONTEXT) { Error error = ValidateContext(display, context); @@ -564,14 +578,20 @@ EGLBoolean EGLAPIENTRY MakeCurrent(EGLDisplay dpy, EGLSurface draw, EGLSurface r UNIMPLEMENTED(); // FIXME } + gl::Context *previousContext = GetGlobalContext(); + SetGlobalDisplay(display); SetGlobalDrawSurface(drawSurface); SetGlobalReadSurface(readSurface); SetGlobalContext(context); - if (context != nullptr && display != nullptr && drawSurface != nullptr) + display->makeCurrent(drawSurface, readSurface, context); + + // Release the surface from the previously-current context, to allow + // destroyed surfaces to delete themselves. + if (previousContext != nullptr && context != previousContext) { - display->makeCurrent(drawSurface, readSurface, context); + previousContext->releaseSurface(); } SetGlobalError(Error(EGL_SUCCESS)); diff --git a/src/tests/angle_end2end_tests.gypi b/src/tests/angle_end2end_tests.gypi index 64dbd89dc..22fa5e46c 100644 --- a/src/tests/angle_end2end_tests.gypi +++ b/src/tests/angle_end2end_tests.gypi @@ -54,8 +54,9 @@ '<(angle_path)/src/tests/end2end_tests/UnpackRowLength.cpp', '<(angle_path)/src/tests/end2end_tests/VertexAttributeTest.cpp', '<(angle_path)/src/tests/end2end_tests/ViewportTest.cpp', - '<(angle_path)/src/tests/standalone_tests/EGLThreadTest.cpp', '<(angle_path)/src/tests/standalone_tests/EGLQueryContextTest.cpp', + '<(angle_path)/src/tests/standalone_tests/EGLSurfaceTest.cpp', + '<(angle_path)/src/tests/standalone_tests/EGLThreadTest.cpp', ], }, 'dependencies': diff --git a/src/tests/end2end_tests/ANGLETest.cpp b/src/tests/end2end_tests/ANGLETest.cpp index 375724f28..c7cbcbf89 100644 --- a/src/tests/end2end_tests/ANGLETest.cpp +++ b/src/tests/end2end_tests/ANGLETest.cpp @@ -3,15 +3,14 @@ #include "OSWindow.h" ANGLETest::ANGLETest(EGLint glesMajorVersion, const EGLPlatformParameters &platform) - : mEGLWindow(NULL) + : mEGLWindow(nullptr) { mEGLWindow = new EGLWindow(1280, 720, glesMajorVersion, platform); } ANGLETest::~ANGLETest() { - delete mEGLWindow; - mEGLWindow = NULL; + SafeDelete(mEGLWindow); } void ANGLETest::SetUp() @@ -50,7 +49,10 @@ void ANGLETest::TearDown() void ANGLETest::swapBuffers() { - mEGLWindow->swap(); + if (mEGLWindow->isGLInitialized()) + { + mEGLWindow->swap(); + } } void ANGLETest::drawQuad(GLuint program, const std::string& positionAttribName, GLfloat quadDepth, GLfloat quadScale) diff --git a/src/tests/standalone_tests/EGLSurfaceTest.cpp b/src/tests/standalone_tests/EGLSurfaceTest.cpp new file mode 100644 index 000000000..f53e05f76 --- /dev/null +++ b/src/tests/standalone_tests/EGLSurfaceTest.cpp @@ -0,0 +1,226 @@ +// +// Copyright 2015 The ANGLE Project Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// +// EGLSurfaceTest: +// Tests pertaining to egl::Surface. +// + +#include +#include +#include +#include + +#include "common/angleutils.h" +#include "OSWindow.h" + +namespace +{ + +class EGLSurfaceTest : public testing::Test +{ + protected: + EGLSurfaceTest() + : mDisplay(EGL_NO_DISPLAY), + mWindowSurface(EGL_NO_SURFACE), + mPbufferSurface(EGL_NO_SURFACE), + mContext(EGL_NO_CONTEXT), + mSecondContext(EGL_NO_CONTEXT), + mOSWindow(nullptr) + { + } + + void SetUp() override + { + mOSWindow = CreateOSWindow(); + mOSWindow->initialize("EGLSurfaceTest", 64, 64); + } + + // Release any resources created in the test body + void TearDown() override + { + mOSWindow->destroy(); + SafeDelete(mOSWindow); + + if (mDisplay != EGL_NO_DISPLAY) + { + eglMakeCurrent(mDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + + if (mWindowSurface != EGL_NO_SURFACE) + { + eglDestroySurface(mDisplay, mWindowSurface); + mWindowSurface = EGL_NO_SURFACE; + } + + if (mPbufferSurface != EGL_NO_SURFACE) + { + eglDestroySurface(mDisplay, mPbufferSurface); + mPbufferSurface = EGL_NO_SURFACE; + } + + if (mContext != EGL_NO_CONTEXT) + { + eglDestroyContext(mDisplay, mContext); + mContext = EGL_NO_CONTEXT; + } + + if (mSecondContext != EGL_NO_CONTEXT) + { + eglDestroyContext(mDisplay, mSecondContext); + mSecondContext = EGL_NO_CONTEXT; + } + + eglTerminate(mDisplay); + mDisplay = EGL_NO_DISPLAY; + } + + ASSERT_TRUE(mWindowSurface == EGL_NO_SURFACE && mContext == EGL_NO_CONTEXT); + } + + void initializeSurface(EGLenum platformType) + { + PFNEGLGETPLATFORMDISPLAYEXTPROC eglGetPlatformDisplayEXT = reinterpret_cast(eglGetProcAddress("eglGetPlatformDisplayEXT")); + ASSERT_TRUE(eglGetPlatformDisplayEXT != nullptr); + + const EGLint displayAttributes[] = + { + EGL_PLATFORM_ANGLE_TYPE_ANGLE, platformType, + EGL_PLATFORM_ANGLE_MAX_VERSION_MAJOR_ANGLE, EGL_DONT_CARE, + EGL_PLATFORM_ANGLE_MAX_VERSION_MINOR_ANGLE, EGL_DONT_CARE, + EGL_PLATFORM_ANGLE_DEVICE_TYPE_ANGLE, EGL_PLATFORM_ANGLE_DEVICE_TYPE_HARDWARE_ANGLE, + EGL_NONE, + }; + + mDisplay = eglGetPlatformDisplayEXT(EGL_PLATFORM_ANGLE_ANGLE, mOSWindow->getNativeDisplay(), displayAttributes); + ASSERT_TRUE(mDisplay != EGL_NO_DISPLAY); + + EGLint majorVersion, minorVersion; + ASSERT_TRUE(eglInitialize(mDisplay, &majorVersion, &minorVersion) == EGL_TRUE); + + eglBindAPI(EGL_OPENGL_ES_API); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + const EGLint configAttributes[] = + { + EGL_RED_SIZE, EGL_DONT_CARE, + EGL_GREEN_SIZE, EGL_DONT_CARE, + EGL_BLUE_SIZE, EGL_DONT_CARE, + EGL_ALPHA_SIZE, EGL_DONT_CARE, + EGL_DEPTH_SIZE, EGL_DONT_CARE, + EGL_STENCIL_SIZE, EGL_DONT_CARE, + EGL_SAMPLE_BUFFERS, 0, + EGL_NONE + }; + + EGLint configCount; + ASSERT_TRUE(eglChooseConfig(mDisplay, configAttributes, &mConfig, 1, &configCount) || (configCount != 1) == EGL_TRUE); + + std::vector surfaceAttributes; + surfaceAttributes.push_back(EGL_NONE); + surfaceAttributes.push_back(EGL_NONE); + + // Create first window surface + mWindowSurface = eglCreateWindowSurface(mDisplay, mConfig, mOSWindow->getNativeWindow(), &surfaceAttributes[0]); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + mPbufferSurface = eglCreatePbufferSurface(mDisplay, mConfig, &surfaceAttributes[0]); + + EGLint contextAttibutes[] = + { + EGL_CONTEXT_CLIENT_VERSION, 2, + EGL_NONE + }; + + mContext = eglCreateContext(mDisplay, mConfig, nullptr, contextAttibutes); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + mSecondContext = eglCreateContext(mDisplay, mConfig, nullptr, contextAttibutes); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + } + + void runMessageLoopTest(EGLSurface secondSurface, EGLContext secondContext) + { + eglMakeCurrent(mDisplay, mWindowSurface, mWindowSurface, mContext); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + // Make a second context current + eglMakeCurrent(mDisplay, secondSurface, secondSurface, secondContext); + eglDestroySurface(mDisplay, mWindowSurface); + + // Create second window surface + std::vector surfaceAttributes; + surfaceAttributes.push_back(EGL_NONE); + surfaceAttributes.push_back(EGL_NONE); + + mWindowSurface = eglCreateWindowSurface(mDisplay, mConfig, mOSWindow->getNativeWindow(), &surfaceAttributes[0]); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + eglMakeCurrent(mDisplay, mWindowSurface, mWindowSurface, mContext); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + mOSWindow->signalTestEvent(); + mOSWindow->messageLoop(); + ASSERT_TRUE(mOSWindow->didTestEventFire()); + + // Simple operation to test the FBO is set appropriately + glClear(GL_COLOR_BUFFER_BIT); + } + + EGLDisplay mDisplay; + EGLSurface mWindowSurface; + EGLSurface mPbufferSurface; + EGLContext mContext; + EGLContext mSecondContext; + EGLConfig mConfig; + OSWindow *mOSWindow; +}; + +// Test a surface bug where we could have two Window surfaces active +// at one time, blocking message loops. See http://crbug.com/475085 +TEST_F(EGLSurfaceTest, MessageLoopBug) +{ + const char *extensionsString = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS); + if (strstr(extensionsString, "EGL_ANGLE_platform_angle_d3d") == nullptr) + { + std::cout << "D3D Platform not supported in ANGLE"; + return; + } + + initializeSurface(EGL_PLATFORM_ANGLE_TYPE_D3D11_ANGLE); + + runMessageLoopTest(EGL_NO_SURFACE, EGL_NO_CONTEXT); +} + +// Tests the message loop bug, but with setting a second context +// instead of null. +TEST_F(EGLSurfaceTest, MessageLoopBugContext) +{ + const char *extensionsString = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS); + if (strstr(extensionsString, "EGL_ANGLE_platform_angle_d3d") == nullptr) + { + std::cout << "D3D Platform not supported in ANGLE"; + return; + } + + initializeSurface(EGL_PLATFORM_ANGLE_TYPE_D3D11_ANGLE); + + runMessageLoopTest(mPbufferSurface, mSecondContext); +} + +// Test a bug where calling makeCurrent twice would release the surface +TEST_F(EGLSurfaceTest, MakeCurrentTwice) +{ + initializeSurface(EGL_PLATFORM_ANGLE_TYPE_DEFAULT_ANGLE); + + eglMakeCurrent(mDisplay, mWindowSurface, mWindowSurface, mContext); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + eglMakeCurrent(mDisplay, mWindowSurface, mWindowSurface, mContext); + ASSERT_TRUE(eglGetError() == EGL_SUCCESS); + + // Simple operation to test the FBO is set appropriately + glClear(GL_COLOR_BUFFER_BIT); +} + +} diff --git a/util/EGLWindow.cpp b/util/EGLWindow.cpp index 0e56a8735..499c33bc8 100644 --- a/util/EGLWindow.cpp +++ b/util/EGLWindow.cpp @@ -229,3 +229,10 @@ void EGLWindow::destroyGL() mDisplay = EGL_NO_DISPLAY; } } + +bool EGLWindow::isGLInitialized() const +{ + return mSurface != EGL_NO_SURFACE && + mContext != EGL_NO_CONTEXT && + mDisplay != EGL_NO_DISPLAY; +} \ No newline at end of file diff --git a/util/EGLWindow.h b/util/EGLWindow.h index fa6c43a9c..b348fe4bf 100644 --- a/util/EGLWindow.h +++ b/util/EGLWindow.h @@ -83,6 +83,7 @@ class EGLWindow : angle::NonCopyable bool initializeGL(OSWindow *osWindow); void destroyGL(); + bool isGLInitialized() const; private: EGLConfig mConfig; diff --git a/util/Event.h b/util/Event.h index 141486414..489ff04bb 100644 --- a/util/Event.h +++ b/util/Event.h @@ -68,6 +68,7 @@ class Event EVENT_MOUSE_MOVED, // The mouse cursor moved EVENT_MOUSE_ENTERED, // The mouse cursor entered the area of the window EVENT_MOUSE_LEFT, // The mouse cursor left the area of the window + EVENT_TEST, // Event for testing purposes }; EventType Type; diff --git a/util/OSWindow.cpp b/util/OSWindow.cpp index a637a55f1..06d920f21 100644 --- a/util/OSWindow.cpp +++ b/util/OSWindow.cpp @@ -51,3 +51,17 @@ void OSWindow::pushEvent(Event event) mEvents.push_back(event); } + +bool OSWindow::didTestEventFire() +{ + Event topEvent; + while (popEvent(&topEvent)) + { + if (topEvent.Type == Event::EVENT_TEST) + { + return true; + } + } + + return false; +} diff --git a/util/OSWindow.h b/util/OSWindow.h index 03b1fc775..221c40387 100644 --- a/util/OSWindow.h +++ b/util/OSWindow.h @@ -37,6 +37,11 @@ class OSWindow virtual bool resize(int width, int height) = 0; virtual void setVisible(bool isVisible) = 0; + virtual void signalTestEvent() = 0; + + // Pops events look for the test event + bool didTestEventFire(); + protected: int mWidth; int mHeight; diff --git a/util/win32/Win32Window.cpp b/util/win32/Win32Window.cpp index 7802f2587..88b8ad78c 100644 --- a/util/win32/Win32Window.cpp +++ b/util/win32/Win32Window.cpp @@ -133,13 +133,13 @@ LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) { case WM_NCCREATE: { - LPCREATESTRUCT pCreateStruct = (LPCREATESTRUCT)lParam; - SetWindowLongPtr(hWnd, GWLP_USERDATA, (LONG_PTR)pCreateStruct->lpCreateParams); + LPCREATESTRUCT pCreateStruct = reinterpret_cast(lParam); + SetWindowLongPtr(hWnd, GWLP_USERDATA, reinterpret_cast(pCreateStruct->lpCreateParams)); return DefWindowProcA(hWnd, message, wParam, lParam); } } - OSWindow *window = (OSWindow*)(LONG_PTR)GetWindowLongPtr(hWnd, GWLP_USERDATA); + Win32Window *window = reinterpret_cast(GetWindowLongPtr(hWnd, GWLP_USERDATA)); if (window) { switch (message) @@ -357,6 +357,14 @@ LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) window->pushEvent(event); break; } + + case WM_USER: + { + Event testEvent; + testEvent.Type = Event::EVENT_TEST; + window->pushEvent(testEvent); + break; + } } } @@ -563,3 +571,8 @@ void Win32Window::pushEvent(Event event) break; } } + +void Win32Window::signalTestEvent() +{ + PostMessage(mNativeWindow, WM_USER, 0, 0); +} diff --git a/util/win32/Win32Window.h b/util/win32/Win32Window.h index 430961997..a2ff03f54 100644 --- a/util/win32/Win32Window.h +++ b/util/win32/Win32Window.h @@ -17,19 +17,21 @@ class Win32Window : public OSWindow Win32Window(); ~Win32Window(); - bool initialize(const std::string &name, size_t width, size_t height); - void destroy(); + bool initialize(const std::string &name, size_t width, size_t height) override; + void destroy() override; - EGLNativeWindowType getNativeWindow() const; - EGLNativeDisplayType getNativeDisplay() const; + EGLNativeWindowType getNativeWindow() const override; + EGLNativeDisplayType getNativeDisplay() const override; - void messageLoop(); + void messageLoop() override; - virtual void pushEvent(Event event); + void pushEvent(Event event) override; - void setMousePosition(int x, int y); - bool resize(int width, int height); - void setVisible(bool isVisible); + void setMousePosition(int x, int y) override; + bool resize(int width, int height) override; + void setVisible(bool isVisible) override; + + void signalTestEvent() override; private: std::string mParentClassName;