Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Add CoreGraphics.Drawing.UnitTests with 22 image-rendering tests.#1348

Merged
DHowett-MSFT merged 7 commits into
microsoft:CGD2Dfrom
DHowett-MSFT:201611-drawing-tests
Nov 15, 2016
Merged

Add CoreGraphics.Drawing.UnitTests with 22 image-rendering tests.#1348
DHowett-MSFT merged 7 commits into
microsoft:CGD2Dfrom
DHowett-MSFT:201611-drawing-tests

Conversation

@DHowett-MSFT

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

Copy link
Copy Markdown

WARNING: Every test here will fail until #1124 is complete.

Right now, this test facade emits images to disk using CGBitmapContext. It works on the reference platform.

I decided to make it a standalone test binary for one reason: I think its ideal interface is multi-modal; it needs to be able to generate a corpus of reference images on disk, and it needs to be able to diff them. For this, it needs a custom EntryPoint (included), and the ability to parse command-like arguments.

The default mode will be one that loads reference images from disk and fails tests if a number of pixels differ. That work is not yet complete.

Ideally, the draw tests here will be able to be plugged into another module that can perhaps render them to screen, or display them as part of a UI-driven flow for comparison and demoing purposes.

Refs #1271.


This change is Reviewable

@DHowett

DHowett commented Nov 10, 2016

Copy link
Copy Markdown
Member

This change contains a Makefile that was copied from Foundation.UnitTests.

PROPOSAL

  • Create UnitTests/UT.common.mk, with the following interface:
# Required
UT_NAME = Foundation # Can be automatically derived.
UT_FILES = $(wildcard *.cpp) $(wildcard *.mm)

# Optional, if we want pluggable entry points.
UT_FILES += $(UT_DEFAULT_FILES)

# Optional
UT_CFLAGS = -Wall -Werror

# A space-separated list of all linked frameworks.
# Foundation will be required by default as Logging uses it.
# Therefore, it might make sense to put it in UT.common.mk.
UT_FRAMEWORKS = CoreFoundation Foundation
UT_RESOURCE_DIRS = data/ otherdata/

include ../UT.common.mk
  • Port Foundation.UnitTests over to it.
  • Do the above work on develop.
  • Consume this after a merge from develop.

@ms-jihua This is pretty quick work; what do you think?

@@ -0,0 +1 @@

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Git does not track directories, it tracks files. This is an empty file intended to keep the data directory around.

namespace testing {
class DrawTest : public ::testing::Test {
protected:
static woc::unique_cf<CGColorSpaceRef> s_deviceColorSpace;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: would prefer these be accessed similar to parameterized test variables than implicitly available (say getContext

class DrawTest : public ::testing::Test {
protected:
static woc::unique_cf<CGColorSpaceRef> s_deviceColorSpace;
CGContextRef context;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be in a unique_cf?

(cherry picked from commit ad17b7af54cf6478ff2daefdd6e5ee99b5585bf1)
(cherry picked from commit 3c45d1ecbae2772cf9f895d9aab5198144488993)
@DHowett-MSFT

Copy link
Copy Markdown
Author

I forgot to add a few test files. That's been fixed.

woc::unique_cf<CGImageRef> image{ CGBitmapContextCreateImage(context) };
ASSERT_NE(nullptr, image);

woc::unique_cf<CFMutableDataRef> imageData{ CFDataCreateMutable(nullptr, 1048576) };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1048576 [](start = 77, length = 7)

would be good to comment how this number was derived

@DHowett-MSFT DHowett-MSFT Nov 10, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

// Number derived by absolutely guessing at a sane size for an image; 1MB
1048576

s_deviceColorSpace.release();
}

CGSize testing::DrawTest::CanvasSize() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CanvasSize() [](start = 26, length = 12)

would we ever care about being able to change this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Actually, the UIKitMimicTest fixture does exactly that; It presents a 2x-scaled context.

CGSize size = CanvasSize();

_context.reset(CGBitmapContextCreate(
nullptr, size.width, size.height, 8, size.width * 4, s_deviceColorSpace.get(), kCGImageAlphaPremultipliedFirst));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

size.width * 4 [](start = 45, length = 14)

  1. are we ever going to have tests that render to colorspaces with not 4 components?
  2. if not, call we call that out in a comment?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That is worth calling out, absolutely.

@rajsesh

rajsesh commented Nov 14, 2016

Copy link
Copy Markdown
Contributor

:shipit:

@DHowett-MSFT DHowett-MSFT merged commit 4e12663 into microsoft:CGD2D Nov 15, 2016
@DHowett-MSFT DHowett-MSFT deleted the 201611-drawing-tests branch November 15, 2016 03:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants