I fixed several issues with the vector drawing in Fuse. A few of them seemed somewhat obvious, so as with all such issues, I wonder why they were even there. In this post-mortem, I look at a few of the fixes and try to understand how they got released.
Nothing but Android butt
We got a report that specifying LineCap="Round" wasn't working on Android. You'd just always end up with butt caps instead. Such reports always worry me because they sound like device compatibility issues. My brain immediately shifts into workaround mode, with concerns dominating my thoughts as I look through the code.
I find the relevant piece of code:
var strokedPaint = CreateStrokedPaint(stroke.Width * _pixelsPerPoint,
(int)stroke.LineJoin, (int)stroke.LineCap, stroke.LineJoinMiterLimit);
It looks okay. It's a typical bridge between Uno code and the native Java code. We pass in the values for stroke.LineJoin and stroke.LineCap. Hop over to the Java code and what do I see:
paint.setStrokeCap(Paint.Cap.BUTT);
paint.setStrokeJoin(Paint.Join.MITER);
Yup. It was just hard-coded, despite having arguments. A linter option that told me about unused arguments would have caught this.
I don't know if our toolchain supports adding this warning for this back-end. It's also one of those annoying things to add in a mature project: we might go from zero to a million warnings.
But how did we not catch this in manual testing? When I wrote the initial core graphics code, I had tested stroke ends. I then transfer my ad-hoc testing into a test suite used to write the other backends. It turns out we didn't have a test for line caps in there. We do now!
DotNet's desire to be closed
We released a feature that allows drawing partial curves. Given a Path element you can specify a start and length value to draw only part of the SVG path. It's a beautiful feature for animations.
About one day after release, as I prepared to demo it in my live stream, I notice strange lines drawing in preview. For example, when drawing part of a circle, it'd draw a connecting line between the ends!
Again, my head shifts into worrying compatibility mode. Finding the source of the defect didn't help:
case LineSegmentType.Move:
path.CloseFigure();
prevPoint = to;
break;
Why are we closing the figure when the SVG data contains a Move operation? Removing that bit and it fixes it. But why were we doing that? Was it a workaround for some backend issue? Is there some lost test case that requires it? Hopefully, I haven't broken something.
How did I miss this is a better question? It turns out the problem appears only on the DotNet backend. We have three different vector drawing backends: DotNet, Android, and CoreGraphics (iOS/Mac). I did most of my testing on the Mac, and a little bit on iOS/Android. I didn't think about thoroughly testing on all backends as I was just reusing an existing API.
It's possible that we just never had a partial shape before. The defect would happen with solid closed shapes, but since visually the result is the same you can't notice. If you have just a single line in a path, the defect doesn't trigger. It's only with multiple line segments it happens. That just wasn't part of the backend testing.
DotNet strokes are on a diet
While fixing the above issue, I noticed something else odd: an arrowhead on one of my demos wasn't the same width as the attached line.
This time I assumed I made a mistake in my demo. Upon investigation, I realized I didn't. The stroke width on the DotNet back-end wasn't accounting for screen density. That seems like something we would have noticed earlier?
On most Windows machines the pixel-to-point density is 1, whereas Android, iOS, and Mac's have non-zero densities. The Android and iOS/OSX backends took care of density. The testing of DotNet on Windows didn't show a problem.
Preview is part of the tooling for Fuse that let's you see a live preview of your app while working on it. It works on both Windows and Mac, but uses the DotNet backend on both of these platforms. However, native builds on Mac use the CoreGraphics backend, which is shared with iOS.
We had test cases that showed stroke width, but none of them had a point of comparison. Looking at the test, in preview, on Mac, it would look just fine. Only if you have a point of reference would you notice the width is wrong. It's easy to think of a test now that would have caught this, but I'm uncertain what kind of approach could have prevented it in the first place. More attentive code reviews possible -- but usually, we focus on code that is there, not code that isn't.
Conclusion?
It's tempting to say that all of these could have been caught with a bit of improved testing. That's one of those curious things about post-mortems: everything looks obvious in retrospect. At the time we were testing a lot, we even built an assisted test app to help.
We tested our intended use-cases on our primary device targets: Android and iOS. The introduction of a new feature exposed defects in the existing code, but only on specific platforms. With so many potential uses for a feature, there's bound to be problems. The challenge is finding them without over-allocating time and energy to testing.
My work on Fuse is full of interesting coding. Follow me on Twitter or watch my stream for more fun with user interfaces and algorithms.
Top comments (0)