From acc39c44bc901caace84e2f6671e67669194ee02 Mon Sep 17 00:00:00 2001 From: zijiehe Date: Wed, 21 Sep 2016 12:23:15 -0700 Subject: [PATCH] Use RgbaColor in DesktopFrameGenerator and add RgbaColorTest This change uses RgbaColor in DesktopFrameGenerator instead of raw uint32_t to avoid potential endian issues. BUG=633802 Review-Url: https://codereview.webrtc.org/2334853002 Cr-Commit-Position: refs/heads/master@{#14337} --- webrtc/modules/BUILD.gn | 1 + .../desktop_frame_generator.cc | 12 +++-- webrtc/modules/desktop_capture/rgba_color.cc | 13 ++++++ webrtc/modules/desktop_capture/rgba_color.h | 8 ++++ .../desktop_capture/rgba_color_unittest.cc | 44 +++++++++++++++++++ 5 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 webrtc/modules/desktop_capture/rgba_color_unittest.cc diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn index 60802ebcbe..3f1ea747ff 100644 --- a/webrtc/modules/BUILD.gn +++ b/webrtc/modules/BUILD.gn @@ -521,6 +521,7 @@ if (rtc_include_tests) { "desktop_capture/mouse_cursor_monitor_unittest.cc", "desktop_capture/rgba_color.cc", "desktop_capture/rgba_color.h", + "desktop_capture/rgba_color_unittest.cc", "desktop_capture/screen_capturer_differ_wrapper_unittest.cc", "desktop_capture/screen_capturer_helper_unittest.cc", "desktop_capture/screen_capturer_mac_unittest.cc", diff --git a/webrtc/modules/desktop_capture/desktop_frame_generator.cc b/webrtc/modules/desktop_capture/desktop_frame_generator.cc index ae5c9492cb..608c8407d8 100644 --- a/webrtc/modules/desktop_capture/desktop_frame_generator.cc +++ b/webrtc/modules/desktop_capture/desktop_frame_generator.cc @@ -17,6 +17,7 @@ #include "webrtc/base/random.h" #include "webrtc/base/timeutils.h" +#include "webrtc/modules/desktop_capture/rgba_color.h" namespace webrtc { @@ -60,11 +61,12 @@ void SetUpdatedRegion(DesktopFrame* frame, } // Paints pixels in |rect| of |frame| to |color|. -void PaintRect(DesktopFrame* frame, DesktopRect rect, uint32_t color) { +void PaintRect(DesktopFrame* frame, DesktopRect rect, RgbaColor rgba_color) { static_assert(DesktopFrame::kBytesPerPixel == sizeof(uint32_t), "kBytesPerPixel should be 4."); RTC_DCHECK(frame->size().width() >= rect.right() && frame->size().height() >= rect.bottom()); + uint32_t color = rgba_color.ToUInt32(); uint8_t* row = frame->GetFrameDataAtPos(rect.top_left()); for (int i = 0; i < rect.height(); i++) { uint32_t* column = reinterpret_cast(row); @@ -76,10 +78,12 @@ void PaintRect(DesktopFrame* frame, DesktopRect rect, uint32_t color) { } // Paints pixels in |region| of |frame| to |color|. -void PaintRegion(DesktopFrame* frame, DesktopRegion* region, uint32_t color) { +void PaintRegion(DesktopFrame* frame, + DesktopRegion* region, + RgbaColor rgba_color) { region->IntersectWith(DesktopRect::MakeSize(frame->size())); for (DesktopRegion::Iterator it(*region); !it.IsAtEnd(); it.Advance()) { - PaintRect(frame, it.rect(), color); + PaintRect(frame, it.rect(), rgba_color); } } @@ -171,7 +175,7 @@ bool BlackWhiteDesktopFramePainter::Paint(DesktopFrame* frame, DesktopRegion* updated_region) { RTC_DCHECK(updated_region->is_empty()); memset(frame->data(), 0, frame->stride() * frame->size().height()); - PaintRegion(frame, &updated_region_, UINT32_MAX); + PaintRegion(frame, &updated_region_, RgbaColor(0xFFFFFFFF)); updated_region_.Swap(updated_region); return true; } diff --git a/webrtc/modules/desktop_capture/rgba_color.cc b/webrtc/modules/desktop_capture/rgba_color.cc index 2342b46915..6d91de4275 100644 --- a/webrtc/modules/desktop_capture/rgba_color.cc +++ b/webrtc/modules/desktop_capture/rgba_color.cc @@ -10,6 +10,8 @@ #include "webrtc/modules/desktop_capture/rgba_color.h" +#include "webrtc/typedefs.h" + namespace webrtc { namespace { @@ -36,6 +38,9 @@ RgbaColor::RgbaColor(uint8_t blue, uint8_t green, uint8_t red) RgbaColor::RgbaColor(const uint8_t* bgra) : RgbaColor(bgra[0], bgra[1], bgra[2], bgra[3]) {} +RgbaColor::RgbaColor(uint32_t bgra) + : RgbaColor(reinterpret_cast(&bgra)) {} + bool RgbaColor::operator==(const RgbaColor& right) const { return blue == right.blue && green == right.green && red == right.red && AlphaEquals(alpha, right.alpha); @@ -45,4 +50,12 @@ bool RgbaColor::operator!=(const RgbaColor& right) const { return !(*this == right); } +uint32_t RgbaColor::ToUInt32() const { +#if defined(WEBRTC_ARCH_LITTLE_ENDIAN) + return blue | (green << 8) | (red << 16) | (alpha << 24); +#else + return (blue << 24) | (green << 16) | (red << 8) | alpha; +#endif +} + } // namespace webrtc diff --git a/webrtc/modules/desktop_capture/rgba_color.h b/webrtc/modules/desktop_capture/rgba_color.h index ff4a258415..1236d03ee6 100644 --- a/webrtc/modules/desktop_capture/rgba_color.h +++ b/webrtc/modules/desktop_capture/rgba_color.h @@ -34,12 +34,20 @@ struct RgbaColor final { // Creates a color from four-byte in BGRA order, i.e. DesktopFrame::data(). explicit RgbaColor(const uint8_t* bgra); + // Creates a color from BGRA channels in a uint format. Consumers should make + // sure the memory order of the uint32_t is always BGRA from left to right, no + // matter the system endian. This function creates an equivalent RgbaColor + // instance from the ToUInt32() result of another RgbaColor instance. + explicit RgbaColor(uint32_t bgra); + // Returns true if |this| and |right| is the same color. bool operator==(const RgbaColor& right) const; // Returns true if |this| and |right| are different colors. bool operator!=(const RgbaColor& right) const; + uint32_t ToUInt32() const; + uint8_t blue; uint8_t green; uint8_t red; diff --git a/webrtc/modules/desktop_capture/rgba_color_unittest.cc b/webrtc/modules/desktop_capture/rgba_color_unittest.cc new file mode 100644 index 0000000000..33e4fb7474 --- /dev/null +++ b/webrtc/modules/desktop_capture/rgba_color_unittest.cc @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2016 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. + */ + +#include "webrtc/modules/desktop_capture/rgba_color.h" + +#include + +#include "testing/gtest/include/gtest/gtest.h" + +namespace webrtc { + +TEST(RgbaColorTest, ConvertFromAndToUInt32) { + static const std::vector cases{ + 0, 1000, 2693, 3725, 4097, 12532, + 19902, 27002, 27723, 30944, 65535, 65536, + 231194, 255985, 322871, 883798, 9585200, 12410056, + 12641940, 30496970, 105735668, 110117847, 482769275, 542368468, + 798173396, 2678656711, 3231043200, UINT32_MAX, + }; + + for (uint32_t value : cases) { + RgbaColor left(value); + ASSERT_EQ(left.ToUInt32(), value); + RgbaColor right(left); + ASSERT_EQ(left.ToUInt32(), right.ToUInt32()); + } +} + +TEST(RgbaColorTest, AlphaChannelEquality) { + RgbaColor left(10, 10, 10, 0); + RgbaColor right(10, 10, 10, 255); + ASSERT_EQ(left, right); + right.alpha = 128; + ASSERT_NE(left, right); +} + +} // namespace webrtc