Skip to content
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 CGPattern via runtime base #2374

Merged
merged 4 commits into from
Mar 31, 2017

Conversation

msft-Jeyaram
Copy link
Contributor

fixes #2353

@@ -518,12 +518,12 @@ - (UIColor*)initWithPatternImage:(UIImage*)image {
callbacks.releaseInfo = 0;
callbacks.drawPattern = __UIColorPatternFill;

_pattern = (id) CGPatternCreateColorspace(self, bounds, m, bounds.size.width, bounds.size.height, 0, NO, &callbacks, pImg->_has32BitAlpha ? _ColorABGR : _ColorBGR);
_pattern = _CGPatternCreateColorspace(self, bounds, m, bounds.size.width, bounds.size.height, 0, NO, &callbacks, pImg->_has32BitAlpha ? _ColorABGR : _ColorBGR);
Copy link
Contributor

@rajsesh rajsesh Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets remove the code in #if 0. #Resolved

@@ -224,7 +224,7 @@ @implementation UIColor {
@public
enum BrushType _type;
UIImage* _image;
Copy link
Contributor

@rajsesh rajsesh Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: while we are at this, could this be StrongId<>? #Resolved

- (void)dealloc {
if (generatedImage) {
CGImageRelease(generatedImage);
inline CGAffineTransform TransformMatrix() const {
Copy link
Contributor

@rajsesh rajsesh Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The name sounds like a verb. Adding a Get will clarify the intent. #Resolved


if (generatedImage) {
CGImageRelease(generatedImage);
inline CGRect Bounds() const {
Copy link
Contributor

@rajsesh rajsesh Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could return const ref. #Resolved

} else {
bitmapInfo = kCGImageAlphaNone;
colorSpace = CGColorSpaceCreateDeviceGray();
inline CGAffineTransform PatternTransform() {
Copy link
Contributor

@rajsesh rajsesh Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: GetPatternTransform() const #Resolved


return generatedImage;
}
inline CGPatternCallbacks Callbacks() const {
Copy link

@DHowett-MSFT DHowett-MSFT Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unused. Either use it or delete IssuePatternCallback #Resolved

@@ -58,5 +42,14 @@ CGRect _CGPatternGetFinalPatternSize(CGPatternRef pattern);
*/
bool _CGPatternIsColored(CGPatternRef pattern);

COREGRAPHICS_EXPORT CGPatternRef _CGPatternCreateColorspace(void* info,
Copy link

@DHowett-MSFT DHowett-MSFT Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't actually take or create a color space? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer needed.


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

@@ -450,6 +450,7 @@ LIBRARY CoreGraphics

; private exports below
_CGPatternCreateFromImage
_CGPatternCreateColorspace
Copy link

@DHowett-MSFT DHowett-MSFT Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only used internally, why is it exported? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it was in UIColor but now that's being removed. taking it out.


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

@msft-Jeyaram
Copy link
Contributor Author

@DHowett-MSFT @rajsesh-msft updated :)


return ret;
}
static const wchar_t* TAG = L"CGPattern";
Copy link
Member

@MSFTFox MSFTFox Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unused with LoggingNative? #Resolved

void* _info;
CGRect _bounds;
CGAffineTransform _transformMatrix;
CGFloat _xStep;
Copy link
Member

@MSFTFox MSFTFox Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xstep and ystep could be CGSize for simplicity. #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's much more implicit this way.


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

Also removed dependency on foundation as we removed usage and removal of  NSObjects.
…fines it as void *,

it should be struct __CGPattern*
@msft-Jeyaram msft-Jeyaram force-pushed the CGPattern_CPPBased_29_3 branch from e0f2870 to fa7f2ec Compare March 31, 2017 16:16
@msft-Jeyaram
Copy link
Contributor Author

Awaiting ARM test verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Foundation Dependency Removal] CGPattern should be implemented via CFRuntimeBase
5 participants