[WebRTC] A real ScreenCapturer test

webrtc/modules/desktop_capture/screen_drawer.h

Index: webrtc/modules/desktop_capture/screen_drawer.h
diff --git a/webrtc/modules/desktop_capture/screen_drawer.h b/webrtc/modules/desktop_capture/screen_drawer.h
index 899fc0f043b841f310ec1a2ae3f09f43875cc996..5a02e31c843413a21477e66752aeb63c2fe2c2ae 100644
--- a/webrtc/modules/desktop_capture/screen_drawer.h
+++ b/webrtc/modules/desktop_capture/screen_drawer.h
@@ -15,30 +15,43 @@
 
 #include <memory>
 
+#include "webrtc/modules/desktop_capture/color.h"
 #include "webrtc/modules/desktop_capture/desktop_geometry.h"
 
 namespace webrtc {
 
-// A set of platform independent functions to draw various of shapes on the
-// screen. This class is for testing ScreenCapturer* implementations only, and
+// A set of basic platform dependent functions to draw various shapes on the
+// screen.
+// Usually ScreenPainter is preferred, instead of using this class directly. As

[Jamie, 2016/08/26 22:29:10]

ScreenPainter no longer exists. Please update the comment.

[Hzj_jie, 2016/08/29 21:57:28]

Done.

+// ScreenPainter provides more complex shapes by using ScreenDrawer for each
+// platform. This class is for testing ScreenCapturer* implementations only, and
 // should not be used in production logic.
 class ScreenDrawer {
  public:
-  // Creates a ScreenDrawer for the current platform.
+  // Creates a ScreenDrawer for the current platform, returns nullptr if no
+  // ScreenDrawer implementation available.
   static std::unique_ptr<ScreenDrawer> Create();
 
   ScreenDrawer() {}
   virtual ~ScreenDrawer() {}
 
-  // Returns a rect, on which this instance can draw.
+  // Returns a rectangle, on which this instance can draw. This rectangle maps
+  // to the physical monitor, i.e. a dot drawing at (100, 100) of a ScreenDrawer
+  // will be captured at (100, 100) by a ScreenCapturer.

[Jamie, 2016/08/26 22:29:10]

Assuming it won't break existing code, let's try to stick to a single
frame-of-reference (the one used by the capturer). Then this comment should be
something like:

Returns the region inside which DrawRectangle is expected to work, in capturer
coordinates (assuming screenCapturer::SelectScreen has not been called). This
region excludes regions of the screen reserved by the OS for things like menu
bars or app launchers.

[Hzj_jie, 2016/08/29 21:57:28]

Done.
One minor change, 'This region _may_ excludes regions of the screen ...'.
ScreenDrawerWin can cover entire screen.

[Jamie, 2016/08/31 17:39:39]

That also works (but if you add "may" you also need to use "exclude", not
"excludes").

[Hzj_jie, 2016/08/31 21:22:39]

:(

   virtual DesktopRect DrawableRegion() = 0;
 
-  // Draws a rectangle to cover |rect| with color |rgba|. Note, rect.bottom()
-  // and rect.right() two lines are not included.
-  virtual void DrawRectangle(DesktopRect rect, uint32_t rgba) = 0;
+  // Draws a rectangle to cover |rect| with |color|. Note, rect.bottom() and
+  // rect.right() two lines are not included. The part of |rect| which is out of
+  // DrawableRegion() will be ignored.
+  virtual void DrawRectangle(DesktopRect rect, Color color) = 0;
 
-  // Clears all content on the screen.
+  // Clears all content on the screen by filling the area with black.
   virtual void Clear() = 0;
+
+  // Blocks current thread until OS finishes previous Draw* actions.

[Jamie, 2016/08/26 22:29:10]

Since there's only DrawRectangle, Draw* is unnecessarily vague :)

[Hzj_jie, 2016/08/29 21:57:28]

Done.

+  // ScreenCapturer should be able to capture the changes after this function
+  // finish.
+  virtual void WaitForPendingPaintings() = 0;

[Jamie, 2016/08/26 22:29:10]

s/Paintings/Draws/ (since that's the verb the corresponding function uses).

[Hzj_jie, 2016/08/29 21:57:28]

Done.

 };
 
 }  // namespace webrtc