Merge pull request #1309 from rajsesh-msft/cherry_pick_11_02

Cherry pick 11 02

Frameworks/CoreGraphics/CGBitmapImage.mm

@@ -389,6 +389,11 @@
 }
 
 CGBitmapImageBacking::~CGBitmapImageBacking() {
+    // release the render target first as it may hold locks on the image
+    if (_renderTarget != nullptr) {
+        _renderTarget->Release();
+    }
+
     if (_cairoLocks != 0 || _imageLocks != 0) {
         TraceWarning(TAG, L"Warning: Image data not unlocked (refcnt=%d, %d)", _cairoLocks, _imageLocks);
 
@@ -400,10 +405,6 @@
         }
     }
 
-    if (_renderTarget != nullptr) {
-        _renderTarget->Release();
-    }
-
     _data->_refCount--;
 
     if (_data->_refCount == 0) {

Frameworks/CoreGraphics/CGContext.mm

@@ -1239,8 +1239,8 @@ bool CGContextIsPointInPath(CGContextRef c, bool eoFill, CGFloat x, CGFloat y) {
     return c->Backing()->CGContextIsPointInPath(eoFill, x, y);
 }
 
-void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun, float lineAscent) {
-    ctx->Backing()->CGContextDrawGlyphRun(glyphRun, lineAscent);
+void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun) {
+    ctx->Backing()->CGContextDrawGlyphRun(glyphRun);
 }
 // TODO 1077:: Remove once D2D render target is implemented
 void _CGContextSetScaleFactor(CGContextRef ctx, float scale) {

Frameworks/CoreGraphics/CGContextCairo.mm

@@ -1832,7 +1832,7 @@
  *
  * @parameter glyphRun DWRITE_GLYPH_RUN object to render
  */
-void CGContextCairo::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent) {
+void CGContextCairo::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun) {
     ObtainLock();
 
     CGContextStrokePath();
@@ -1853,33 +1853,25 @@
 
     // Apply the text transformation (text position, text matrix) in text space rather than user space
     // This means flipping the coordinate system,
-    // and applying the transformation about the center of the glyph run rather than about the baseline
-    // Flip and translate by the difference between the center and the baseline, apply text transforms, then flip and translate back
+    // Apply text position, where it will be translated to correct position given text matrix value
+    CGAffineTransform textTransform =
+        CGAffineTransformTranslate(curState->curTextMatrix, curState->curTextPosition.x, curState->curTextPosition.y);
 
-    // Transform to text space
-    // Technically there should be a horizontal translation to the center as well,
-    // but it's to the center of _each individual glyph_, as the reference platform applies the text matrix to each glyph individually
-    // Uncertain whether it's ever going to be worth it to support this using DWrite, so just ignore it for now
-    CGAffineTransform transform = CGAffineTransformMake(1, 0, 0, -1, 0, -lineAscent / 2.0f);
-
-    // Apply text transforms
-    transform = CGAffineTransformConcat(curState->curTextMatrix, transform);
-
-    // Undo transform to text space
-    transform = CGAffineTransformConcat(CGAffineTransformMake(1, 0, 0, -1, 0, lineAscent / 2.0f), transform);
-
-    transform = CGAffineTransformTranslate(transform, curState->curTextPosition.x, curState->curTextPosition.y);
+    // Undo assumed inversion about Y axis
+    textTransform = CGAffineTransformConcat(CGAffineTransformMake(1, 0, 0, -1, 0, 0), textTransform);
 
     // Find transform that user created by multiplying given transform by necessary transforms to draw with CoreText
+    // First multiply by inverse scale to get properly scaled values
+    // Then undo assumed inversion about Y axis
+    // Finally inverse translate by height
+    // All of which are rolled into one concatenation
     float height = _imgDest->Backing()->Height();
     CGAffineTransform userTransform =
-        CGAffineTransformConcat(curState->curTransform, CGAffineTransformMake(1.0f / _scale, 0, 0, 1.0f / _scale, 0, -height / _scale));
-
-    // Apply the context CTM
-    transform = CGAffineTransformConcat(transform, userTransform);
+        CGAffineTransformConcat(curState->curTransform, CGAffineTransformMake(1.0f / _scale, 0, 0, -1.0f / _scale, 0, height / _scale));
 
-    // Perform anti-clockwise rotation required to match the reference platform.
-    imgRenderTarget->SetTransform(D2D1::Matrix3x2F(transform.a, -transform.b, transform.c, transform.d, transform.tx, -transform.ty));
+    // Apply the two transforms giving us the final result
+    CGAffineTransform transform = CGAffineTransformConcat(textTransform, userTransform);
+    imgRenderTarget->SetTransform(D2D1::Matrix3x2F(transform.a, transform.b, transform.c, transform.d, transform.tx, transform.ty));
 
     // Draw the glyph using ID2D1RenderTarget
     ComPtr<ID2D1SolidColorBrush> brush;

Frameworks/CoreGraphics/CGContextImpl.mm

@@ -779,7 +779,7 @@
     return NULL;
 }
 
