Adding functionality to paint and signal buffer swap for ozone surface factory.

ui/base/ozone/surface_factory_ozone.h

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_BASE_OZONE_SURFACE_LNUX_FACTORY_OZONE_H_
 #define UI_BASE_OZONE_SURFACE_LNUX_FACTORY_OZONE_H_
 
+#include <EGL/egl.h>

[rjkroege, 2013/08/26 21:48:08]

this is not a good idea. 

+
 #include "ui/base/ui_export.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/rect.h"
 
+class SkBitmap;

[rjkroege, 2013/08/26 21:48:08]

SKia isn't in a name space? interesting. I learn something new everyday.

+
 namespace gfx {
 class Screen;
 class VSyncProvider;
 } //  namespace gfx
 
 namespace ui {
 
 class UI_EXPORT SurfaceFactoryOzone {
  public:
   SurfaceFactoryOzone();
   virtual ~SurfaceFactoryOzone();
 
   // Returns the instance
   static SurfaceFactoryOzone* GetInstance();
 
   // Returns a display spec as in |CreateDisplayFromSpec| for the default
   // native surface.
   virtual const char* DefaultDisplaySpec();
 
   // Sets the implementation delegate. Ownership is retained by the caller.
   static void SetInstance(SurfaceFactoryOzone* impl);
 
   // TODO(rjkroege): decide how to separate screen/display stuff from SFOz
   // This method implements gfx::Screen, particularly useful in Desktop Aura.
   virtual gfx::Screen* CreateDesktopScreen();
 
-  // TODO(rjkroege): Add a status code if necessary.
   // Configures the display hardware. Must be called from within the GPU
   // process before the sandbox has been activated.
-  virtual void InitializeHardware() = 0;
+  virtual bool InitializeHardware() = 0;

[rjkroege, 2013/08/26 21:48:08]

an enum would be preferable? And what does the status mean? :-)

[dnicoara, 2013/08/27 14:41:41]

I'm not sure what the status would be. I'm seeing this as a binary problem where
the hardware is either initialized or not.

Wouldn't a status be implementation dependent?

 
   // Cleans up display hardware state. Call this from within the GPU process.
   // This method must be safe to run inside of the sandbox.
   virtual void ShutdownHardware() = 0;
 
+  // Returns the native EGL display. This is generally needed in creating
+  // EGL windows.
+  virtual EGLNativeDisplayType GetNativeDisplay() = 0;

[rjkroege, 2013/08/26 21:48:08]

return an int. Or we need to modify some of the dfn's in khronos/egl.h to have
some kind of typedef that does not require the EGL display stuff here.

Also: please make these methods not pure virtual to avoid breaking down-stream
implementors... We can make them pure once that the downstream implementations
have had a chance to catch up.

[dnicoara, 2013/08/27 14:41:41]

Wouldn't this be platform dependent? I'm not sure how having it as an 'int' work
without having multiple #ifdefs in gl_surface_egl to perform the appropriate
casts/initializations.

That said, Symbian is the only platform where it is defined as an 'int'. Others
use a pointer type, so returning a 'void*' may be acceptable.

The other alternative is to push eglGetDisplay into the surface factory, so we'd
hide this all together. In this case I'd replace this function with "EGLDisplay
GetEglDisplay()".

+
   // Obtains an AcceleratedWidget backed by a native Linux framebuffer.
   // The  returned AcceleratedWidget is an opaque token that must realized
   // before it can be used to create a GL surface.
   virtual gfx::AcceleratedWidget GetAcceleratedWidget() = 0;
 
   // Realizes an AcceleratedWidget so that the returned AcceleratedWidget
   // can be used to to create a GLSurface. This method may only be called in
   // a process that has a valid GL context.
   virtual gfx::AcceleratedWidget RealizeAcceleratedWidget(
       gfx::AcceleratedWidget w) = 0;
 
   // Sets up GL bindings for the native surface.
   virtual bool LoadEGLGLES2Bindings() = 0;
 
   // If possible attempts to resize the given AcceleratedWidget instance and if
   // a resize action was performed returns true, otherwise false (native
   // hardware may only support a single fixed size).
   virtual bool AttemptToResizeAcceleratedWidget(
       gfx::AcceleratedWidget w,
       const gfx::Rect& bounds) = 0;
 
+  // Called after the appropriate GL swap buffers command. Used if extra work
+  // is needed to perform the actual buffer swap.
+  virtual bool SwapBuffers() = 0;

[rjkroege, 2013/08/26 21:48:08]

this should take the accelerated widget as an argument in case there are
multiple different widgets with different vsync properties?

[dnicoara, 2013/08/27 14:41:41]

Done.

+
   // Returns a gfx::VsyncProvider for the provided AcceleratedWidget. Note
   // that this may be called after we have entered the sandbox so if there are
   // operations (e.g. opening a file descriptor providing vsync events) that
   // must be done outside of the sandbox, they must have been completed
   // in InitializeHardware. Returns NULL on error.
   virtual gfx::VSyncProvider* GetVSyncProvider(gfx::AcceleratedWidget w) = 0;
 
+  // Used to paint a bitmap directly to the surface.

[rjkroege, 2013/08/26 21:48:08]

You are not using this method in this CL yes? Perhaps it belongs in a different
CL where it is becoming a requirement?

[dnicoara, 2013/08/27 14:41:41]

Sure, I was thinking of having it in here to discuss the API, but we can do it
later.

+  virtual void Paint(const SkBitmap& bitmap, const gfx::Rect& rect) = 0;
+
   // Create a default SufaceFactoryOzone implementation useful for tests.
   static SurfaceFactoryOzone* CreateTestHelper();
 
  private:
   static SurfaceFactoryOzone* impl_; // not owned
 };
 
 }  // namespace ui
 
-
 #endif  // UI_BASE_OZONE_SURFACE_LNUX_FACTORY_OZONE_H_