-
Notifications
You must be signed in to change notification settings - Fork 805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementation of Linear Gradient #1535
Changes from 2 commits
613b276
bfdf479
01b435e
b3df981
28a0a8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ | |
#import "CGPathInternal.h" | ||
#import "CGIWICBitmap.h" | ||
#import "CGPatternInternal.h" | ||
#import "CGGradientInternal.h" | ||
|
||
#import <CFCppBase.h> | ||
|
||
|
@@ -49,6 +50,9 @@ | |
|
||
static const wchar_t* TAG = L"CGContext"; | ||
|
||
// Coordinate offset to support CGGradientDrawingOptions | ||
static const float s_CGGradientOffsetPoint = 1E-45; | ||
|
||
enum _CGCoordinateMode : unsigned int { _kCGCoordinateModeDeviceSpace = 0, _kCGCoordinateModeUserSpace }; | ||
|
||
// A drawing context is represented by a number of layers, each with their own drawing state: | ||
|
@@ -2356,14 +2360,96 @@ void CGContextDrawTiledImage(CGContextRef context, CGRect rect, CGImageRef image | |
|
||
#pragma region Drawing Operations - Gradient + Shading | ||
/** | ||
@Status Stub | ||
* 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not appear to insert them in order. Are we required to do that for D2D's sake? #ByDesign There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tested it and d2d uses the position rather than the order as it seems In reply to: 93370134 [](ancestors = 93370134) |
||
gradientStops[location].position = position; | ||
// set the edge location to be transparent | ||
D2D1_GRADIENT_STOP transparent; | ||
transparent.color = D2D1::ColorF(0, 0, 0, 0); | ||
transparent.position = location; | ||
gradientStops.push_back(transparent); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: just emplace_back{colorf..., location} #WontFix |
||
} | ||
|
||
/* | ||
* Convert CGGradient to D2D1_GRADIENT_STOP | ||
*/ | ||
static std::vector<D2D1_GRADIENT_STOP> __CGGradientToD2D1GradientStop(CGContextRef context, | ||
CGGradientRef gradient, | ||
CGGradientDrawingOptions options) { | ||
unsigned long gradientCount = _CGGradientGetCount(gradient); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. super nit: no implicit int please #WontFix |
||
std::vector<D2D1_GRADIENT_STOP> gradientStops(gradientCount); | ||
|
||
CGFloat* colorComponents = _CGGradientGetColorComponents(gradient); | ||
CGFloat* locations = _CGGradientGetStopLocation(gradient); | ||
for (unsigned long i = 0; i < gradientCount; ++i) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. super nit: same as above #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i += 4 would be clearer than multiplying below #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need the indexing to be incremental by 1 + keeps it simple In reply to: 92449056 [](ancestors = 92449056) |
||
// TODO #1541: The indexing needs to get updated based on colorspace (for non RGBA) | ||
unsigned int colorIndex = (i * 4); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add a note or a todo referencing the CGColorSpace issue; this hardcoded |
||
gradientStops[i].color = D2D1::ColorF(colorComponents[colorIndex], | ||
colorComponents[colorIndex + 1], | ||
colorComponents[colorIndex + 2], | ||
colorComponents[colorIndex + 3]); | ||
gradientStops[i].position = locations[i]; | ||
} | ||
|
||
// we want to support CGGradientDrawingOptions, but by default d2d will extend the region via repeating the brush (or other | ||
// 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. | ||
|
||
if (!(options & kCGGradientDrawsBeforeStartLocation)) { | ||
__CGGradientInsertTransparentColor(gradientStops, 0, s_CGGradientOffsetPoint); | ||
} | ||
|
||
if (!(options & kCGGradientDrawsAfterEndLocation)) { | ||
__CGGradientInsertTransparentColor(gradientStops, 1, 1 - s_CGGradientOffsetPoint); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1.0 - #Resolved |
||
} | ||
|
||
return gradientStops; | ||
} | ||
/** | ||
@Status Interoperable | ||
*/ | ||
void CGContextDrawLinearGradient( | ||
CGContextRef context, CGGradientRef gradient, CGPoint startPoint, CGPoint endPoint, CGGradientDrawingOptions options) { | ||
NOISY_RETURN_IF_NULL(context); | ||
NOISY_RETURN_IF_NULL(gradient); | ||
RETURN_IF(!context->ShouldDraw()); | ||
|
||
UNIMPLEMENTED(); | ||
RETURN_IF(_CGGradientGetCount(gradient) == 0); | ||
|
||
std::vector<D2D1_GRADIENT_STOP> gradientStops = __CGGradientToD2D1GradientStop(context, gradient, options); | ||
|
||
ComPtr<ID2D1GradientStopCollection> gradientStopCollection; | ||
|
||
ComPtr<ID2D1DeviceContext> deviceContext = context->DeviceContext(); | ||
FAIL_FAST_IF_FAILED(deviceContext->CreateGradientStopCollection(gradientStops.data(), | ||
gradientStops.size(), | ||
D2D1_GAMMA_2_2, | ||
D2D1_EXTEND_MODE_CLAMP, | ||
&gradientStopCollection)); | ||
|
||
ComPtr<ID2D1LinearGradientBrush> linearGradientBrush; | ||
FAIL_FAST_IF_FAILED(deviceContext->CreateLinearGradientBrush( | ||
D2D1::LinearGradientBrushProperties(_CGPointToD2D_F(startPoint), _CGPointToD2D_F(endPoint)), | ||
D2D1::BrushProperties(context->CurrentGState().alpha, | ||
__CGAffineTransformToD2D_F(CGContextGetUserSpaceToDeviceSpaceTransform(context))), | ||
gradientStopCollection.Get(), | ||
&linearGradientBrush)); | ||
|
||
// Area to fill | ||
D2D1_SIZE_F targetSize = deviceContext->GetSize(); | ||
D2D1_RECT_F region = D2D1::RectF(0, 0, targetSize.width, targetSize.height); | ||
|
||
ComPtr<ID2D1CommandList> commandList; | ||
FAIL_FAST_IF_FAILED(context->DrawToCommandList(_kCGCoordinateModeDeviceSpace, | ||
nullptr, | ||
&commandList, | ||
[&](CGContextRef context, ID2D1DeviceContext* deviceContext) { | ||
deviceContext->FillRectangle(®ion, linearGradientBrush.Get()); | ||
return S_OK; | ||
})); | ||
FAIL_FAST_IF_FAILED(context->DrawImage(commandList.Get())); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ - (void)dealloc { | |
@end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: update copyright #Resolved |
||
|
||
__CGGradient::__CGGradient() : _components(NULL), _locations(NULL) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switch to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
object_setClass((id) this, [CGNSGradient class]); | ||
object_setClass((id)this, [CGNSGradient class]); | ||
} | ||
|
||
__CGGradient::~__CGGradient() { | ||
|
@@ -181,3 +181,23 @@ CFTypeID CGGradientGetTypeID() { | |
UNIMPLEMENTED(); | ||
return StubReturn(); | ||
} | ||
|
||
CGFloat* _CGGradientGetStopLocation(CGGradientRef gradient) { | ||
RETURN_NULL_IF(!gradient); | ||
return gradient->_locations; | ||
} | ||
|
||
CGFloat* _CGGradientGetColorComponents(CGGradientRef gradient) { | ||
RETURN_NULL_IF(!gradient); | ||
return gradient->_components; | ||
} | ||
|
||
unsigned long _CGGradientGetCount(CGGradientRef gradient) { | ||
RETURN_RESULT_IF_NULL(gradient, 0); | ||
return gradient->_count; | ||
} | ||
|
||
CGColorSpaceModel _CGGradientGetColorSpaceModel(CGGradientRef gradient) { | ||
RETURN_RESULT_IF_NULL(gradient, kCGColorSpaceModelRGB); | ||
return gradient->_colorSpaceModel; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
//****************************************************************************** | ||
// | ||
// Copyright (c) 2016 Intel Corporation. All rights reserved. | ||
// Copyright (c) 2015 Microsoft Corporation. All rights reserved. | ||
// Copyright (c) Microsoft. All rights reserved. | ||
// | ||
// This code is licensed under the MIT License (MIT). | ||
// | ||
|
@@ -17,8 +17,8 @@ | |
|
||
#pragma once | ||
|
||
#include "CoreGraphics/CGGradient.h" | ||
#include "CoreGraphicsInternal.h" | ||
#import <CoreGraphics/CGGradient.h> | ||
#import "CoreGraphicsInternal.h" | ||
#include <objc/runtime.h> | ||
|
||
class __CGGradient : private objc_object { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should fix this class up while you're doing this. No DWORD, CppBase, etc. #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're implementing CGGradient in this PR, why not just fix it now? #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
@@ -34,3 +34,11 @@ class __CGGradient : private objc_object { | |
void initWithColorComponents(const float* components, const float* locations, size_t count, CGColorSpaceRef colorspace); | ||
void initWithColors(CFArrayRef components, const float* locations, CGColorSpaceRef colorspace); | ||
}; | ||
|
||
unsigned long _CGGradientGetCount(CGGradientRef gradient); | ||
|
||
CGFloat* _CGGradientGetStopLocation(CGGradientRef gradient); | ||
|
||
CGFloat* _CGGradientGetColorComponents(CGGradientRef gradient); | ||
|
||
CGColorSpaceModel _CGGradientGetColorSpaceModel(CGGradientRef gradient); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,195 @@ | ||
//****************************************************************************** | ||
// | ||
// Copyright (c) Microsoft. All rights reserved. | ||
// | ||
// This code is licensed under the MIT License (MIT). | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
// THE SOFTWARE. | ||
// | ||
//****************************************************************************** | ||
|
||
#include "DrawingTest.h" | ||
|
||
#pragma region LinearGradient | ||
DRAW_TEST_F(CGGradient, LinearGradient, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[2] = { 0, 1 }; | ||
CGFloat components[8] = { 0.0, 0.0, 1, 1.0, 1.0, 0, 0, 1.0 }; | ||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 2); | ||
|
||
CGPoint startPoint = CGPointMake(0, 0); | ||
CGPoint endPoint = CGPointMake(512, 1024); | ||
|
||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
static void _drawShortLinearGradientWithOptions(CGContextRef context, CGRect bounds, CGGradientDrawingOptions option) { | ||
CGFloat locations[2] = { 0, 1 }; | ||
CGFloat components[8] = { 0.0, 1, 0.0, 1.0, 1.0, 0, 0, 1.0 }; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 2); | ||
|
||
CGRect borderRect = CGRectInset(bounds, 30, 50); | ||
|
||
CGContextDrawLinearGradient(context, gradient, borderRect.origin, CGPointMake(borderRect.size.width, borderRect.size.height), option); | ||
|
||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientShortBothSides_Options_0, UIKitMimicTest) { | ||
_drawShortLinearGradientWithOptions(GetDrawingContext(), GetDrawingBounds(), 0); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientShortBothSides_Options_kCGGradientDrawsBeforeStartLocation, UIKitMimicTest) { | ||
_drawShortLinearGradientWithOptions(GetDrawingContext(), GetDrawingBounds(), kCGGradientDrawsBeforeStartLocation); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientShortBothSides_Options_kCGGradientDrawsAfterEndLocation, UIKitMimicTest) { | ||
_drawShortLinearGradientWithOptions(GetDrawingContext(), GetDrawingBounds(), kCGGradientDrawsAfterEndLocation); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientInvalidCount, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0 }; | ||
|
||
CGFloat components[] = { | ||
0.85, 0, 0, 1.0, | ||
}; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use unique_cf for tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use std::extent to keep track of array length rather than hardcoding for all of these #WontFix |
||
CGPoint startPoint = CGPointMake(0, 0); | ||
CGPoint endPoint = CGPointMake(512, 1024); | ||
|
||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradient2, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0, 0.33, 0.66, 1.0 }; | ||
|
||
CGFloat components[] = { | ||
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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nearly identical above, make into helper function #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the helpers would end up being a plain convoluted wrappers around CGContextDrawLinearGradient In reply to: 93475425 [](ancestors = 93475425) |
||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 4); | ||
CGPoint startPoint = CGPointMake(0, 0); | ||
CGPoint endPoint = CGPointMake(512, 1024); | ||
|
||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradient2Short, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0, 0.33, 1.0 }; | ||
|
||
CGFloat components[] = { 0.85, 0, 0, 1.0, 1, 0, 0, 1.0, 0.85, 0.3, 0, 1.0 }; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 3); | ||
CGPoint startPoint = CGPointMake(120, 200); | ||
CGPoint endPoint = CGPointMake(350, 800); | ||
|
||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradient3, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0, 0.5, 1 }; | ||
|
||
CGFloat components[] = { | ||
1, 0, 0, 1.0, 0, 1, 0, 1.0, 0, 0, 1, 1.0, | ||
}; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 3); | ||
CGPoint startPoint = CGPointMake(0, 0); | ||
CGPoint endPoint = CGPointMake(512, 1024); | ||
|
||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientWithLowOpacity, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0, 0.5, 1 }; | ||
|
||
CGFloat components[] = { | ||
1, 0, 0, 0.1, 0, 1, 0, 0.9, 0, 0, 1, 0.8, | ||
}; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 3); | ||
CGPoint startPoint = CGPointMake(300, 750); | ||
CGPoint endPoint = CGPointMake(0, 0); | ||
|
||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientWithAlpha, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0, 0.25, 0.5, 0.6, 0.8, 0.9, 1 }; | ||
|
||
CGFloat components[] = { | ||
1, 0, 0, 1, 0.4, 0.1, 0.5, 1, 0.50, 0.2, 0.99, 1, 0.41, 0.56, 0, 1, 0.12, 0.12, .3, 1, 0.9, 0.4, 1, 1, 0.2, 0.3, 0.8, 1, | ||
}; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 7); | ||
CGPoint startPoint = CGPointMake(512, 1024); | ||
CGPoint endPoint = CGPointMake(0, 0); | ||
|
||
CGContextSetAlpha(context, 0.75); | ||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsBeforeStartLocation); | ||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
DRAW_TEST_F(CGGradient, LinearGradientWithLowOpacityShort, UIKitMimicTest) { | ||
CGContextRef context = GetDrawingContext(); | ||
|
||
CGFloat locations[] = { 0.0, 0.5, 1 }; | ||
|
||
CGFloat components[] = { | ||
1, 0, 0, 0.8, 0, 1, 0, 0.9, 0, 0, 1, 0.1, | ||
}; | ||
|
||
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); | ||
CGGradientRef gradient = CGGradientCreateWithColorComponents(colorspace, components, locations, 3); | ||
CGPoint startPoint = CGPointMake(250, 300); | ||
CGPoint endPoint = CGPointMake(0, 0); | ||
CGContextDrawLinearGradient(context, gradient, startPoint, endPoint, kCGGradientDrawsAfterEndLocation); | ||
|
||
CFRelease(colorspace); | ||
CFRelease(gradient); | ||
} | ||
|
||
#pragma endregion LinearGradient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be sc_ #WontFix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm s_k* is more appropriate #Resolved