Make inheritance heirarchy for egl::Surface.

src/libANGLE/Display.cpp

@@ -407,17 +407,8 @@ Error Display::initialize()
         if (mDisplayExtensions.deviceQuery)
         {
             rx::DeviceImpl *impl = nullptr;
-            error = mImplementation->getDevice(&impl);
-            if (error.isError())
-            {
-                return error;
-            }
-
-            error = Device::CreateDevice(this, impl, &mDevice);
-            if (error.isError())
-            {
-                return error;
-            }
+            ANGLE_TRY(mImplementation->getDevice(&impl));
+            ANGLE_TRY(Device::CreateDevice(this, impl, &mDevice));
         }
         else
         {
@@ -539,32 +530,21 @@ Error Display::createWindowSurface(const Config *configuration, EGLNativeWindowT
 {
     if (mImplementation->testDeviceLost())
     {
-        Error error = restoreLostDevice();
-        if (error.isError())
-        {
-            return error;
-        }
+        ANGLE_TRY(restoreLostDevice());
     }
 
-    rx::SurfaceImpl *surfaceImpl = mImplementation->createWindowSurface(configuration, window, attribs);
-    ASSERT(surfaceImpl != nullptr);
+    std::unique_ptr<Surface> surface(
+        new WindowSurface(mImplementation, configuration, window, attribs));
+    ANGLE_TRY(surface->initialize());
 
-    Error error = surfaceImpl->initialize();
-    if (error.isError())
-    {
-        SafeDelete(surfaceImpl);
-        return error;
-    }
-
-    Surface *surface = new Surface(surfaceImpl, EGL_WINDOW_BIT, configuration, attribs);
-    mImplementation->getSurfaceSet().insert(surface);
+    ASSERT(outSurface != nullptr);
+    *outSurface = surface.release();
+    mImplementation->getSurfaceSet().insert(*outSurface);
 
     WindowSurfaceMap *windowSurfaces = GetWindowSurfaces();
     ASSERT(windowSurfaces && windowSurfaces->find(window) == windowSurfaces->end());
-    windowSurfaces->insert(std::make_pair(window, surface));
+    windowSurfaces->insert(std::make_pair(window, *outSurface));
 
-    ASSERT(outSurface != nullptr);
-    *outSurface = surface;
     return egl::Error(EGL_SUCCESS);
 }
 
@@ -574,28 +554,16 @@ Error Display::createPbufferSurface(const Config *configuration, const Attribute
 
     if (mImplementation->testDeviceLost())
     {
-        Error error = restoreLostDevice();
-        if (error.isError())
-        {
-            return error;
-        }
-    }
-
-    rx::SurfaceImpl *surfaceImpl = mImplementation->createPbufferSurface(configuration, attribs);
-    ASSERT(surfaceImpl != nullptr);
-
-    Error error = surfaceImpl->initialize();
-    if (error.isError())
-    {
-        SafeDelete(surfaceImpl);
-        return error;
+        ANGLE_TRY(restoreLostDevice());
     }
 
-    Surface *surface = new Surface(surfaceImpl, EGL_PBUFFER_BIT, configuration, attribs);
-    mImplementation->getSurfaceSet().insert(surface);
+    std::unique_ptr<Surface> surface(new PbufferSurface(mImplementation, configuration, attribs));
+    ANGLE_TRY(surface->initialize());
 
     ASSERT(outSurface != nullptr);
-    *outSurface = surface;
+    *outSurface = surface.release();
+    mImplementation->getSurfaceSet().insert(*outSurface);
+
     return egl::Error(EGL_SUCCESS);
 }
 
@@ -606,28 +574,17 @@ Error Display::createPbufferFromClientBuffer(const Config *configuration, EGLCli
 
     if (mImplementation->testDeviceLost())
     {
-        Error error = restoreLostDevice();
-        if (error.isError())
-        {
-            return error;
-        }
-    }
-
-    rx::SurfaceImpl *surfaceImpl = mImplementation->createPbufferFromClientBuffer(configuration, shareHandle, attribs);
-    ASSERT(surfaceImpl != nullptr);
-
-    Error error = surfaceImpl->initialize();
-    if (error.isError())
-    {
-        SafeDelete(surfaceImpl);
-        return error;
+        ANGLE_TRY(restoreLostDevice());
     }
 
-    Surface *surface = new Surface(surfaceImpl, EGL_PBUFFER_BIT, configuration, attribs);
-    mImplementation->getSurfaceSet().insert(surface);
+    std::unique_ptr<Surface> surface(
+        new PbufferSurface(mImplementation, configuration, shareHandle, attribs));
+    ANGLE_TRY(surface->initialize());
 
     ASSERT(outSurface != nullptr);
-    *outSurface = surface;
+    *outSurface = surface.release();
+    mImplementation->getSurfaceSet().insert(*outSurface);
+
     return egl::Error(EGL_SUCCESS);
 }
 
@@ -638,28 +595,17 @@ Error Display::createPixmapSurface(const Config *configuration, NativePixmapType
 
     if (mImplementation->testDeviceLost())
     {
-        Error error = restoreLostDevice();
-        if (error.isError())
-        {
-            return error;
-        }
-    }
-
-    rx::SurfaceImpl *surfaceImpl = mImplementation->createPixmapSurface(configuration, nativePixmap, attribs);
-    ASSERT(surfaceImpl != nullptr);
-
-    Error error = surfaceImpl->initialize();
-    if (error.isError())
-    {
-        SafeDelete(surfaceImpl);
-        return error;
+        ANGLE_TRY(restoreLostDevice());
     }
 
-    Surface *surface = new Surface(surfaceImpl, EGL_PIXMAP_BIT, configuration, attribs);
-    mImplementation->getSurfaceSet().insert(surface);
+    std::unique_ptr<Surface> surface(
+        new PixmapSurface(mImplementation, configuration, nativePixmap, attribs));
+    ANGLE_TRY(surface->initialize());
 
     ASSERT(outSurface != nullptr);
-    *outSurface = surface;
+    *outSurface = surface.release();
+    mImplementation->getSurfaceSet().insert(*outSurface);
+
     return egl::Error(EGL_SUCCESS);
 }
 
@@ -673,11 +619,7 @@ Error Display::createImage(gl::Context *context,
 
     if (mImplementation->testDeviceLost())
     {
-        Error error = restoreLostDevice();
-        if (error.isError())
-        {
-            return error;
-        }
+        ANGLE_TRY(restoreLostDevice());
     }
 
     egl::ImageSibling *sibling = nullptr;
@@ -698,11 +640,7 @@ Error Display::createImage(gl::Context *context,
     rx::ImageImpl *imageImpl = mImplementation->createImage(target, sibling, attribs);
     ASSERT(imageImpl != nullptr);
 
-    Error error = imageImpl->initialize();
-    if (error.isError())
-    {
-        return error;
-    }
+    ANGLE_TRY(imageImpl->initialize());
 
     Image *image = new Image(imageImpl, target, sibling, attribs);
 
@@ -753,11 +691,7 @@ Error Display::createContext(const Config *configuration, gl::Context *shareCont
 
 Error Display::makeCurrent(egl::Surface *drawSurface, egl::Surface *readSurface, gl::Context *context)
 {
-    Error error = mImplementation->makeCurrent(drawSurface, readSurface, context);
-    if (error.isError())
-    {
-        return error;
-    }
+    ANGLE_TRY(mImplementation->makeCurrent(drawSurface, readSurface, context));
 
     if (context != nullptr && drawSurface != nullptr)
     {

src/libANGLE/Surface.cpp

@@ -10,14 +10,16 @@
 
 #include "libANGLE/Surface.h"
 
-#include "libANGLE/Config.h"
-#include "libANGLE/Framebuffer.h"
-#include "libANGLE/Texture.h"
 
 #include <EGL/eglext.h>
 
 #include <iostream>
 
+#include "libANGLE/Config.h"
+#include "libANGLE/Framebuffer.h"
+#include "libANGLE/Texture.h"
+#include "libANGLE/renderer/EGLImplFactory.h"
+
 namespace egl
 {
 
@@ -65,9 +67,6 @@ Surface::Surface(rx::SurfaceImpl *impl,
     }
 
     mOrientation = static_cast<EGLint>(attributes.get(EGL_SURFACE_ORIENTATION_ANGLE, 0));
-
-    mDefaultFramebuffer = createDefaultFramebuffer();
-    ASSERT(mDefaultFramebuffer != nullptr);
 }
 
 Surface::~Surface()
@@ -86,6 +85,17 @@ Surface::~Surface()
     SafeDelete(mImplementation);
 }
 
+Error Surface::initialize()
+{
+    ANGLE_TRY(mImplementation->initialize());
+
+    // Must happen after implementation initialize for OSX.
+    mDefaultFramebuffer = createDefaultFramebuffer();
+    ASSERT(mDefaultFramebuffer != nullptr);
+
+    return Error(EGL_SUCCESS);
+}
+
 void Surface::setIsCurrent(bool isCurrent)
 {
     if (isCurrent)
@@ -258,4 +268,57 @@ gl::Framebuffer *Surface::createDefaultFramebuffer()
 
     return framebuffer;
 }
+
+WindowSurface::WindowSurface(rx::EGLImplFactory *implFactory,
+                             const egl::Config *config,
+                             EGLNativeWindowType window,
+                             const AttributeMap &attribs)
+    : Surface(implFactory->createWindowSurface(config, window, attribs),
+              EGL_WINDOW_BIT,
+              config,
+              attribs)
+{
+}
+
+WindowSurface::~WindowSurface()
+{
+}
+
+PbufferSurface::PbufferSurface(rx::EGLImplFactory *implFactory,
+                               const Config *config,
+                               const AttributeMap &attribs)
+    : Surface(implFactory->createPbufferSurface(config, attribs), EGL_PBUFFER_BIT, config, attribs)
+{
+}
+
+PbufferSurface::PbufferSurface(rx::EGLImplFactory *implFactory,
+                               const Config *config,
+                               EGLClientBuffer shareHandle,
+                               const AttributeMap &attribs)
+    : Surface(implFactory->createPbufferFromClientBuffer(config, shareHandle, attribs),
+              EGL_PBUFFER_BIT,
+              config,
+              attribs)
+{
 }
+
+PbufferSurface::~PbufferSurface()
+{
+}
+
+PixmapSurface::PixmapSurface(rx::EGLImplFactory *implFactory,
+                             const Config *config,
+                             NativePixmapType nativePixmap,
+                             const AttributeMap &attribs)
+    : Surface(implFactory->createPixmapSurface(config, nativePixmap, attribs),
+              EGL_PIXMAP_BIT,
+              config,
+              attribs)
+{
+}
+
+PixmapSurface::~PixmapSurface()
+{
+}
+
+}  // namespace egl

src/libANGLE/Surface.h

@@ -25,22 +25,27 @@ class Framebuffer;
 class Texture;
 }
 
+namespace rx
+{
+class EGLImplFactory;
+}
+
 namespace egl
 {
 class AttributeMap;
 class Display;
 struct Config;
 
-class Surface final : public gl::FramebufferAttachmentObject
+class Surface : public gl::FramebufferAttachmentObject
 {
   public:
-    Surface(rx::SurfaceImpl *impl, EGLint surfaceType, const egl::Config *config, const AttributeMap &attributes);
+    virtual ~Surface();
 
-    rx::SurfaceImpl *getImplementation() { return mImplementation; }
-    const rx::SurfaceImpl *getImplementation() const { return mImplementation; }
+    rx::SurfaceImpl *getImplementation() const { return mImplementation; }
 
     EGLint getType() const;
 
+    Error initialize();
     Error swap();
     Error postSubBuffer(EGLint x, EGLint y, EGLint width, EGLint height);
     Error querySurfacePointerANGLE(EGLint attribute, void **value);
@@ -86,8 +91,11 @@ class Surface final : public gl::FramebufferAttachmentObject
 
     bool directComposition() const { return mDirectComposition; }
 
-  private:
-    virtual ~Surface();
+  protected:
+    Surface(rx::SurfaceImpl *impl,
+            EGLint surfaceType,
+            const egl::Config *config,
+            const AttributeMap &attributes);
     rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const override { return mImplementation; }
 
     gl::Framebuffer *createDefaultFramebuffer();
@@ -126,6 +134,39 @@ class Surface final : public gl::FramebufferAttachmentObject
     BindingPointer<gl::Texture> mTexture;
 };
 
-}
+class WindowSurface final : public Surface
+{
+  public:
+    WindowSurface(rx::EGLImplFactory *implFactory,
+                  const Config *config,
+                  EGLNativeWindowType window,
+                  const AttributeMap &attribs);
+    ~WindowSurface() override;
+};
+
+class PbufferSurface final : public Surface
+{
+  public:
+    PbufferSurface(rx::EGLImplFactory *implFactory,
+                   const Config *config,
+                   const AttributeMap &attribs);
+    PbufferSurface(rx::EGLImplFactory *implFactory,
+                   const Config *config,
+                   EGLClientBuffer shareHandle,
+                   const AttributeMap &attribs);
+    ~PbufferSurface() override;
+};
+
+class PixmapSurface final : public Surface
+{
+  public:
+    PixmapSurface(rx::EGLImplFactory *implFactory,
+                  const Config *config,
+                  NativePixmapType nativePixmap,
+                  const AttributeMap &attribs);
+    ~PixmapSurface() override;
+};
+
+}  // namespace egl
 
 #endif   // LIBANGLE_SURFACE_H_

src/libANGLE/Surface_unittest.cpp

@@ -14,8 +14,10 @@
 #include "libANGLE/Surface.h"
 #include "libANGLE/renderer/FramebufferImpl_mock.h"
 #include "libANGLE/renderer/SurfaceImpl.h"
+#include "tests/angle_unittests_utils.h"
 
 using namespace rx;
+using namespace testing;
 
 namespace
 {
@@ -44,16 +46,16 @@ class MockSurfaceImpl : public rx::SurfaceImpl
 
 TEST(SurfaceTest, DestructionDeletesImpl)
 {
-    MockFramebufferImpl *framebuffer = new MockFramebufferImpl;
+    NiceMock<MockEGLFactory> factory;
 
     MockSurfaceImpl *impl = new MockSurfaceImpl;
     EXPECT_CALL(*impl, getSwapBehavior());
-    EXPECT_CALL(*impl, createDefaultFramebuffer(testing::_)).WillOnce(testing::Return(framebuffer));
+    EXPECT_CALL(factory, createWindowSurface(_, _, _)).WillOnce(Return(impl));
 
     egl::Config config;
-    egl::Surface *surface = new egl::Surface(impl, EGL_WINDOW_BIT, &config, egl::AttributeMap());
+    egl::Surface *surface = new egl::WindowSurface(
+        &factory, &config, static_cast<EGLNativeWindowType>(0), egl::AttributeMap());
 
-    EXPECT_CALL(*framebuffer, destroy()).Times(1).RetiresOnSaturation();
     EXPECT_CALL(*impl, destroy()).Times(1).RetiresOnSaturation();
 
     surface->onDestroy();
@@ -61,7 +63,7 @@ TEST(SurfaceTest, DestructionDeletesImpl)
     // Only needed because the mock is leaked if bugs are present,
     // which logs an error, but does not cause the test to fail.
     // Ordinarily mocks are verified when destroyed.
-    testing::Mock::VerifyAndClear(impl);
+    Mock::VerifyAndClear(impl);
 }
 
 } // namespace

src/libANGLE/renderer/d3d/DisplayD3D.cpp

@@ -168,14 +168,14 @@ SurfaceImpl *DisplayD3D::createWindowSurface(const egl::Config *configuration,
                                              const egl::AttributeMap &attribs)
 {
     ASSERT(mRenderer != nullptr);
-    return SurfaceD3D::createFromWindow(mRenderer, mDisplay, configuration, window, attribs);
+    return new WindowSurfaceD3D(mRenderer, mDisplay, configuration, window, attribs);
 }
 
 SurfaceImpl *DisplayD3D::createPbufferSurface(const egl::Config *configuration,
                                               const egl::AttributeMap &attribs)
 {
     ASSERT(mRenderer != nullptr);
-    return SurfaceD3D::createOffscreen(mRenderer, mDisplay, configuration, nullptr, attribs);
+    return new PbufferSurfaceD3D(mRenderer, mDisplay, configuration, nullptr, attribs);
 }
 
 SurfaceImpl *DisplayD3D::createPbufferFromClientBuffer(const egl::Config *configuration,
@@ -183,7 +183,7 @@ SurfaceImpl *DisplayD3D::createPbufferFromClientBuffer(const egl::Config *config
                                                        const egl::AttributeMap &attribs)
 {
     ASSERT(mRenderer != nullptr);
-    return SurfaceD3D::createOffscreen(mRenderer, mDisplay, configuration, shareHandle, attribs);
+    return new PbufferSurfaceD3D(mRenderer, mDisplay, configuration, shareHandle, attribs);
 }
 
 SurfaceImpl *DisplayD3D::createPixmapSurface(const egl::Config *configuration,

src/libANGLE/renderer/d3d/SurfaceD3D.cpp

@@ -21,26 +21,6 @@
 namespace rx
 {
 
-SurfaceD3D *SurfaceD3D::createFromWindow(RendererD3D *renderer,
-                                         egl::Display *display,
-                                         const egl::Config *config,
-                                         EGLNativeWindowType window,
-                                         const egl::AttributeMap &attribs)
-{
-    return new SurfaceD3D(renderer, display, config, window, static_cast<EGLClientBuffer>(0),
-                          attribs);
-}
-
-SurfaceD3D *SurfaceD3D::createOffscreen(RendererD3D *renderer,
-                                        egl::Display *display,
-                                        const egl::Config *config,
-                                        EGLClientBuffer shareHandle,
-                                        const egl::AttributeMap &attribs)
-{
-    return new SurfaceD3D(renderer, display, config, static_cast<EGLNativeWindowType>(0),
-                          shareHandle, attribs);
-}
-
 SurfaceD3D::SurfaceD3D(RendererD3D *renderer,
                        egl::Display *display,
                        const egl::Config *config,
@@ -354,4 +334,35 @@ gl::Error SurfaceD3D::getAttachmentRenderTarget(const gl::FramebufferAttachment:
     return gl::Error(GL_NO_ERROR);
 }
 
+WindowSurfaceD3D::WindowSurfaceD3D(RendererD3D *renderer,
+                                   egl::Display *display,
+                                   const egl::Config *config,
+                                   EGLNativeWindowType window,
+                                   const egl::AttributeMap &attribs)
+    : SurfaceD3D(renderer, display, config, window, static_cast<EGLClientBuffer>(0), attribs)
+{
 }
+
+WindowSurfaceD3D::~WindowSurfaceD3D()
+{
+}
+
+PbufferSurfaceD3D::PbufferSurfaceD3D(RendererD3D *renderer,
+                                     egl::Display *display,
+                                     const egl::Config *config,
+                                     EGLClientBuffer shareHandle,
+                                     const egl::AttributeMap &attribs)
+    : SurfaceD3D(renderer,
+                 display,
+                 config,
+                 static_cast<EGLNativeWindowType>(0),
+                 shareHandle,
+                 attribs)
+{
+}
+
+PbufferSurfaceD3D::~PbufferSurfaceD3D()
+{
+}
+
+}  // namespace rc

src/libANGLE/renderer/d3d/SurfaceD3D.h

@@ -25,16 +25,6 @@ class RendererD3D;
 class SurfaceD3D : public SurfaceImpl
 {
   public:
-    static SurfaceD3D *createFromWindow(RendererD3D *renderer,
-                                        egl::Display *display,
-                                        const egl::Config *config,
-                                        EGLNativeWindowType window,
-                                        const egl::AttributeMap &attribs);
-    static SurfaceD3D *createOffscreen(RendererD3D *renderer,
-                                       egl::Display *display,
-                                       const egl::Config *config,
-                                       EGLClientBuffer shareHandle,
-                                       const egl::AttributeMap &attribs);
     ~SurfaceD3D() override;
     void releaseSwapChain();
 
@@ -65,7 +55,7 @@ class SurfaceD3D : public SurfaceImpl
     gl::Error getAttachmentRenderTarget(const gl::FramebufferAttachment::Target &target,
                                         FramebufferAttachmentRenderTarget **rtOut) override;
 
-  private:
+  protected:
     SurfaceD3D(RendererD3D *renderer,
                egl::Display *display,
                const egl::Config *config,
@@ -98,7 +88,28 @@ class SurfaceD3D : public SurfaceImpl
     HANDLE mShareHandle;
 };
 
+class WindowSurfaceD3D : public SurfaceD3D
+{
+  public:
+    WindowSurfaceD3D(RendererD3D *renderer,
+                     egl::Display *display,
+                     const egl::Config *config,
+                     EGLNativeWindowType window,
+                     const egl::AttributeMap &attribs);
+    ~WindowSurfaceD3D() override;
+};
 
-}
+class PbufferSurfaceD3D : public SurfaceD3D
+{
+  public:
+    PbufferSurfaceD3D(RendererD3D *renderer,
+                      egl::Display *display,
+                      const egl::Config *config,
+                      EGLClientBuffer shareHandle,
+                      const egl::AttributeMap &attribs);
+    ~PbufferSurfaceD3D() override;
+};
+
+}  // namespace rx
 
 #endif // LIBANGLE_RENDERER_D3D_SURFACED3D_H_