UI Image fixes #636

shahmeeresmail · 2016-07-12T00:16:47Z

Fixes graphical corruption in UI elements (examples: UIDatePicker, UISegmentControl) resulting in a checkerboard pattern being drawn and optimized patched draw function. The corruption is what is seen in issue #622 and is visible in the Segments and DatePicker sections of WOCCATALOG. This also resolves issue #674 since it implements the use of blendMode and alpha for the various drawAtPoint and drawInRect functions.

Details:

Instead of creating a pattern and calling a CB function (drawNinePatchCallback) to subdivide image into patches and draw individually, the change was made to just call a function which subdivides the images into patches and draws (drawPatches)
Insets improperly scaled in many places
drawPatches optimized to subdivide the image into the smallest number of patches to reduce draw setup and state management overhead
Depending on what insets are set, 1-9 patched draws may be needed. Optimized drawPatches function to do 1 to 9 draws instead of 3 to 9 draws (effectively reducing min draws from 3 to 1) and minimize the number of draws used.
Make makeRect and drawPatches functions inline

…ges with imageCaps: - drawNinePatchCallback renamed to drawPatches - Instead of creating a pattern and calling a CB function (drawNinePatchCallback) to subdivide image into patches and draw individually, the change was made to just call a function which subdivides the images into patches and draws (drawPatches) - Insets improperly scaled in many places - drawPatches optimized to subdivide the image into the smallest number of patches to reduce draw setup and state management overhead - make drawPatches and makeRect inline

msft-Jeyaram · 2016-07-12T19:12:52Z

Frameworks/UIKit/UIImage.mm

+                                   makeRect((dst->origin.x + leftCap),
+                                            (dst->origin.y + botCap),
+                                            (dst->size.width - leftCap - rightCap),
+                                            (dst->size.height - topCap - botCap)));
        } else {
            assert(0);


assert(0); [](start = 11, length = 11)

although this is old code, it would be much better to Throw and exception with some information on why this is an issue.
It would provide much more information rather than just asserting.

Replaced assert with unimplemented_with_msg macro.

…oreGraphicsFixes

rajsesh · 2016-07-13T02:12:11Z

@Ramu-msft is added to the review. #Closed

rajsesh · 2016-07-13T02:12:23Z

@davelamb is added to the review. #Closed

davelamb · 2016-07-13T22:20:19Z

Frameworks/UIKit/UIImage.mm

+                                            (dst->size.height - topCap - botCap)));
+        }
+    } else {
+        UNIMPLEMENTED_WITH_MSG("Patched draws only supported when sum of topCap and botCap is less than the height of the UI element.");


This doesn't seem to be the case on the reference platform. It seems to draw the caps as a ratio between the cap sizes.

That's true and this is a pre-existing issue which isn't resolved here. We should create another pull request in the future to address this and maybe even a functional test to make sure the output produced in this scenario matches that of the reference platform.

davelamb · 2016-07-14T00:11:14Z

We've root caused the crashing issue to a missing call to EMMS (_mm_empty, clears MMX state) and the tiling issue to x/y step size being incorrect in the call to CGPatternCreate. I have a small fix that addresses both these issues that we'd like to push out that shouldn't collide (but for one line) with your change.

…os, blendMode, alpha), drawAtPoint(pos), drawAtPoint(pos, blendMode, alpha) - When alpha and blendMode are not provided in drawRect and drawAtPoint functions, use values from the current graphics state instead of hardcoded values which force full opacity or transparency and a KCGBlendModeNormal and thus ignoring the app's intentions - In drawRect/drawAtPoint, set opacity (alpha) as part of draw setup - Use cairo_paint_with_alpha using the current alpha/opacity state instead of using just cairo_paint

jaredhms · 2016-07-22T20:12:29Z

}

delete and just call CGRectMake?

Refers to: Frameworks/UIKit/UIImage.mm:948 in 532b33e. [](commit_id = 532b33e, deletion_comment = False)

jaredhms · 2016-07-22T20:29:55Z

