Implementation of Linear Gradient #1535

msft-Jeyaram · 2016-12-14T01:43:54Z

This change is

DHowett-MSFT · 2016-12-14T01:55:44Z

Frameworks/CoreGraphics/CGGradient.mm

@@ -36,7 +36,7 @@ - (void)dealloc {
 @end

 __CGGradient::__CGGradient() : _components(NULL), _locations(NULL) {


Switch to CoreFoundation::CppBase? 😄 #WontFix

#1541 will in the future

In reply to: 92309043 [](ancestors = 92309043)

aballway · 2016-12-14T17:45:59Z

Frameworks/CoreGraphics/CGContext.mm

+* Convert CGGradient to D2D1_GRADIENT_STOP
+*/
+static inline std::vector<D2D1_GRADIENT_STOP> __CGGradientToD2D1GradientStop(CGGradientRef gradient, CGGradientDrawingOptions options) {
+    unsigned long gradientCount = _CGGradientGetCount(gradient);


super nit: no implicit int please #WontFix

aballway · 2016-12-14T17:47:08Z

Frameworks/CoreGraphics/CGContext.mm

+
+    float* colorComponenets = _CGGradientGetColorComponents(gradient);
+    float* locations = _CGGradientGetStopLocation(gradient);
+    for (unsigned long i = 0; i < gradientCount; ++i) {


super nit: same as above #WontFix

aballway · 2016-12-14T17:47:24Z

Frameworks/CoreGraphics/CGContext.mm

+
+    float* colorComponenets = _CGGradientGetColorComponents(gradient);
+    float* locations = _CGGradientGetStopLocation(gradient);
+    for (unsigned long i = 0; i < gradientCount; ++i) {


i += 4 would be clearer than multiplying below #WontFix

we need the indexing to be incremental by 1 + keeps it simple

In reply to: 92449056 [](ancestors = 92449056)

aballway · 2016-12-14T17:47:57Z

Frameworks/CoreGraphics/CGContext.mm

+
+    if (options == kCGGradientDrawsAfterEndLocation) {
+        // Set the edge to be just a bit lower on the location
+        gradientStops[0].position = 1E-45;


super magic number #WontFix

the comments explains it.
Pretty much get close to the border as possible.
so smallest delta is needed.

In reply to: 92449152 [](ancestors = 92449152)

That doesn't mean you need an arbitrary value hardcoded in. FLT_MIN? #ByDesign

sadly D2D is interpreting that as 0

In reply to: 92517563 [](ancestors = 92517563)

aballway · 2016-12-14T17:51:21Z

Frameworks/CoreGraphics/CGGradient.mm

+}
+
+unsigned long _CGGradientGetCount(CGGradientRef gradient) {
+    if (!gradient) {


nit: keep consistent with macro pattern #Resolved

aballway · 2016-12-14T17:54:13Z

Frameworks/include/CGGradientInternal.h

-#include "CoreGraphics/CGGradient.h"
-#include "CoreGraphicsInternal.h"
+#import <CoreGraphics/CGGradient.h>
+#import "CoreGraphicsInternal.h"
 #include <objc/runtime.h>

 class __CGGradient : private objc_object {


should fix this class up while you're doing this. No DWORD, CppBase, etc. #WontFix

not part of this scope.
Yep does definitely needs to be updated #1541

In reply to: 92450352 [](ancestors = 92450352)

If you're implementing CGGradient in this PR, why not just fix it now? #WontFix

i'm not implementing CGGradient (it's already implemented in a poor way).
I'm merely reading the data. It needs to be properly implemented (separate CR #1541)

In reply to: 92517645 [](ancestors = 92517645)

aballway · 2016-12-14T17:55:29Z

Frameworks/include/CGGradientInternal.h

+
+float* _CGGradientGetStopLocation(CGGradientRef gradient);
+
+float* _CGGradientGetColorComponents(CGGradientRef gradient);


don't much like returning raw float* here. I think this entire class could use some updating #ByDesign

Agreed, but again it's internal access + we can wrap it at a cost,

In reply to: 92450607 [](ancestors = 92450607)

aballway · 2016-12-14T17:55:56Z

tests/UnitTests/CoreGraphics.drawing/CGContextDrawingTests.cpp

+
+    CGFloat locations[2] = { 1.0, 0.0 };
+    CGFloat components[8] = { 0.0, 0.5, 0.0, 1.0, 1.0, 1.0, 0.8, 1.0 };
+    CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();


Need to be released #Resolved

LOL missed.

In reply to: 92450707 [](ancestors = 92450707)

DHowett-MSFT · 2016-12-14T18:48:30Z

I think it is part of the scope of this pull request! #ByDesign

msft-Jeyaram · 2016-12-14T18:51:13Z

Created a bug, it isn't blocking this PR for say.
Still tracked figured out.

In reply to: 267120339 [](ancestors = 267120339)

aballway · 2016-12-14T23:52:52Z

Frameworks/CoreGraphics/CGContext.mm

+
+    if (options == kCGGradientDrawsAfterEndLocation) {
+        // Set the edge to be just a bit lower on the location
+        gradientStops[0].position = 1E-45;


That doesn't mean you need an arbitrary value hardcoded in. FLT_MIN? #ByDesign

aballway · 2016-12-14T23:53:33Z

Frameworks/include/CGGradientInternal.h

-#include "CoreGraphics/CGGradient.h"
-#include "CoreGraphicsInternal.h"
+#import <CoreGraphics/CGGradient.h>
+#import "CoreGraphicsInternal.h"
 #include <objc/runtime.h>

 class __CGGradient : private objc_object {


If you're implementing CGGradient in this PR, why not just fix it now? #WontFix

DHowett-MSFT

Please rebase and remove tiled images.

DHowett-MSFT · 2016-12-15T01:42:13Z

Frameworks/CoreGraphics/CGContext.mm

+    D2D1_RECT_F region = D2D1::RectF(0, 0, targetSize.width, targetSize.height);
+
+    deviceContext->BeginDraw();
+    deviceContext->FillRectangle(&region, linearGradientBrush.Get());


Please put this through the rendering stack as in my comment in #1542. Otherwise, you get no clipping and no masking and no global transform. #Resolved

DHowett-MSFT · 2016-12-15T01:42:48Z

tests/UnitTests/CoreGraphics.drawing/CGContextDrawingTests.cpp

+    CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 3);
+    CGPoint startPoint = CGPointMake(250, 300);
+    CGPoint endPoint = CGPointMake(0, 0);
+    CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsAfterEndLocation);


These are a good candidate for parameterized testing! #ByDesign

going to leave it as, there are too many parameters that will keep on changing and making it will reduce readability.

In reply to: 92529089 [](ancestors = 92529089)

DHowett-MSFT · 2016-12-15T01:44:42Z

Frameworks/CoreGraphics/CGContext.mm

@@ -2050,12 +2051,20 @@ void CGContextDrawTiledImage(CGContextRef context, CGRect rect, CGImageRef image
        gradientStops[i].position = locations[i];
    }

+    if (options == kCGGradientDrawsAfterEndLocation) {


Please add support for kCGGradientDrawsBeforeStartLocation.

I don't know what this is doing, actually:
It's setting the start to be at 1E-45, and then setting the end (gradientCount) to be at .. 0?
So a gradient 0.1 0.2 0.3 would become: 1E-45 0.2 0.3 0 ??
#ByDesign

talked off line, no that would be 1e-45, 0.1, 0.2, 0.3 ...
by default kCGGradientDrawsBeforeStartLocation is supported (it's pretty much a binary option)

In reply to: 92529276 [](ancestors = 92529276)

aballway

Looks like you have test images for Tiled Images stuff in here, is that by design?

rajsesh · 2016-12-16T19:50:40Z

Frameworks/CoreGraphics/CGContext.mm

+    // This will be used to insert the transparent color.
+    std::vector<D2D1_GRADIENT_STOP> gradientStops((options == kCGGradientDrawsAfterEndLocation) ? gradientCount + 1 : gradientCount);
+
+    float* colorComponenets = _CGGradientGetColorComponents(gradient);


colorComponents. Also CGFloat* (could be different from float*) #Resolved

aballway · 2016-12-20T16:11:47Z

tests/UnitTests/CoreGraphics.drawing/CGContextDrawingTests.cpp

+    CGFloat locations[2] = { 1.0, 0.0 };
+    CGFloat components[8] = { 0.0, 0.5, 0.0, 1.0, 1.0, 1.0, 0.8, 1.0 };
+    woc::unique_cf<CGColorSpaceRef> rgbColorSpace(CGColorSpaceCreateDeviceRGB());
+    CGGradientRef gradient = CGGradientCreateWithColorComponents(rgbColorSpace.get(), components, locations, 2);


needs to be released? #Resolved

aballway · 2016-12-20T16:13:02Z

tests/UnitTests/CoreGraphics.drawing/CGContextDrawingTests.cpp

+        0.85, 0, 0, 1.0,
+    };
+
+    CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();


71-77 is almost the same for all of these. Break into helper function #WontFix

technically we are only going to be able to fully modularize 2-3 lines and we will have multitude of variables being passed in, which ends up being just like the DrawLinearGradient.

In reply to: 93269632 [](ancestors = 93269632)

aballway · 2016-12-20T16:14:21Z

Frameworks/CoreGraphics/CGContext.mm

+/*
+* Convert CGGradient to D2D1_GRADIENT_STOP
+*/
+static inline std::vector<D2D1_GRADIENT_STOP> __CGGradientToD2D1GradientStop(CGGradientRef gradient, CGGradientDrawingOptions options) {


this seems pretty hefty for an inline #Resolved

not supposed to be

In reply to: 93269884 [](ancestors = 93269884)

aballway · 2016-12-20T16:15:06Z

Frameworks/CoreGraphics/CGContext.mm

@@ -48,6 +48,8 @@

 static const wchar_t* TAG = L"CGContext";

+static float s_CGGradientStartPoint = 1E-45;


const? move comment explaining this to here #Resolved

DHowett-MSFT · 2016-12-21T03:41:06Z

Frameworks/CoreGraphics/CGContext.mm

+    // effect based on the extend mode).   We support that by inserting a point (with transparent color) close to the start/end points, such
+    // that d2d will automatically extend the transparent color, thus we obtain the desired effect for CGGradientDrawingOptions.
+
+    switch (options) {


This could be modelled as two ifs. It looks like that is the intended configuration, since the two constants are bitshifted.

if (!(options & kCGGradientDrawsAfterEndLocation)) { // insert transparent end } if (!(options & ...before start...)) { // insert transparent start }

Using a switch requires you to duplicate the stops in two different branches. #Closed

DHowett-MSFT · 2016-12-21T03:41:32Z

Frameworks/CoreGraphics/CGContext.mm

+* Insert a transparent color at the specified 'location'.
+* This will also move the color at the specified 'location' to the supplied 'position'
+*/
+static inline void __CGGradientInsertTransparentColor(std::vector<D2D1_GRADIENT_STOP>& gradientStops, int location, float position) {


This does not appear to insert them in order. Are we required to do that for D2D's sake? #ByDesign

tested it and d2d uses the position rather than the order as it seems

In reply to: 93370134 [](ancestors = 93370134)

DHowett-MSFT · 2016-12-21T03:41:55Z

Frameworks/CoreGraphics/CGContext.mm

@@ -49,6 +50,9 @@

 static const wchar_t* TAG = L"CGContext";

+// Coordinate offset to support CGGradientDrawingOptions
+static const float s_CGGradientStartPoint = 1E-45;


this is a "start point" but you are subtracting it from 1 later. That seems like a bad constant name :) #Resolved

LOL yep

In reply to: 93370154 [](ancestors = 93370154)

DHowett-MSFT · 2016-12-21T03:42:34Z

Frameworks/CoreGraphics/CGContext.mm

+    CGFloat* colorComponents = _CGGradientGetColorComponents(gradient);
+    CGFloat* locations = _CGGradientGetStopLocation(gradient);
+    for (unsigned long i = 0; i < gradientCount; ++i) {
+        unsigned int colorIndex = (i * 4);


please add a note or a todo referencing the CGColorSpace issue; this hardcoded *4 will bite somebody otherwise. #Resolved

DHowett-MSFT · 2016-12-21T03:44:23Z

tests/UnitTests/CoreGraphics.drawing/CGContextDrawingTests.cpp

@@ -589,3 +589,181 @@ DRAW_TEST(CGContext, PremultipliedAlphaImage) {
    CGContextSetRGBFillColor(context, 1.0, 0.0, 0.0, 0.5);
    CGContextFillRect(context, { 0, 0, 100, 100 });
 }
+
+#pragma region LinearGradient


Can you move these to another drawing test file named similarly to the others? CGContext_GradientDrawingTests.cpp or something similar?

Our hygiene is subpar here, we can improve incrementally. #Resolved

DHowett-MSFT · 2016-12-21T03:44:50Z

tests/UnitTests/CoreGraphics.drawing/CGContextDrawingTests.cpp

+    _drawShortLinearGradientWithOptions(GetDrawingContext(), GetDrawingBounds(), 0);
+}
+
+DRAW_TEST_F(CGContext, LinearGradientShortBothSides_Options_kCGGradientDrawsBeforeStartLocation, UIKitMimicTest) {


Each of these tests that resolves to a single function is better modelled as a parameterized test. #WontFix

DHowett-MSFT

🔢

aballway · 2016-12-21T16:38:02Z

Frameworks/CoreGraphics/CGContext.mm

+    D2D1_GRADIENT_STOP transparent;
+    transparent.color = D2D1::ColorF(0, 0, 0, 0);
+    transparent.position = location;
+    gradientStops.push_back(transparent);


nit: just emplace_back{colorf..., location} #WontFix

aballway · 2016-12-21T16:41:16Z

Frameworks/CoreGraphics/CGContext.mm

+    }
+
+    if (!(options & kCGGradientDrawsAfterEndLocation)) {
+        __CGGradientInsertTransparentColor(gradientStops, 1, 1 - s_CGGradientOffsetPoint);


1.0 - #Resolved

aballway · 2016-12-21T16:41:30Z

Frameworks/CoreGraphics/CGContext.mm

@@ -49,6 +50,9 @@

 static const wchar_t* TAG = L"CGContext";

+// Coordinate offset to support CGGradientDrawingOptions
+static const float s_CGGradientOffsetPoint = 1E-45;


should be sc_ #WontFix

Hmm s_k* is more appropriate #Resolved

aballway · 2016-12-21T16:45:38Z

Frameworks/CoreGraphics/CGGradient.mm

@@ -36,7 +36,7 @@ - (void)dealloc {
 @end


nit: update copyright #Resolved

aballway · 2016-12-21T16:47:31Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+        0.85, 0, 0, 1.0,
+    };
+
+    CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();


nit: use unique_cf for tests
c'mon I've gotten several of this exact nit from you :P #WontFix

haha I think these are simpler for now :)

In reply to: 93475189 [](ancestors = 93475189)

aballway · 2016-12-21T16:48:53Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+        0.85, 0, 0, 1.0, 1, 0, 0, 1.0, 0.85, 0.3, 0, 1.0, 0.1, 0, 0.9, 1.0,
+    };
+
+    CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();


nearly identical above, make into helper function #WontFix

the helpers would end up being a plain convoluted wrappers around CGContextDrawLinearGradient

In reply to: 93475425 [](ancestors = 93475425)

aballway · 2016-12-21T16:51:44Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+    };
+
+    CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();
+    CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 0);


nit: use std::extent to keep track of array length rather than hardcoding for all of these #WontFix

aballway · 2016-12-21T18:10:31Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+static void _drawLinearGradient(CGContextRef context,
+                                CGPoint startPoint,
+                                CGPoint endPoint,
+                                CGFloat* components,


pass this in as an array rather than a pointer, and you can just use std::extent on it rather than passing in count #Resolved

aballway · 2016-12-21T18:11:40Z

Frameworks/CoreGraphics/CGContext.mm

@@ -51,7 +51,7 @@
 static const wchar_t* TAG = L"CGContext";

 // Coordinate offset to support CGGradientDrawingOptions
-static const float s_CGGradientOffsetPoint = 1E-45;
+static const float s_kCGGradientOffsetPoint = 1E-45;


that works too XD #ByDesign

aballway · 2016-12-21T18:14:18Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+                        CGPointMake(borderRect.size.width, borderRect.size.height),
+                        components,
+                        locations,
+                        _countof(locations),


use extent instead of _countof which is MS only and shouldn't work on reference platform. #Resolved

darn i forgot about that #Resolved

msft-Jeyaram · 2016-12-21T18:33:25Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+                                CGPointMake(512, 1024),
+                                kCGGradientDrawsBeforeStartLocation);
+    CFRelease(colorspace);
+    CFRelease(gradient);


this is going to stay likes this. given it's an odd case and i'd rather not generalize the function more for an odd case. #ByDesign

aballway

Should add some tests to verify everything with odd locations rather than them all bumping against the edges of the image. Since you'll be changing this up in #1541 it'd be nice to make sure no edge cases regress.

aballway · 2016-12-21T18:47:14Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+                        kCGGradientDrawsBeforeStartLocation);
+}
+
+DISABLED_DRAW_TEST_F(CGGradient, LinearGradientShortBothSides_Options_0, UIKitMimicTest) {


These are begging to be parameterized #WontFix

these are simple enough for now

In reply to: 93496267 [](ancestors = 93496267)

aballway · 2016-12-21T18:49:15Z

tests/unittests/CoreGraphics.drawing/CGContextDrawing_GradientTests.cpp

+    CFRelease(gradient);
+}
+
+DISABLED_DRAW_TEST_F(CGGradient, LinearGradient2, UIKitMimicTest) {


Should make these paremeterized too, combinations of locations/components, different points, changes in alpha, etc. #WontFix

uh it's going to be a huge wrapped convoluted function, going to lock it down on this.

In reply to: 93496632 [](ancestors = 93496632)

msftclas added the cla-not-required label Dec 14, 2016

msft-Jeyaram changed the title ~~Linear gradient push~~ Implementation of Linear Gradient Dec 14, 2016

DHowett-MSFT reviewed Dec 14, 2016

View reviewed changes

msft-Jeyaram force-pushed the LinearGradient_Push branch from cb1baad to 9a99f65 Compare December 14, 2016 02:22

aballway suggested changes Dec 14, 2016

View reviewed changes

DHowett-MSFT suggested changes Dec 15, 2016

View reviewed changes

msft-Jeyaram force-pushed the LinearGradient_Push branch from a879990 to c1f17f5 Compare December 15, 2016 04:10

aballway reviewed Dec 16, 2016

View reviewed changes

rajsesh approved these changes Dec 16, 2016

View reviewed changes

aballway suggested changes Dec 20, 2016

View reviewed changes

Implementation of Linear gradients with full optional support

613b276

msft-Jeyaram force-pushed the LinearGradient_Push branch from 56b7a3e to 613b276 Compare December 21, 2016 02:26

DHowett-MSFT reviewed Dec 21, 2016

View reviewed changes

DHowett-MSFT suggested changes Dec 21, 2016

View reviewed changes

cr feedback and moving tests to file

bfdf479

aballway reviewed Dec 21, 2016

View reviewed changes

some nit picks

01b435e

aballway reviewed Dec 21, 2016

View reviewed changes

more nits

b3df981

msft-Jeyaram commented Dec 21, 2016

View reviewed changes

disabling tests

28a0a8c

aballway reviewed Dec 21, 2016

View reviewed changes

msft-Jeyaram merged commit 0c1e282 into microsoft:CGD2D Dec 21, 2016

		@@ -36,7 +36,7 @@ - (void)dealloc {
		@end

		__CGGradient::__CGGradient() : _components(NULL), _locations(NULL) {


		float* _CGGradientGetStopLocation(CGGradientRef gradient);

		float* _CGGradientGetColorComponents(CGGradientRef gradient);

		@@ -48,6 +48,8 @@

		static const wchar_t* TAG = L"CGContext";

		static float s_CGGradientStartPoint = 1E-45;

Implementation of Linear Gradient #1535

Implementation of Linear Gradient #1535

Conversation

msft-Jeyaram commented Dec 14, 2016 • edited by rajsesh Loading

DHowett-MSFT Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

msft-Jeyaram Dec 14, 2016 • edited Loading

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett-MSFT commented Dec 14, 2016 via email • edited by msft-Jeyaram Loading

msft-Jeyaram commented Dec 14, 2016

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

aballway Dec 14, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

DHowett-MSFT left a comment

Choose a reason for hiding this comment

DHowett-MSFT Dec 15, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

DHowett-MSFT Dec 15, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett-MSFT Dec 15, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

msft-Jeyaram Dec 16, 2016 • edited Loading

Choose a reason for hiding this comment

aballway left a comment • edited Loading

Choose a reason for hiding this comment

rajsesh Dec 16, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

aballway Dec 20, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

aballway Dec 20, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 20, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aballway Dec 20, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

DHowett-MSFT Dec 21, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

DHowett-MSFT Dec 21, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett-MSFT Dec 21, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett-MSFT Dec 21, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

DHowett-MSFT Dec 21, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

DHowett-MSFT Dec 21, 2016 • edited by msft-Jeyaram Loading

Choose a reason for hiding this comment

msft-Jeyaram commented Dec 14, 2016 •

edited by rajsesh

Loading

DHowett-MSFT Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

msft-Jeyaram Dec 14, 2016 •

edited

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT commented Dec 14, 2016 via email •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 14, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 15, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 15, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 15, 2016 •

edited by msft-Jeyaram

Loading

msft-Jeyaram Dec 16, 2016 •

edited

Loading

aballway left a comment •

edited

Loading

rajsesh Dec 16, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 20, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 20, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 20, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 20, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 21, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 21, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 21, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 21, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 21, 2016 •

edited by msft-Jeyaram

Loading

DHowett-MSFT Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

msft-Jeyaram Dec 21, 2016 •

edited

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

msft-Jeyaram Dec 21, 2016 •

edited

Loading

msft-Jeyaram Dec 21, 2016 •

edited

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading

aballway Dec 21, 2016 •

edited by msft-Jeyaram

Loading