Task/pattern fill #526
Task/pattern fill #526
Conversation
Update Pattern model Fix Cherry pick Last cherry pick Fix Merge fixes
| @@ -129,6 +129,8 @@ class ShapeRenderer: NodeRenderer { | |||
| ctx!.setFillColor(color.toCG()) | |||
| } else if let gradient = fill as? Gradient { | |||
| drawGradient(gradient, ctx: ctx, opacity: opacity) | |||
| } else if let pattern = fill as? Pattern { | |||
There was a problem hiding this comment.
It is not related to this PR (recommendation only), maybe we should convert this (in another similar place too) if constructions with many cases to appropriate switch cycle?
switch fill { case let gradient = fill as? Gradient: }
There was a problem hiding this comment.
I honestly don't see why switch would be better in this case (I think of it as a thingy for enums only), but I can do it, since I don't care very much
| } | ||
| case "polyline": | ||
| if let polyline = parsePolyline(node) { | ||
| let mask = try getMask(style, locus: polyline) | ||
| return Shape(form: polyline, fill: getFillColor(style, groupStyle: style), stroke: getStroke(style, groupStyle: style), place: position, opacity: getOpacity(style), clip: getClipPath(style, locus: polyline), mask: mask, tag: getTag(element)) | ||
| return Shape(form: polyline, fill: getFillColor(style, groupStyle: style, locus: polyline), stroke: getStroke(style, groupStyle: style), place: position, opacity: getOpacity(style), clip: getClipPath(style, locus: polyline), mask: mask, tag: getTag(element)) |
There was a problem hiding this comment.
Recommendation comment. We can add to swiftlint rule for 120-130 character string. Long strings not readable, especially in GitHub web version.
There was a problem hiding this comment.
Not only are they unreadable - they are unmergable. And the problem is not the length of the line per se, but the pure quantity of arguments. Not much can be done about that, since I don't think splitting each of them into 15 lines will help anything
Source/render/ShapeRenderer.swift
Outdated
| return | ||
| } | ||
| if !pattern.userSpace { | ||
| BoundsUtils.applyTransformToNodeInRespectiveCoords(respectiveNode: pattern.content, absoluteLocus: shape.form) |
There was a problem hiding this comment.
This line looks like a bug for me. You're modifying pattern during rendering. We should never change user model. Moreover these changes will accumulate, so pattern.content should be incorrect after several draws.
| for groupNode in patternGroup.contents { | ||
| applyTransformToNodeInRespectiveCoords(respectiveNode: groupNode, absoluteLocus: absoluteLocus) | ||
| } | ||
| } |
There was a problem hiding this comment.
What if respectiveNode is not a Shape and not a Group? Is it normal case? Or should we throw an exception for this?
f3dm76
force-pushed
the
task/pattern-fill
branch
from
e846d1d to
29bb3a5
Compare
No description provided.