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

Fix the drawing tests to not use Objective-C and build on OS X. #1470

Merged
merged 4 commits into from
Nov 30, 2016

Conversation

DHowett-MSFT
Copy link

@DHowett-MSFT DHowett-MSFT commented Nov 29, 2016

This fixes #1453 and #1448.

I thought that #1453 was the cause of the teardown crash I was seeing, so it got fixed here; it turns out that CGImageDestination was overreleasing a CGImage's CGDataProvider.


This change is Reviewable


virtual std::string GetResourcePath(const std::string& resource) override {
// Using / as a path separator is valid on both Windows and OS X.
return __ModuleDirectory() + "/data/" + resource;
Copy link
Contributor

@msft-Jeyaram msft-Jeyaram Nov 30, 2016

Choose a reason for hiding this comment

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

"/data/" [](start = 36, length = 10)

Curious: In windows the files aren't copied into data.
Also it would be better not to hard code /data/ in this function and pass it in?
(make it more generic? maybe?) #ByDesign

Copy link
Author

@DHowett-MSFT DHowett-MSFT Nov 30, 2016

Choose a reason for hiding this comment

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

The Drawing test vcxproj absolutely copies a data/ directory.

If we standardize on data/, we don't have to propagate that decision into every test file consumer (and centralizing it here lets us change it in a single place if we do decide to.) #ByDesign

Copy link
Contributor

Choose a reason for hiding this comment

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

for some reason I was thinking it wasn't. Double checked and it is.
gotcha.


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

@msft-Jeyaram
Copy link
Contributor

msft-Jeyaram commented Nov 30, 2016

/* static / DrawingTestConfigImpl DrawingTestConfig::Get() {

static? #ByDesign


Refers to: tests/UnitTests/CoreGraphics.drawing/EntryPoint.cpp:93 in f000645. [](commit_id = f000645, deletion_comment = False)

@DHowett-MSFT
Copy link
Author

/* static / DrawingTestConfigImpl DrawingTestConfig::Get() {

Yes.

This is a static member function on DrawingTestConfig.

Since we cannot annotate it as such via classical means, this comment serves to illustrate that it is static.


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


Refers to: tests/UnitTests/CoreGraphics.drawing/EntryPoint.cpp:93 in f000645. [](commit_id = f000645, deletion_comment = False)

@@ -576,8 +576,7 @@ bool _CGIsValidRenderTargetPixelFormat(WICPixelFormatGUID pixelFormat) {
HRESULT _CGImageGetWICImageSource(CGImageRef image, IWICBitmap** source) {
RETURN_HR_IF_NULL(E_INVALIDARG, image);
RETURN_HR_IF_NULL(E_POINTER, source);
*source = image->ImageSource().Get();
return S_OK;
return image->ImageSource().CopyTo(source);
Copy link
Contributor

@rajsesh rajsesh Nov 30, 2016

Choose a reason for hiding this comment

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

😮 #Resolved


DISABLED_DRAW_TEST_F(CGContext, DrawIntoRect, UIKitMimicTest) {
// Draw a portion of an image into a different region.
CFDataRef data = (CFDataRef)[NSData dataWithContentsOfFile:getPathToFile(@"png3.9.png")];
woc::unique_cf<CGDataProviderRef> dataProvider(CGDataProviderCreateWithCFData(data));
auto drawingConfig = DrawingTestConfig::Get();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not a good use of auto when the return type is known.

Copy link
Author

Choose a reason for hiding this comment

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

Very true.


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

@DHowett-MSFT DHowett-MSFT merged commit 5afebc1 into microsoft:CGD2D Nov 30, 2016
@DHowett-MSFT DHowett-MSFT deleted the 201612-defgh branch November 30, 2016 02:29
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.

7 participants