DirectX based screen capturer logic

webrtc/modules/desktop_capture/desktop_geometry.h

 /*
  *  Copyright (c) 2013 The WebRTC 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 in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 87 matching lines @@
 
   DesktopRect() : left_(0), top_(0), right_(0), bottom_(0) {}
 
   int32_t left() const { return left_; }
   int32_t top() const { return top_; }
   int32_t right() const { return right_; }
   int32_t bottom() const { return bottom_; }
   int32_t width() const { return right_ - left_; }
   int32_t height() const { return bottom_ - top_; }
 
+  // Create a new DesktopRect instance to represent a rectangle which is
+  // |padding| pixels larger than current instance in each direction. This is
+  // for VideoEncoderVpx and ScreenCapturerWinDirectx.

[Sergey Ulanov, 2016/04/14 23:10:42]

Don't need this sentence (it will expire quickly after we start using this
function in other places)

[Hzj_jie, 2016/04/15 19:42:17]

Done.

+  // Note, left and top of the new instance won't be less than zero, and right
+  // and bottom wont't be larger than the desktop size.

[Sergey Ulanov, 2016/04/14 23:10:42]

So it looks like this function currently does two things: it expands the rect
and then crops it to fit the screen. I don't think it should be responsible for
both. We already have IntersectWith() to crop a rect, so you can achieve the
same effect using this expression:
  rect.Expand(padding);
  rect.IntersectWith(DesktopRect::MakeSize(size));

[Hzj_jie, 2016/04/15 19:42:17]

Done.

+  DesktopRect Expand(uint32_t padding, const DesktopSize& size) const;

[Sergey Ulanov, 2016/04/14 23:10:42]

Other functions in this class mutate the rect in-place. For consistency I think
we want to do the same thing here.

[Sergey Ulanov, 2016/04/14 23:10:42]

nit: Move this function below, after other mutators.

[Sergey Ulanov, 2016/04/14 23:10:42]

padding should be int.
From https://google.github.io/styleguide/cppguide.html#Integer_Types :
  document that a variable is non-negative using assertions. Don't use an
unsigned type.

[Hzj_jie, 2016/04/15 19:42:17]

Considering this is a simple class, mutable eventually make the code even
longer. Such as,

DesktopRect new_rect = existing_rect.Expand(100);

will become

DesktopRect new_rect = existing_rect;
new_rect.Expand(100);

I remember lots of Java simple classes are immutable, since copying a set of
pointers are actually cheap, which is the exactly similar scenario as here.

But I agree keeping consistency is also important.

[Hzj_jie, 2016/04/15 19:42:17]

Done.

[Hzj_jie, 2016/04/15 19:42:17]

Done.

+
   DesktopVector top_left() const { return DesktopVector(left_, top_); }
   DesktopSize size() const { return DesktopSize(width(), height()); }
 
   bool is_empty() const { return left_ >= right_ || top_ >= bottom_; }
 
   bool equals(const DesktopRect& other) const {
     return left_ == other.left_ && top_ == other.top_ &&
         right_ == other.right_ && bottom_ == other.bottom_;
   }
 
@@ skipping 18 matching lines @@
   int32_t left_;
   int32_t top_;
   int32_t right_;
   int32_t bottom_;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_DESKTOP_CAPTURE_DESKTOP_GEOMETRY_H_