-void CGContextImpl::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent) {
+void CGContextImpl::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun) {
 }
 
 // TODO 1077:: Remove once D2D render target is implemented

Frameworks/CoreText/CTFont.mm

@@ -902,4 +902,31 @@ CFDataRef CTFontCopyTable(CTFontRef font, CTFontTableTag table, CTFontTableOptio
 CFTypeID CTFontGetTypeID() {
     static CFTypeID __kCTFontTypeID = _CFRuntimeRegisterClass(&__CTFontClass);
     return __kCTFontTypeID;
+}
+
+// Private function for getting font weight for XAML
+DWRITE_FONT_WEIGHT _CTFontGetDWriteWeight(CTFontRef font) {
+    ComPtr<IDWriteFontFace3> fontFace3;
+    if (font && SUCCEEDED(font->_dwriteFontFace.As(&fontFace3))) {
+        return fontFace3->GetWeight();
+    }
+    return DWRITE_FONT_WEIGHT_NORMAL;
+}
+
+// Private function for getting font stretch for XAML
+DWRITE_FONT_STRETCH _CTFontGetDWriteStretch(CTFontRef font) {
+    ComPtr<IDWriteFontFace3> fontFace3;
+    if (font && SUCCEEDED(font->_dwriteFontFace.As(&fontFace3))) {
+        return fontFace3->GetStretch();
+    }
+    return DWRITE_FONT_STRETCH_NORMAL;
+}
+
+// Private function for getting font style for XAML
+DWRITE_FONT_STYLE _CTFontGetDWriteStyle(CTFontRef font) {
+    ComPtr<IDWriteFontFace3> fontFace3;
+    if (font && SUCCEEDED(font->_dwriteFontFace.As(&fontFace3))) {
+        return fontFace3->GetStyle();
+    }
+    return DWRITE_FONT_STYLE_NORMAL;
 }

Frameworks/CoreText/CTFrame.mm

@@ -122,8 +122,9 @@ void CTFrameDraw(CTFrameRef frameRef, CGContextRef ctx) {
         CGContextScaleCTM(ctx, 1.0f, -1.0f);
 
         for (_CTLine* line in static_cast<id<NSFastEnumeration>>(frame->_lines)) {
+            // Y position must be negative because the context is inverted
             CGContextSetTextPosition(ctx, line->_lineOrigin.x, -line->_lineOrigin.y);
-            _CTLineDraw(static_cast<CTLineRef>(line), ctx, false);
+            CTLineDraw(static_cast<CTLineRef>(line), ctx);
         }
 
         // Restore CTM and Text Matrix to values before we modified them

Frameworks/CoreText/CTLine.mm

@@ -215,40 +215,28 @@ CTLineRef CTLineCreateJustifiedLine(CTLineRef line, CGFloat justificationFactor,
     return StubReturn();
 }
 
-void _CTLineDraw(CTLineRef lineRef, CGContextRef ctx, bool adjustTextPosition) {
-    if (!lineRef) {
+/**
+ @Status Interoperable
+*/
+void CTLineDraw(CTLineRef lineRef, CGContextRef ctx) {
+    if (lineRef == nil || ctx == nil) {
         return;
     }
 
     _CTLine* line = static_cast<_CTLine*>(lineRef);
-    CGPoint curTextPos = {};
-    if (adjustTextPosition) {
-        curTextPos = CGContextGetTextPosition(ctx);
-        CGContextSetTextPosition(ctx, curTextPos.x, curTextPos.y + line->_relativeYOffset);
-    }
 
     for (size_t i = 0; i < [line->_runs count]; ++i) {
         _CTRun* curRun = [line->_runs objectAtIndex:i];
         if (i > 0) {
             // Adjusts x position relative to the last run drawn
-            curTextPos = CGContextGetTextPosition(ctx);
+            CGPoint curTextPos = CGContextGetTextPosition(ctx);
             CGContextSetTextPosition(ctx, curTextPos.x + curRun->_relativeXOffset, curTextPos.y);
         }
 
-        // Get height of the line so we draw with the correct baseline for each run
-        CGFloat ascent;
-        CTLineGetTypographicBounds(lineRef, &ascent, nullptr, nullptr);
-        _CTRunDraw(static_cast<CTRunRef>(curRun), ctx, CFRange{}, false, ascent);
+        CTRunDraw(static_cast<CTRunRef>(curRun), ctx, CFRange{});
     }
 }
 
-/**
- @Status Interoperable
-*/
-void CTLineDraw(CTLineRef lineRef, CGContextRef ctx) {
-    _CTLineDraw(lineRef, ctx, true);
-}
-
 /**
  @Status Interoperable
 */

Frameworks/CoreText/CTRun.mm

@@ -268,18 +268,17 @@ CGRect CTRunGetImageBounds(CTRunRef run, CGContextRef context, CFRange range) {
     return StubReturn();
 }
 
-void _CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange, bool adjustTextPosition, CGFloat lineAscent) {
+/**
+ @Status Interoperable
+ @Notes
+*/
+void CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange) {
     _CTRun* curRun = static_cast<_CTRun*>(run);
     if (!curRun || textRange.length < 0L || textRange.location < 0L ||
         textRange.location + textRange.length > curRun->_dwriteGlyphRun.glyphCount) {
         return;
     }
 
-    if (adjustTextPosition) {
-        CGPoint curTextPos = CGContextGetTextPosition(ctx);
-        CGContextSetTextPosition(ctx, curTextPos.x, curTextPos.y + curRun->_relativeYOffset);
-    }
-
     id fontColor = [curRun->_attributes objectForKey:(id)kCTForegroundColorAttributeName];
     if (fontColor == nil) {
         CFBooleanRef useContextColor =
@@ -294,27 +293,15 @@ void _CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange, bool adjustTe
 
     if (textRange.location == 0L && (textRange.length == 0L || textRange.length == curRun->_dwriteGlyphRun.glyphCount)) {
         // Print the whole glyph run
-        CGContextDrawGlyphRun(ctx, &curRun->_dwriteGlyphRun, lineAscent);
+        CGContextDrawGlyphRun(ctx, &curRun->_dwriteGlyphRun);
     } else {
         if (textRange.length == 0L) {
             textRange.length = curRun->_dwriteGlyphRun.glyphCount - textRange.location;
         }
 
         // Only print glyphs in range
         DWRITE_GLYPH_RUN runInRange = __GetGlyphRunForDrawingInRange(curRun->_dwriteGlyphRun, textRange);
-        CGContextDrawGlyphRun(ctx, &runInRange, lineAscent);
-    }
-}
-
-/**
- @Status Interoperable
- @Notes
-*/
-void CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange) {
-    if (run && ctx) {
-        CGFloat ascent;
-        CTRunGetTypographicBounds(run, {}, &ascent, nullptr, nullptr);
-        _CTRunDraw(run, ctx, textRange, true, ascent);
+        CGContextDrawGlyphRun(ctx, &runInRange);
     }
 }
 

Frameworks/CoreText/DWriteWrapper_CoreText.mm

@@ -204,7 +204,7 @@ bool _CloneDWriteGlyphRun(_In_ DWRITE_GLYPH_RUN const* src, _Out_ DWRITE_GLYPH_R
     for (size_t i = 0; i < [subString length]; i += attributeRange.length) {
         NSDictionary* attribs = [static_cast<NSAttributedString*>(string) attributesAtIndex:i + range.location
                                                                       longestEffectiveRange:&attributeRange
-                                                                                    inRange:{ i, [subString length] }];
+                                                                                    inRange:{ i + range.location, [subString length] }];
 
         const DWRITE_TEXT_RANGE dwriteRange = { attributeRange.location, attributeRange.length };
 

Frameworks/QuartzCore/CALayer.mm

@@ -574,7 +574,7 @@ - (void)display {
                 if (useVector) {
                     // target = new CGVectorImage(width, height, _ColorBGR);
                 } else {
-                    drawContext = _CGBitmapContextCreateWithFormat(width, height, _ColorBGR);
+                    drawContext = _CGBitmapContextCreateWithFormat(width, height, _ColorBGRX);
                 }
                 priv->drewOpaque = TRUE;
             } else {

Frameworks/UIKit/NSLayoutManager.mm

@@ -246,10 +246,7 @@ - (void)drawGlyphsForGlyphRange:(NSRange)range atPoint:(CGPoint)position {
     for (int curLine = 0; curLine < count; curLine++) {
         CTLineRef line = (CTLineRef)_ctLines[curLine];
         CGContextSaveGState(curCtx);
-
-        CGFloat ascent, leading;
-        CTLineGetTypographicBounds(line, &ascent, nullptr, &leading);
-        CGContextSetTextPosition(curCtx, _lineOrigins[curLine].x, -(_lineOrigins[curLine].y + ascent + leading));
+        CGContextSetTextPosition(curCtx, _lineOrigins[curLine].x, -_lineOrigins[curLine].y);
         CTLineDraw(line, curCtx);
         CGContextRestoreGState(curCtx);
     }

Frameworks/UIKit/NSString+UIKitAdditions.mm

@@ -104,10 +104,7 @@ static void drawString(UIFont* font,
     for (size_t i = 0; i < origins.size(); ++i) {
         // Need to set text position so each line will be drawn in the correct position relative to each other
         // Y positions will be negative because we are drawing with the coordinate system flipped to what CoreText is expecting
-        // Translated down by lineheight (ascent - descent + leading) to set origin in the correct position
-        CGFloat ascent, descent, leading;
-        CTLineGetTypographicBounds(static_cast<CTLineRef>(lines[i]), &ascent, &descent, &leading);
-        CGContextSetTextPosition(context, rect.origin.x + origins[i].x, -(rect.origin.y + origins[i].y + ascent - descent + leading));
+        CGContextSetTextPosition(context, rect.origin.x + origins[i].x, -(rect.origin.y + origins[i].y));
         CTLineDraw(static_cast<CTLineRef>(lines[i]), context);
     }
 

Frameworks/UIKit/StarboardXaml/CompositorInterface.h

@@ -21,6 +21,16 @@
 #include "winobjc\winobjc.h"
 #include <ppltasks.h>
 
+#ifdef __clang__
+#include <COMIncludes.h>
+#endif
+
+#include <DWrite_3.h>
+
+#ifdef __clang__
+#include <COMIncludes_End.h>
+#endif
+
 class DisplayNode;
 class DisplayTexture;
 class DisplayAnimation;
@@ -159,8 +169,9 @@ class DisplayTextureXamlGlyphs : public DisplayTexture {
     float _fontSize;
     float _lineHeight;
     bool _centerVertically;
-    bool _isBold = false;
-    bool _isItalic = false;
+    DWRITE_FONT_WEIGHT _fontWeight;
+    DWRITE_FONT_STRETCH _fontStretch;
+    DWRITE_FONT_STYLE _fontStyle;
 
     DisplayTextureXamlGlyphs();
     ~DisplayTextureXamlGlyphs();

Frameworks/UIKit/StarboardXaml/CompositorInterface.mm

@@ -329,9 +329,10 @@ void SetParams(UIFont* font,
         _centerVertically = centerVertically;
         _lineHeight = [font ascender] - [font descender];
 
-        int mask = [font fontDescriptor].symbolicTraits;
-        _isBold = (mask & UIFontDescriptorTraitBold) > 0;
-        _isItalic = (mask & UIFontDescriptorTraitItalic) > 0;
+        _fontWeight = [font _fontWeight];
+        _fontStretch = [font _fontStretch];
+        _fontStyle = [font _fontStyle];
+
         std::wstring wideBuffer = Strings::NarrowToWide<std::wstring>(text);
 
         // The Font Family names DWrite will return are not always compatible with Xaml

Frameworks/UIKit/StarboardXaml/XamlCompositor.cpp

@@ -561,17 +561,9 @@ void DisplayTextureXamlGlyphs::ConstructGlyphs(const Microsoft::WRL::Wrappers::H
     textControl->Foreground = ref new Windows::UI::Xaml::Media::SolidColorBrush(textColor);
     textControl->FontFamily = ref new Windows::UI::Xaml::Media::FontFamily(reinterpret_cast<Platform::String^>(fontFamilyName.Get()));
 
-    if (_isBold) {
-        textControl->FontWeight = Windows::UI::Text::FontWeights::Bold;
-    } else {
-        textControl->FontWeight = Windows::UI::Text::FontWeights::Normal;
-    }
-
-    if (_isItalic) {
-        textControl->FontStyle = Windows::UI::Text::FontStyle::Italic;
-    } else {
-        textControl->FontStyle = Windows::UI::Text::FontStyle::Normal;
-    }
+    textControl->FontWeight = Windows::UI::Text::FontWeight{ static_cast<unsigned short>(_fontWeight) };
+    textControl->FontStretch = static_cast<Windows::UI::Text::FontStretch>(_fontStretch);
+    textControl->FontStyle = static_cast<Windows::UI::Text::FontStyle>(_fontStyle);
 
     switch (_horzAlignment) {
         case alignLeft:

Frameworks/UIKit/UIFont.mm

@@ -34,6 +34,7 @@
 #import "LoggingNative.h"
 #import "UICTFont.h"
 #import "UIFontInternal.h"
+#import <CoreTextInternal.h>
 
 #include <COMIncludes.h>
 #import <DWrite.h>
@@ -296,4 +297,22 @@ - (NSString*)_compatibleFamilyName {
     return static_cast<NSString*>(_DWriteGetFamilyNameForFontName(static_cast<CFStringRef>([self fontName])));
 }
 
+// WinObjC-only extension for compatibility issues between DWrite and Xaml
+// Returns the weight of the font Xaml can use
+- (DWRITE_FONT_WEIGHT)_fontWeight {
+    return _CTFontGetDWriteWeight(static_cast<CTFontRef>(self));
+}
+
+// WinObjC-only extension for compatibility issues between DWrite and Xaml
+// Returns the stretch of the font Xaml can use
+- (DWRITE_FONT_STRETCH)_fontStretch {
+    return _CTFontGetDWriteStretch(static_cast<CTFontRef>(self));
+}
+
+// WinObjC-only extension for compatibility issues between DWrite and Xaml
+// Returns the style of the font Xaml can use
+- (DWRITE_FONT_STYLE)_fontStyle {
+    return _CTFontGetDWriteStyle(static_cast<CTFontRef>(self));
+}
+
 @end

Frameworks/include/CGContextCairo.h

@@ -138,7 +138,7 @@ class CGContextCairo : public CGContextImpl {
     virtual bool CGContextIsPointInPath(bool eoFill, float x, float y);
     virtual CGPathRef CGContextCopyPath(void);
 
-    virtual void CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent);
+    virtual void CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun);
 
     // TODO 1077:: Remove once D2D render target is implemented
     virtual void _CGContextSetScaleFactor(float scale);