static void drawLeftCap(UIImage* self, CGContextRef cur, CGRect pos) {

Can we delete this?

Refers to: Frameworks/UIKit/UIImage.mm:821 in 532b33e. [](commit_id = 532b33e, deletion_comment = False)

jaredhms · 2016-07-22T20:30:02Z

static void drawTopCap(UIImage* self, CGContextRef cur, CGRect pos) {

Can we delete this?

Refers to: Frameworks/UIKit/UIImage.mm:883 in 532b33e. [](commit_id = 532b33e, deletion_comment = False)

jaredhms · 2016-07-22T20:30:41Z

static void drawLeftAndTopCap(UIImage* self, CGContextRef ctx, CGRect rect) {

Can we delete this?

Refers to: Frameworks/UIKit/UIImage.mm:950 in 532b33e. [](commit_id = 532b33e, deletion_comment = False)

shahmeeresmail · 2016-07-22T21:28:25Z

I don't see why we can't delete the drawTopCap, drawLeftCap or drawLeftAndTopCap methods. They are unused but I originaly left them alone because I wasn't sure if it was envisioned to use them as part of a future change.

shahmeeresmail · 2016-07-22T21:29:45Z

I will also remove makeRect and substitute existing references to it with CGRectMake and make a few small changes to drawInRect(pos) and drawAtPoint(point) functions.

…eRect, removed unused static functions, made changes to drawInRect(pos) and drawAtPoint(pos) so that blendMode and alpha default to normal and 1.0f as per the spec.

jaredhms · 2016-07-25T17:07:40Z

Frameworks/UIKit/UIImage.mm

-    srcRect.origin.y = float(getImage(self)->Backing()->Height());
-    srcRect.size.width = float(getImage(self)->Backing()->Width());
-    srcRect.size.height = -((float)getImage(self)->Backing()->Height());
+    srcRect.origin.y = float(imgBacking->Height());


float( [](start = 23, length = 6)

nit: it's a bid odd that two of these are constructing a float rather than casting to a float

jaredhms · 2016-07-25T17:17:20Z

In CI build.

jaredhms · 2016-07-25T17:19:45Z

This reverts commit a106d3d.

…ave the fixes for High-DPI issues ready to merge. This reverts commit a106d3d.

Shahmeer Esmail added 2 commits July 11, 2016 15:48

Moved a comment inside an if statement.

be5942a

msftclas added the cla-already-signed label Jul 12, 2016

msft-Jeyaram reviewed Jul 12, 2016
View reviewed changes

Shahmeer Esmail added 3 commits July 12, 2016 16:30

Replace asserts with UNSUPPORTED_MSG macros.

e110851

Minor change.

799c50c

Merge branch 'develop' of https://github.com/Microsoft/WinObjC into C…

e4004cc

…oreGraphicsFixes

rajsesh added the Area-Foundation label Jul 13, 2016

rajsesh added Area-User Experience and removed Area-Foundation labels Jul 13, 2016

rajsesh assigned Ramu-msft Jul 13, 2016

davelamb reviewed Jul 13, 2016
View reviewed changes

Merge remote-tracking branch 'upstream/develop' into CoreGraphicsFixes

390f511

shahmeeresmail changed the title ~~Core graphics fixes~~ UI Image fixes Jul 15, 2016

Shahmeer Esmail added 2 commits July 21, 2016 16:50

Update copyright headers and remove kCGBlendModeInvalid enum

532b33e

Removed makeRect and substituted previous references to it with CGMak…

51f4f4f

…eRect, removed unused static functions, made changes to drawInRect(pos) and drawAtPoint(pos) so that blendMode and alpha default to normal and 1.0f as per the spec.

jaredhms reviewed Jul 25, 2016
View reviewed changes

jaredhms merged commit a106d3d into microsoft:develop Jul 25, 2016

Ramu-msft added a commit that referenced this pull request Jul 27, 2016

Revert "UI Image fixes (#636)"

68c7e96

This reverts commit a106d3d.

rajsesh pushed a commit that referenced this pull request Aug 2, 2016

Per shiproom discussion, reverting "UI Image fixes (#636)" until we h…

f34a09a

…ave the fixes for High-DPI issues ready to merge. This reverts commit a106d3d.

rajsesh mentioned this pull request Sep 6, 2016

Give effect to blendMode and alpha/opacity values in parameters #674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI Image fixes #636

UI Image fixes #636

shahmeeresmail commented Jul 12, 2016 •

edited

Loading

msft-Jeyaram Jul 12, 2016

shahmeeresmail Jul 12, 2016

rajsesh commented Jul 13, 2016

rajsesh commented Jul 13, 2016

davelamb Jul 13, 2016

shahmeeresmail Jul 13, 2016 •

edited

Loading

davelamb commented Jul 14, 2016

jaredhms commented Jul 22, 2016

jaredhms commented Jul 22, 2016

jaredhms commented Jul 22, 2016

jaredhms commented Jul 22, 2016

shahmeeresmail commented Jul 22, 2016 •

edited

Loading

shahmeeresmail commented Jul 22, 2016 •

edited

Loading

jaredhms Jul 25, 2016

jaredhms commented Jul 25, 2016

jaredhms commented Jul 25, 2016

UI Image fixes #636

UI Image fixes #636

Conversation

shahmeeresmail commented Jul 12, 2016 • edited Loading

msft-Jeyaram Jul 12, 2016

Choose a reason for hiding this comment

shahmeeresmail Jul 12, 2016

Choose a reason for hiding this comment

rajsesh commented Jul 13, 2016

rajsesh commented Jul 13, 2016

davelamb Jul 13, 2016

Choose a reason for hiding this comment

shahmeeresmail Jul 13, 2016 • edited Loading

Choose a reason for hiding this comment

davelamb commented Jul 14, 2016

jaredhms commented Jul 22, 2016

jaredhms commented Jul 22, 2016

jaredhms commented Jul 22, 2016

jaredhms commented Jul 22, 2016

shahmeeresmail commented Jul 22, 2016 • edited Loading

shahmeeresmail commented Jul 22, 2016 • edited Loading

jaredhms Jul 25, 2016

Choose a reason for hiding this comment

jaredhms commented Jul 25, 2016

jaredhms commented Jul 25, 2016

shahmeeresmail commented Jul 12, 2016 •

edited

Loading

shahmeeresmail Jul 13, 2016 •

edited

Loading

shahmeeresmail commented Jul 22, 2016 •

edited

Loading

shahmeeresmail commented Jul 22, 2016 •

edited

Loading