Skip to content

Commit

Permalink
[FancyZones] Validate zone rect before adding it into layout (#6249)
Browse files Browse the repository at this point in the history
* Validate zone rect before adding it into layout

* Rename variables for better code readability
  • Loading branch information
vldmr11080 committed Sep 8, 2020
1 parent c5f9027 commit 13426ea
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 54 deletions.
19 changes: 18 additions & 1 deletion src/modules/fancyzones/lib/Zone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@

#include "common/monitors.h"

namespace
{
bool ValidateZoneRect(const RECT& rect)
{
int width = rect.right - rect.left;
int height = rect.bottom - rect.top;
return rect.left >= 0 && rect.right >= 0 && rect.top >= 0 && rect.bottom >= 0 && width >= 0 && height >= 0;
}
}

struct Zone : winrt::implements<Zone, IZone>
{
public:
Expand Down Expand Up @@ -122,5 +132,12 @@ RECT Zone::ComputeActualZoneRect(HWND window, HWND zoneWindow) noexcept

winrt::com_ptr<IZone> MakeZone(const RECT& zoneRect) noexcept
{
return winrt::make_self<Zone>(zoneRect);
if (ValidateZoneRect(zoneRect))
{
return winrt::make_self<Zone>(zoneRect);
}
else
{
return nullptr;
}
}
82 changes: 48 additions & 34 deletions src/modules/fancyzones/lib/ZoneSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,6 @@ bool ZoneSet::IsZoneEmpty(int zoneIndex) noexcept

bool ZoneSet::CalculateFocusLayout(Rect workArea, int zoneCount) noexcept
{
bool success = true;

long left{ 100 };
long top{ 100 };
long right{ left + long(workArea.width() * 0.4) };
Expand All @@ -496,27 +494,30 @@ bool ZoneSet::CalculateFocusLayout(Rect workArea, int zoneCount) noexcept
long focusRectXIncrement = (zoneCount <= 1) ? 0 : 50;
long focusRectYIncrement = (zoneCount <= 1) ? 0 : 50;

if (left >= right || top >= bottom || left < 0 || right < 0 || top < 0 || bottom < 0)
{
success = false;
}

for (int i = 0; i < zoneCount; i++)
{
AddZone(MakeZone(focusZoneRect));
auto zone = MakeZone(focusZoneRect);
if (zone)
{
AddZone(zone);
}
else
{
// All zones within zone set should be valid in order to use its functionality.
m_zones.clear();
return false;
}
focusZoneRect.left += focusRectXIncrement;
focusZoneRect.right += focusRectXIncrement;
focusZoneRect.bottom += focusRectYIncrement;
focusZoneRect.top += focusRectYIncrement;
}

return success;
return true;
}

bool ZoneSet::CalculateColumnsAndRowsLayout(Rect workArea, FancyZonesDataTypes::ZoneSetLayoutType type, int zoneCount, int spacing) noexcept
{
bool success = true;

long totalWidth;
long totalHeight;

Expand All @@ -538,26 +539,31 @@ bool ZoneSet::CalculateColumnsAndRowsLayout(Rect workArea, FancyZonesDataTypes::

// Note: The expressions below are NOT equal to total{Width|Height} / zoneCount and are done
// like this to make the sum of all zones' sizes exactly total{Width|Height}.
for (int zone = 0; zone < zoneCount; zone++)
for (int zoneIndex = 0; zoneIndex < zoneCount; ++zoneIndex)
{
if (type == FancyZonesDataTypes::ZoneSetLayoutType::Columns)
{
right = left + (zone + 1) * totalWidth / zoneCount - zone * totalWidth / zoneCount;
right = left + (zoneIndex + 1) * totalWidth / zoneCount - zoneIndex * totalWidth / zoneCount;
bottom = totalHeight + spacing;
}
else
{ //Rows
right = totalWidth + spacing;
bottom = top + (zone + 1) * totalHeight / zoneCount - zone * totalHeight / zoneCount;
bottom = top + (zoneIndex + 1) * totalHeight / zoneCount - zoneIndex * totalHeight / zoneCount;
}

if (left >= right || top >= bottom || left < 0 || right < 0 || top < 0 || bottom < 0)


auto zone = MakeZone(RECT{ left, top, right, bottom });
if (zone)
{
success = false;
AddZone(zone);
}
else
{
// All zones within zone set should be valid in order to use its functionality.
m_zones.clear();
return false;
}

RECT focusZoneRect{ left, top, right, bottom };
AddZone(MakeZone(focusZoneRect));

if (type == FancyZonesDataTypes::ZoneSetLayoutType::Columns)
{
Expand All @@ -569,7 +575,7 @@ bool ZoneSet::CalculateColumnsAndRowsLayout(Rect workArea, FancyZonesDataTypes::
}
}

return success;
return true;
}

bool ZoneSet::CalculateGridLayout(Rect workArea, FancyZonesDataTypes::ZoneSetLayoutType type, int zoneCount, int spacing) noexcept
Expand Down Expand Up @@ -664,15 +670,20 @@ bool ZoneSet::CalculateCustomLayout(Rect workArea, int spacing) noexcept
int width = zone.width;
int height = zone.height;

if (x < 0 || y < 0 || width < 0 || height < 0)
{
return false;
}

DPIAware::Convert(m_config.Monitor, x, y);
DPIAware::Convert(m_config.Monitor, width, height);

AddZone(MakeZone(RECT{ x, y, x + width, y + height }));
auto zone = MakeZone(RECT{ x, y, x + width, y + height });
if (zone)
{
AddZone(zone);
}
else
{
// All zones within zone set should be valid in order to use its functionality.
m_zones.clear();
return false;
}
}

return true;
Expand All @@ -689,8 +700,6 @@ bool ZoneSet::CalculateCustomLayout(Rect workArea, int spacing) noexcept

bool ZoneSet::CalculateGridZones(Rect workArea, FancyZonesDataTypes::GridLayoutInfo gridLayoutInfo, int spacing)
{
bool success = true;

long totalWidth = workArea.width() - (spacing * (gridLayoutInfo.columns() + 1));
long totalHeight = workArea.height() - (spacing * (gridLayoutInfo.rows() + 1));
struct Info
Expand Down Expand Up @@ -747,17 +756,22 @@ bool ZoneSet::CalculateGridZones(Rect workArea, FancyZonesDataTypes::GridLayoutI
long right = columnInfo[maxCol].End;
long bottom = rowInfo[maxRow].End;

if (left >= right || top >= bottom || left < 0 || right < 0 || top < 0 || bottom < 0)
auto zone = MakeZone(RECT{ left, top, right, bottom });
if (zone)
{
success = false;
AddZone(zone);
}
else
{
// All zones within zone set should be valid in order to use its functionality.
m_zones.clear();
return false;
}

AddZone(MakeZone(RECT{ left, top, right, bottom }));
}
}
}

return success;
return true;
}

void ZoneSet::StampWindow(HWND window, size_t bitmask) noexcept
Expand Down
52 changes: 33 additions & 19 deletions src/modules/fancyzones/tests/UnitTests/ZoneSet.Spec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ namespace FancyZonesUnitTests
TEST_METHOD (AddOne)
{
winrt::com_ptr<IZone> zone = MakeZone({ 0, 0, 100, 100 });
Assert::IsNotNull(zone.get());
m_set->AddZone(zone);
auto zones = m_set->GetZones();
Assert::AreEqual((size_t)1, zones.size());
Expand All @@ -95,6 +96,7 @@ namespace FancyZonesUnitTests
TEST_METHOD (AddManySame)
{
winrt::com_ptr<IZone> zone = MakeZone({ 0, 0, 100, 100 });
Assert::IsNotNull(zone.get());
for (size_t i = 0; i < 1024; i++)
{
m_set->AddZone(zone);
Expand All @@ -110,6 +112,7 @@ namespace FancyZonesUnitTests
for (size_t i = 0; i < 1024; i++)
{
winrt::com_ptr<IZone> zone = MakeZone({ 0, 0, 100, 100 });
Assert::IsNotNull(zone.get());
m_set->AddZone(zone);
auto zones = m_set->GetZones();
Assert::AreEqual(i + 1, zones.size());
Expand All @@ -122,7 +125,12 @@ namespace FancyZonesUnitTests
{
for (size_t i = 0; i < 1024; i++)
{
winrt::com_ptr<IZone> zone = MakeZone({ rand() % 10, rand() % 10, rand() % 100, rand() % 100 });
int left = rand() % 10;
int top = rand() % 10;
int right = left + 1 + rand() % 100;
int bottom = top + 1 + rand() % 100;
winrt::com_ptr<IZone> zone = MakeZone({ left, top, right, bottom });
Assert::IsNotNull(zone.get());
m_set->AddZone(zone);
auto zones = m_set->GetZones();
Assert::AreEqual(i + 1, zones.size());
Expand All @@ -131,6 +139,30 @@ namespace FancyZonesUnitTests
}
}

TEST_METHOD (MakeZoneFromZeroRect)
{
winrt::com_ptr<IZone> zone = MakeZone({ 0, 0, 0, 0 });
Assert::IsNotNull(zone.get());
}

TEST_METHOD (MakeZoneFromInvalidRectWidth)
{
winrt::com_ptr<IZone> zone = MakeZone({ 100, 100, 99, 101 });
Assert::IsNull(zone.get());
}

TEST_METHOD (MakeZoneFromInvalidRectHeight)
{
winrt::com_ptr<IZone> zone = MakeZone({ 100, 100, 101, 99 });
Assert::IsNull(zone.get());
}

TEST_METHOD (MakeZoneFromInvalidRectCoords)
{
winrt::com_ptr<IZone> zone = MakeZone({ -1, -1, -1, -1 });
Assert::IsNull(zone.get());
}

TEST_METHOD (ZoneFromPointEmpty)
{
auto actual = m_set->ZonesFromPoint(POINT{ 0, 0 });
Expand Down Expand Up @@ -270,24 +302,6 @@ namespace FancyZonesUnitTests
compareZones(zone4, m_set->GetZones()[actual[3]]);
}

TEST_METHOD (ZoneFromPointWithNotNormalizedRect)
{
winrt::com_ptr<IZone> zone = MakeZone({ 100, 100, 0, 0 });
m_set->AddZone(zone);

auto actual = m_set->ZonesFromPoint(POINT{ 50, 50 });
Assert::IsTrue(actual.size() == 0);
}

TEST_METHOD (ZoneFromPointWithZeroRect)
{
winrt::com_ptr<IZone> zone = MakeZone({ 0, 0, 0, 0 });
m_set->AddZone(zone);

auto actual = m_set->ZonesFromPoint(POINT{ 0, 0 });
Assert::IsTrue(actual.size() == 0);
}

TEST_METHOD (ZoneIndexFromWindowUnknown)
{
winrt::com_ptr<IZone> zone = MakeZone({ 0, 0, 100, 100 });
Expand Down

0 comments on commit 13426ea

Please sign in to comment.