Skip to content

Commit

Permalink
Fix regions generation on windows.
Browse files Browse the repository at this point in the history
On windows HRGN for window is generated from the SkPath using dumb
function that doesn`t respects anything except simple polygons. It's
impossible to create window with round corners on windows. Replacing
implementation of that function with a new one will fix this.

R=asvitkine@chromium.org

BUG=322360

Review URL: https://codereview.chromium.org/673623002

Cr-Commit-Position: refs/heads/master@{#304045}
  • Loading branch information
alex-ac authored and Commit bot committed Nov 13, 2014
1 parent a34e8b7 commit 2cb37ad
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 26 deletions.
12 changes: 8 additions & 4 deletions chrome/browser/ui/views/panels/panel_frame_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,14 @@ void PanelFrameView::SetWindowCornerStyle(panel::CornerStyle corner_style) {

gfx::Path window_mask;
GetWindowMask(size(), &window_mask);
base::win::ScopedRegion new_region(gfx::CreateHRGNFromSkPath(window_mask));

if (current_region_result == ERROR ||
!::EqualRgn(current_region, new_region)) {
base::win::ScopedRegion new_region;
if (!window_mask.isEmpty())
new_region.Set(gfx::CreateHRGNFromSkPath(window_mask));

const bool has_current_region = current_region != NULL;
const bool has_new_region = new_region != NULL;
if (has_current_region != has_new_region ||
(has_current_region && !::EqualRgn(current_region, new_region))) {
// SetWindowRgn takes ownership of the new_region.
::SetWindowRgn(native_window, new_region.release(), TRUE);
}
Expand Down
1 change: 1 addition & 0 deletions ui/gfx/gfx_tests.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
'font_fallback_win_unittest.cc',
'icon_util_unittest.cc',
'icon_util_unittests.rc',
'path_win_unittest.cc',
'platform_font_win_unittest.cc',
],
'msvs_settings': {
Expand Down
15 changes: 5 additions & 10 deletions ui/gfx/path_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,11 @@ HRGN CreateHRGNFromSkRegion(const SkRegion& region) {
}

HRGN CreateHRGNFromSkPath(const SkPath& path) {
int point_count = path.getPoints(NULL, 0);
scoped_ptr<SkPoint[]> points(new SkPoint[point_count]);
path.getPoints(points.get(), point_count);
scoped_ptr<POINT[]> windows_points(new POINT[point_count]);
for (int i = 0; i < point_count; ++i) {
windows_points[i].x = SkScalarRoundToInt(points[i].fX);
windows_points[i].y = SkScalarRoundToInt(points[i].fY);
}

return ::CreatePolygonRgn(windows_points.get(), point_count, ALTERNATE);
SkRegion clip_region;
clip_region.setRect(path.getBounds().round());
SkRegion region;
region.setPath(path, clip_region);
return CreateHRGNFromSkRegion(region);
}

// See path_aura.cc for Aura definition of these methods:
Expand Down
2 changes: 1 addition & 1 deletion ui/gfx/path_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace gfx {
GFX_EXPORT HRGN CreateHRGNFromSkRegion(const SkRegion& path);

// Creates a new HRGN given |path|. The caller is responsible for destroying
// the returned region.
// the returned region. Returns empty region (not NULL) for empty path.
GFX_EXPORT HRGN CreateHRGNFromSkPath(const SkPath& path);

} // namespace gfx
Expand Down
111 changes: 111 additions & 0 deletions ui/gfx/path_win_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright 2014 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.

#include "ui/gfx/path_win.h"

#include <algorithm>
#include <vector>

#include "base/win/scoped_gdi_object.h"
#include "skia/ext/skia_utils_win.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkRRect.h"
#include "ui/gfx/path.h"

namespace gfx {

namespace {

// Get rectangles from the |region| and convert them to SkIRect.
std::vector<SkIRect> GetRectsFromHRGN(HRGN region) {
// Determine the size of output buffer required to receive the region.
DWORD bytes_size = GetRegionData(region, 0, NULL);
CHECK_NE((DWORD)0, bytes_size);

// Fetch the Windows RECTs that comprise the region.
std::vector<char> buffer(bytes_size);
LPRGNDATA region_data = reinterpret_cast<LPRGNDATA>(buffer.data());
DWORD result = GetRegionData(region, bytes_size, region_data);
CHECK_EQ(bytes_size, result);

// Pull out the rectangles into a SkIRect vector to return to caller.
const LPRECT rects = reinterpret_cast<LPRECT>(&region_data->Buffer[0]);
std::vector<SkIRect> sk_rects(region_data->rdh.nCount);
std::transform(rects, rects + region_data->rdh.nCount,
sk_rects.begin(), skia::RECTToSkIRect);

return sk_rects;
}

} // namespace

// Test that rectangle with round corners stil has round corners after
// converting from SkPath to the HRGN.
TEST(CreateHRGNFromSkPathTest, RoundCornerTest) {
const SkIRect rects[] = {
{ 17, 0, 33, 1},
{ 12, 1, 38, 2},
{ 11, 2, 39, 3},
{ 9, 3, 41, 4},
{ 8, 4, 42, 5},
{ 6, 5, 44, 6},
{ 5, 6, 45, 8},
{ 4, 8, 46, 9},
{ 3, 9, 47, 11},
{ 2, 11, 48, 12},
{ 1, 12, 49, 17},
{ 0, 17, 50, 33},
{ 1, 33, 49, 38},
{ 2, 38, 48, 39},
{ 3, 39, 47, 41},
{ 4, 41, 46, 42},
{ 5, 42, 45, 44},
{ 6, 44, 44, 45},
{ 8, 45, 42, 46},
{ 9, 46, 41, 47},
{ 11, 47, 39, 48},
{ 12, 48, 38, 49},
{ 17, 49, 33, 50},
};

Path path;
SkRRect rrect;
rrect.setRectXY(SkRect::MakeWH(50, 50), 20, 20);
path.addRRect(rrect);
base::win::ScopedRegion region(CreateHRGNFromSkPath(path));
const std::vector<SkIRect>& region_rects = GetRectsFromHRGN(region);
EXPECT_EQ(arraysize(rects), region_rects.size());
for (size_t i = 0; i < arraysize(rects) && i < region_rects.size(); ++i)
EXPECT_EQ(rects[i], region_rects[i]);
}

// Check that a path enclosing two non-adjacent areas is correctly translated
// to a non-contiguous region.
TEST(CreateHRGNFromSkPathTest, NonContiguousPath) {
const SkIRect rects[] = {
{ 0, 0, 50, 50},
{ 100, 100, 150, 150},
};

Path path;
for (const SkIRect& rect : rects) {
path.addRect(SkRect::Make(rect));
}
base::win::ScopedRegion region(CreateHRGNFromSkPath(path));
const std::vector<SkIRect>& region_rects = GetRectsFromHRGN(region);
ASSERT_EQ(arraysize(rects), region_rects.size());
for (size_t i = 0; i < arraysize(rects); ++i)
EXPECT_EQ(rects[i], region_rects[i]);
}

// Check that empty region is returned for empty path.
TEST(CreateHRGNFromSkPathTest, EmptyPath) {
Path path;
base::win::ScopedRegion empty_region(::CreateRectRgn(0, 0, 0, 0));
base::win::ScopedRegion region(CreateHRGNFromSkPath(path));
EXPECT_TRUE(::EqualRgn(empty_region, region));
}

} // namespace gfx

23 changes: 12 additions & 11 deletions ui/views/win/hwnd_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "base/bind.h"
#include "base/debug/trace_event.h"
#include "base/win/scoped_gdi_object.h"
#include "base/win/win_util.h"
#include "base/win/windows_version.h"
#include "ui/base/touch/touch_enabled.h"
Expand Down Expand Up @@ -1132,14 +1133,14 @@ void HWNDMessageHandler::ResetWindowRegion(bool force, bool redraw) {

// Changing the window region is going to force a paint. Only change the
// window region if the region really differs.
HRGN current_rgn = CreateRectRgn(0, 0, 0, 0);
base::win::ScopedRegion current_rgn(CreateRectRgn(0, 0, 0, 0));
int current_rgn_result = GetWindowRgn(hwnd(), current_rgn);

RECT window_rect;
GetWindowRect(hwnd(), &window_rect);
HRGN new_region;
base::win::ScopedRegion new_region;
if (custom_window_region_) {
new_region = ::CreateRectRgn(0, 0, 0, 0);
new_region.Set(::CreateRectRgn(0, 0, 0, 0));
::CombineRgn(new_region, custom_window_region_.Get(), NULL, RGN_COPY);
} else if (IsMaximized()) {
HMONITOR monitor = MonitorFromWindow(hwnd(), MONITOR_DEFAULTTONEAREST);
Expand All @@ -1148,23 +1149,23 @@ void HWNDMessageHandler::ResetWindowRegion(bool force, bool redraw) {
GetMonitorInfo(monitor, &mi);
RECT work_rect = mi.rcWork;
OffsetRect(&work_rect, -window_rect.left, -window_rect.top);
new_region = CreateRectRgnIndirect(&work_rect);
new_region.Set(CreateRectRgnIndirect(&work_rect));
} else {
gfx::Path window_mask;
delegate_->GetWindowMask(gfx::Size(window_rect.right - window_rect.left,
window_rect.bottom - window_rect.top),
&window_mask);
new_region = gfx::CreateHRGNFromSkPath(window_mask);
if (!window_mask.isEmpty())
new_region.Set(gfx::CreateHRGNFromSkPath(window_mask));
}

if (current_rgn_result == ERROR || !EqualRgn(current_rgn, new_region)) {
const bool has_current_region = current_rgn != 0;
const bool has_new_region = new_region != 0;
if (has_current_region != has_new_region ||
(has_current_region && !EqualRgn(current_rgn, new_region))) {
// SetWindowRgn takes ownership of the HRGN created by CreateNativeRegion.
SetWindowRgn(hwnd(), new_region, redraw);
} else {
DeleteObject(new_region);
SetWindowRgn(hwnd(), new_region.release(), redraw);
}

DeleteObject(current_rgn);
}

void HWNDMessageHandler::UpdateDwmNcRenderingPolicy() {
Expand Down

0 comments on commit 2cb37ad

Please sign in to comment.