From 570410071d67fd7b8ec1e56bb3e9ca80c78d1de1 Mon Sep 17 00:00:00 2001 From: Jamie Madill Date: Thu, 14 Mar 2019 13:33:24 +0000 Subject: [PATCH] Revert "Fix several WGL test failures." This reverts commit 13a8c4d84e16137c47cb2f958b24c08797527cfc. Reason for revert: Seems to have led to a flakier situation: https://ci.chromium.org/p/chromium/builders/ci/Win10%20FYI%20Debug%20%28NVIDIA%29/3006 https://ci.chromium.org/p/chromium/builders/try/win-angle-rel/1231 Original change's description: > Fix several WGL test failures. > > SimpleOperationTest.ClearAndSwap/ES2_WGL failed when run in isolation, > since getGLWindow()->hasError() would report a previous error, > instead of result of swapBuffers(). > When running after an OPENGL test, swapBuffers() would clear > the previous error, but that doesn't happen in isolation. > > The previous error is from loading WGL functions, some of which are > expected not to be present. Clear the error in GetProcAddressWithFallback, > but verify that there is no error entering it. > > This uncovers more errors in angle_perftests: > DrawCallPerfBenchmark.Run/wgl > DrawCallPerfBenchmark.Run/wgl_tex_change > DrawCallPerfBenchmark.Run/wgl_vbo_change > DrawElementsPerfBenchmark.Run/wgl_ushort > They come from redundant calls when destroying a window. Fix this as well. > > Several more errors where uncovered by debug prints, fix those, too. > > Bug: angleproject:3153 > Change-Id: I559c098be9dcdfd3add83f045f745d190250b986 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1515602 > Commit-Queue: Yuly Novikov > Reviewed-by: Shahbaz Youssefi TBR=ynovikov@chromium.org,geofflang@chromium.org,syoussefi@chromium.org,jmadill@chromium.org Change-Id: I095fadc0dd3a2c998c1dc86f3760184ae6fd7309 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: angleproject:3153 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1523527 Reviewed-by: Jamie Madill Commit-Queue: Jamie Madill --- src/gpu_info_util/SystemInfo_win.cpp | 11 ---- util/windows/WGLWindow.cpp | 59 +++++++++---------- util/windows/win32/Win32Window.cpp | 84 ++-------------------------- util/windows/win32/Win32Window.h | 4 -- 4 files changed, 31 insertions(+), 127 deletions(-) diff --git a/src/gpu_info_util/SystemInfo_win.cpp b/src/gpu_info_util/SystemInfo_win.cpp index 76cdf8fae..48aae181b 100644 --- a/src/gpu_info_util/SystemInfo_win.cpp +++ b/src/gpu_info_util/SystemInfo_win.cpp @@ -241,20 +241,9 @@ bool GetSystemInfo(SystemInfo *info) } ASSERT(foundPrimary); - ASSERT(GetLastError() == ERROR_SUCCESS); // nvd3d9wrap.dll is loaded into all processes when Optimus is enabled. HMODULE nvd3d9wrap = GetModuleHandleW(L"nvd3d9wrap.dll"); info->isOptimus = nvd3d9wrap != nullptr; - // ERROR_MOD_NOT_FOUND is expected from GetModuleHandleW, reset last error if it happens. - if (GetLastError() != ERROR_SUCCESS && GetLastError() != ERROR_MOD_NOT_FOUND) - { - WARN() << "Unexpected error calling GetModuleHandleW: 0x" << std::hex << GetLastError() - << std::endl; - } - else - { - SetLastError(ERROR_SUCCESS); - } return true; } diff --git a/util/windows/WGLWindow.cpp b/util/windows/WGLWindow.cpp index 1a92879bb..45f94c3a7 100644 --- a/util/windows/WGLWindow.cpp +++ b/util/windows/WGLWindow.cpp @@ -9,11 +9,9 @@ #include "util/windows/WGLWindow.h" -#include "common/debug.h" #include "common/string_utils.h" #include "util/OSWindow.h" #include "util/system_utils.h" -#include "util/windows/win32/Win32Window.h" #include @@ -41,44 +39,24 @@ HMODULE gCurrentModule = nullptr; angle::GenericProc WINAPI GetProcAddressWithFallback(const char *name) { - ASSERT(GetLastError() == ERROR_SUCCESS); angle::GenericProc proc = reinterpret_cast(gCurrentWGLGetProcAddress(name)); - // ERROR_INVALID_HANDLE and ERROR_PROC_NOT_FOUND are expected from wglGetProcAddress, - // reset last error if they happen. - if (GetLastError() != ERROR_SUCCESS && GetLastError() != ERROR_INVALID_HANDLE && - GetLastError() != ERROR_PROC_NOT_FOUND) - { - std::cerr << "Unexpected error calling wglGetProcAddress: 0x" << std::hex << GetLastError() - << std::endl; - } - else - { - SetLastError(ERROR_SUCCESS); - } - if (proc) { return proc; } - proc = reinterpret_cast(GetProcAddress(gCurrentModule, name)); - // ERROR_PROC_NOT_FOUND is expected from GetProcAddress, reset last error if it happens. - if (GetLastError() != ERROR_SUCCESS && GetLastError() != ERROR_PROC_NOT_FOUND) - { - std::cerr << "Unexpected error calling GetProcAddress: 0x" << std::hex << GetLastError() - << std::endl; - } - else - { - SetLastError(ERROR_SUCCESS); - } - return proc; + return reinterpret_cast(GetProcAddress(gCurrentModule, name)); } bool HasExtension(const std::vector &extensions, const char *ext) { return std::find(extensions.begin(), extensions.end(), ext) != extensions.end(); } + +void DumpLastWindowsError() +{ + std::cerr << "Last Windows error code: 0x" << std::hex << GetLastError() << std::endl; +} } // namespace WGLWindow::WGLWindow(int glesMajorVersion, int glesMinorVersion) @@ -104,15 +82,30 @@ bool WGLWindow::initializeGL(OSWindow *osWindow, angle::Library *glWindowingLibr gCurrentModule = reinterpret_cast(glWindowingLibrary->getNative()); angle::LoadWGL(GetProcAddressWithFallback); - Win32Window *win32Window = static_cast(osWindow); - if (!win32Window->setPixelFormat(GetDefaultPixelFormatDescriptor())) + mWindow = osWindow->getNativeWindow(); + mDeviceContext = GetDC(mWindow); + const PIXELFORMATDESCRIPTOR pixelFormatDescriptor = GetDefaultPixelFormatDescriptor(); + + int pixelFormat = ChoosePixelFormat(mDeviceContext, &pixelFormatDescriptor); + if (pixelFormat == 0) { - std::cerr << "Failed to set default pixel format." << std::endl; + std::cerr << "Could not find a compatible pixel format." << std::endl; + DumpLastWindowsError(); return false; } - mWindow = osWindow->getNativeWindow(); - mDeviceContext = GetDC(mWindow); + // According to the Windows docs, it is an error to set a pixel format twice. + int currentPixelFormat = GetPixelFormat(mDeviceContext); + if (currentPixelFormat != pixelFormat) + { + if (SetPixelFormat(mDeviceContext, pixelFormat, &pixelFormatDescriptor) != TRUE) + { + std::cerr << "Failed to set the pixel format." << std::endl; + DumpLastWindowsError(); + return false; + } + } + mWGLContext = _wglCreateContext(mDeviceContext); if (!mWGLContext) { diff --git a/util/windows/win32/Win32Window.cpp b/util/windows/win32/Win32Window.cpp index 6f98b08ad..7d5145747 100644 --- a/util/windows/win32/Win32Window.cpp +++ b/util/windows/win32/Win32Window.cpp @@ -8,13 +8,10 @@ #include "util/windows/win32/Win32Window.h" -#include #include #include "common/debug.h" -namespace -{ Key VirtualKeyCodeToKey(WPARAM key, LPARAM flags) { switch (key) @@ -231,12 +228,6 @@ Key VirtualKeyCodeToKey(WPARAM key, LPARAM flags) return Key(0); } -void DumpLastWindowsError() -{ - std::cerr << "Last Windows error code: 0x" << std::hex << GetLastError() << std::endl; -} -} // namespace - LRESULT CALLBACK Win32Window::WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) { switch (message) @@ -494,10 +485,7 @@ LRESULT CALLBACK Win32Window::WndProc(HWND hWnd, UINT message, WPARAM wParam, LP } Win32Window::Win32Window() - : mParentClassRegistered(false), - mChildClassRegistered(false), - mPixelFormatIsSet(false), - mIsVisible(false), + : mIsVisible(false), mSetVisibleTimer(CreateTimer()), mIsMouseInWindow(false), mNativeWindow(0), @@ -543,7 +531,6 @@ bool Win32Window::initialize(const std::string &name, size_t width, size_t heigh { return false; } - mParentClassRegistered = true; WNDCLASSEXA childWindowClass = {0}; childWindowClass.cbSize = sizeof(WNDCLASSEXA); @@ -561,7 +548,6 @@ bool Win32Window::initialize(const std::string &name, size_t width, size_t heigh { return false; } - mChildClassRegistered = true; DWORD parentStyle = WS_CAPTION | WS_THICKFRAME | WS_MINIMIZEBOX | WS_MAXIMIZEBOX | WS_SYSMENU; DWORD parentExtendedStyle = WS_EX_APPWINDOW; @@ -574,10 +560,7 @@ bool Win32Window::initialize(const std::string &name, size_t width, size_t heigh sizeRect.right - sizeRect.left, sizeRect.bottom - sizeRect.top, nullptr, nullptr, GetModuleHandle(nullptr), this); - // An OpenGL window should be created with the WS_CLIPCHILDREN and WS_CLIPSIBLINGS styles. - // Additionally, the window class attribute should not include the CS_PARENTDC style. - mNativeWindow = CreateWindowExA(WS_EX_NOPARENTNOTIFY, mChildClassName.c_str(), name.c_str(), - WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS, 0, 0, + mNativeWindow = CreateWindowExA(0, mChildClassName.c_str(), name.c_str(), WS_CHILD, 0, 0, static_cast(width), static_cast(height), mParentWindow, nullptr, GetModuleHandle(nullptr), this); @@ -611,16 +594,8 @@ void Win32Window::destroy() mParentWindow = 0; } - if (mParentClassRegistered) - { - UnregisterClassA(mParentClassName.c_str(), nullptr); - mParentClassRegistered = false; - } - if (mChildClassRegistered) - { - UnregisterClassA(mChildClassName.c_str(), nullptr); - mChildClassRegistered = false; - } + UnregisterClassA(mParentClassName.c_str(), nullptr); + UnregisterClassA(mChildClassName.c_str(), nullptr); } bool Win32Window::takeScreenshot(uint8_t *pixelData) @@ -834,52 +809,6 @@ void Win32Window::setVisible(bool isVisible) } } -bool Win32Window::setPixelFormat(const PIXELFORMATDESCRIPTOR pixelFormatDescriptor) -{ - HDC deviceContext = GetDC(getNativeWindow()); - int pixelFormat = ChoosePixelFormat(deviceContext, &pixelFormatDescriptor); - if (pixelFormat == 0) - { - std::cerr << "Could not find a compatible pixel format." << std::endl; - DumpLastWindowsError(); - return false; - } - - ASSERT(GetLastError() == ERROR_SUCCESS); - int currentPixelFormat = GetPixelFormat(deviceContext); - if (GetLastError() != ERROR_SUCCESS) - { - // ERROR_INVALID_PIXEL_FORMAT is expected from GetPixelFormat, when !mPixelFormatIsSet, - // reset last error if it happens. - if (mPixelFormatIsSet || GetLastError() != ERROR_INVALID_PIXEL_FORMAT) - { - std::cerr << "Unexpected error calling GetPixelFormat: 0x" << std::hex << GetLastError() - << std::endl; - return false; - } - else - { - SetLastError(ERROR_SUCCESS); - } - } - - // According to the Windows docs, it is an error to set a pixel format twice. - if (mPixelFormatIsSet) - { - return currentPixelFormat == pixelFormat; - } - - if (SetPixelFormat(deviceContext, pixelFormat, &pixelFormatDescriptor) != TRUE) - { - std::cerr << "Failed to set the pixel format." << std::endl; - DumpLastWindowsError(); - return false; - } - - mPixelFormatIsSet = true; - return true; -} - void Win32Window::pushEvent(Event event) { OSWindow::pushEvent(event); @@ -887,10 +816,7 @@ void Win32Window::pushEvent(Event event) switch (event.Type) { case Event::EVENT_RESIZED: - if (mNativeWindow) - { - MoveWindow(mNativeWindow, 0, 0, mWidth, mHeight, FALSE); - } + MoveWindow(mNativeWindow, 0, 0, mWidth, mHeight, FALSE); break; default: break; diff --git a/util/windows/win32/Win32Window.h b/util/windows/win32/Win32Window.h index fd46a3b80..1846a1fe1 100644 --- a/util/windows/win32/Win32Window.h +++ b/util/windows/win32/Win32Window.h @@ -38,7 +38,6 @@ class Win32Window : public OSWindow bool setPosition(int x, int y) override; bool resize(int width, int height) override; void setVisible(bool isVisible) override; - bool setPixelFormat(const PIXELFORMATDESCRIPTOR pixelFormatDescriptor); void signalTestEvent() override; @@ -47,9 +46,6 @@ class Win32Window : public OSWindow std::string mParentClassName; std::string mChildClassName; - bool mParentClassRegistered; - bool mChildClassRegistered; - bool mPixelFormatIsSet; bool mIsVisible; Timer *mSetVisibleTimer;