Fix #347: Fill color disappears if there is stroke color

exyte · Jan 25, 2019 · 4dceea8 · 4dceea8
1 parent 301d8eb
commit 4dceea8
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 14 deletions.
diff --git a/Source/model/scene/Text.swift b/Source/model/scene/Text.swift
@@ -18,8 +18,8 @@ open class Text: Node {
         set(val) { fontVar.value = val }
     }
 
-    public let fillVar: Variable<Fill>
-    open var fill: Fill {
+    public let fillVar: Variable<Fill?>
+    open var fill: Fill? {
         get { return fillVar.value }
         set(val) { fillVar.value = val }
     }
@@ -42,10 +42,10 @@ open class Text: Node {
         set(val) { baselineVar.value = val }
     }
 
-    public init(text: String, font: Font? = nil, fill: Fill = Color.black, stroke: Stroke? = nil, align: Align = .min, baseline: Baseline = .top, place: Transform = Transform.identity, opaque: Bool = true, opacity: Double = 1, clip: Locus? = nil, mask: Node? = nil, effect: Effect? = nil, visible: Bool = true, tag: [String] = []) {
+    public init(text: String, font: Font? = nil, fill: Fill? = Color.black, stroke: Stroke? = nil, align: Align = .min, baseline: Baseline = .top, place: Transform = Transform.identity, opaque: Bool = true, opacity: Double = 1, clip: Locus? = nil, mask: Node? = nil, effect: Effect? = nil, visible: Bool = true, tag: [String] = []) {
         self.textVar = Variable<String>(text)
         self.fontVar = Variable<Font?>(font)
-        self.fillVar = Variable<Fill>(fill)
+        self.fillVar = Variable<Fill?>(fill)
         self.strokeVar = Variable<Stroke?>(stroke)
         self.alignVar = Variable<Align>(align)
         self.baselineVar = Variable<Baseline>(baseline)

diff --git a/Source/render/TextRenderer.swift b/Source/render/TextRenderer.swift
@@ -46,16 +46,26 @@ class TextRenderer: NodeRenderer {
         let font = getMFont()
         // positive NSBaselineOffsetAttributeName values don't work, couldn't find why
         // for now move the rect itself
+        var attributes: [NSAttributedString.Key: Any] = [NSAttributedString.Key.font: font]
+        var hasFill = false
         if var color = text.fill as? Color {
             color = RenderUtils.applyOpacity(color, opacity: opacity)
-            var attributes = [NSAttributedString.Key.font: font,
-                              NSAttributedString.Key.foregroundColor: getTextColor(color)]
-            if let stroke = text.stroke {
-                if let c = stroke.fill as? Color {
-                    attributes[NSAttributedString.Key.strokeColor] = getTextColor(c)
-                }
-                attributes[NSAttributedString.Key.strokeWidth] = stroke.width as NSObject?
+            attributes[NSAttributedString.Key.foregroundColor] = getTextColor(color)
+            hasFill = true
+        }
+        if let stroke = text.stroke {
+            if let c = stroke.fill as? Color {
+                attributes[NSAttributedString.Key.strokeColor] = getTextColor(c)
+            }
+            var width = stroke.width
+            if (hasFill) {
+                // To use fill and stroke at the same time width should be negative:
+                // https://developer.apple.com/library/archive/qa/qa1531/_index.html
+                width *= -1
             }
+            attributes[NSAttributedString.Key.strokeWidth] = width as NSObject?
+        }
+        if (attributes.count > 1) {
             MGraphicsPushContext(context)
             message.draw(in: getBounds(font), withAttributes: attributes)
             MGraphicsPopContext()

diff --git a/Source/svg/SVGParser.swift b/Source/svg/SVGParser.swift
@@ -848,7 +848,7 @@ open class SVGParser {
         let string = text.text
         let position = pos.move(dx: getDoubleValue(text, attribute: "x") ?? 0, dy: getDoubleValue(text, attribute: "y") ?? 0)
 
-        return Text(text: string, font: getFont(fontName: fontName, fontWeight: fontWeight, fontSize: fontSize), fill: fill ?? Color.black, stroke: stroke, align: anchorToAlign(textAnchor), baseline: .bottom, place: position, opacity: opacity, tag: getTag(text))
+        return Text(text: string, font: getFont(fontName: fontName, fontWeight: fontWeight, fontSize: fontSize), fill: fill, stroke: stroke, align: anchorToAlign(textAnchor), baseline: .bottom, place: position, opacity: opacity, tag: getTag(text))
     }
 
     // REFACTOR
@@ -895,7 +895,7 @@ open class SVGParser {
         }
         trimmedString = withWhitespace ? " \(trimmedString)" : trimmedString
         let text = Text(text: trimmedString, font: getFont(fontName: fontName, fontWeight: fontWeight, fontSize: fontSize),
-                        fill: fill ?? Color.black, stroke: stroke, align: anchorToAlign(textAnchor), baseline: .alphabetic,
+                        fill: fill, stroke: stroke, align: anchorToAlign(textAnchor), baseline: .alphabetic,
                         place: Transform().move(dx: bounds.x + bounds.w, dy: bounds.y), opacity: opacity)
         collection.append(text)
         if tagRange.location >= fullString.length { // leave recursion
@@ -919,7 +919,7 @@ open class SVGParser {
         let attributes = getStyleAttributes([:], element: element)
 
         return Text(text: text, font: getFont(attributes, fontName: fontName, fontWeight: fontWeight, fontSize: fontSize),
-                    fill: ((attributes["fill"] != nil) ? getFillColor(attributes)! : fill) ?? Color.black, stroke: stroke ?? getStroke(attributes),
+                    fill: (attributes["fill"] != nil) ? getFillColor(attributes)! : fill, stroke: stroke ?? getStroke(attributes),
                     align: anchorToAlign(textAnchor ?? getTextAnchor(attributes)), baseline: .alphabetic,
                     place: pos, opacity: getOpacity(attributes), tag: getTag(element))
     }