Colors in feature attributes are misinterpreted when used in key path expressions

[1ec5]

NSColor and UIColor objects used as attribute values of MGLFeature are no longer automatically premultiplied the correct number of times, so an MGLFillStyleLayer.fillColor expression based on a feature attribute would render features with the wrong fill color.

Expected behavior

For convenience’s sake, MGLFeature automatically converts attribute values of type NSColor (on macOS) or UIColor (iOS) to a CSS-formatted rgba() string, which happens to be compatible with color-typed paint properties in the style specification:

In addition to the Foundation types, you may also set an attribute to an NSColor (macOS) or UIColor (iOS), which will be converted into its CSS string representation when the feature is added to an MGLShapeSource. This can be convenient when using the attribute to supply a value for a color-typed layout or paint attribute via a key path expression.

The developer would expect this conversion to be accurate, so that the result of setting the MGLFillStyleLayer.fillColor property would match the normal appearance of the NSColor or UIColor that was stored in the feature attribute. For example, the following code should result in a translucent, light blue fill:

let highlightFeature = MGLPolygonFeature(coordinates: coordinates, count: UInt(points.count))
highlightFeature.attributes["fill"] = UIColor(red: 145.0 / 255.0, green: 174.0 / 255.0, blue: 255.0 / 255.0, alpha: 0.5)
let highlightSource = MGLShapeSource(identifier: "highlight", shape: highlightFeature, options: nil)
style.addSource(highlightSource)

let highlightLayer = MGLFillStyleLayer(identifier: "highlight", source: highlightSource)
highlightLayer.fillColor = NSExpression(forKeyPath: "fill")
style.addLayer(highlightLayer)

Actual behavior

The code actually results in a drab gray fill.

Diagnosis

The convenience conversion works by stuffing the NSColor or UIColor in a constant value expression and converting the expression to a JSON-formatted object:

mapbox-gl-native-ios/platform/darwin/src/MGLFeature.mm

Line 539 in 6a5db0b

feature.properties = [attributes mgl_propertyMap];

mapbox-gl-native-ios/platform/darwin/src/NSDictionary+MGLAdditions.mm

Lines 17 to 18 in 6a5db0b

	NSExpression *expression = [NSExpression expressionForConstantValue:self[key]];
	propertyMap[[key UTF8String]] = expression.mgl_constantMBGLValue;

The conversion to JSON takes the four color channels as is without premultiplying the alpha channel into the red, green, and blue channels:

mapbox-gl-native-ios/platform/darwin/src/NSExpression+MGLAdditions.mm

Lines 320 to 322 in 6a5db0b

	} else if ([value isKindOfClass:[MGLColor class]]) {
	auto hexString = [(MGLColor *)value mgl_colorForPremultipliedValue].stringify();
	return { hexString };

mapbox-gl-native-ios/platform/ios/src/UIColor+MGLAdditions.mm

Lines 15 to 21 in 6a5db0b

	// Intended to be used when providing colors on the basis of an NSExpression
	- (mbgl::Color)mgl_colorForPremultipliedValue
	{
	CGFloat r, g, b, a;
	[self getRed:&r green:&g blue:&b alpha:&a];
	return { static_cast<float>(r), static_cast<float>(g), static_cast<float>(b), static_cast<float>(a) };
	}

From mapbox/mapbox-gl-native#8025 until #266, this conversion from NSColor/UIColor to JSON premultiplied the color, based on a historical expectation that mbgl::Color was always premultiplied:

mapbox-gl-native-ios/platform/darwin/src/NSExpression+MGLAdditions.mm

Lines 310 to 312 in 23b5b46

	} else if ([value isKindOfClass:[MGLColor class]]) {
	auto hexString = [(MGLColor *)value mgl_color].stringify();
	return { hexString };

mapbox-gl-native-ios/platform/ios/src/UIColor+MGLAdditions.mm

Lines 5 to 13 in 6a5db0b

	- (mbgl::Color)mgl_color
	{
	CGFloat r, g, b, a;
	[self getRed:&r green:&g blue:&b alpha:&a];
	// UIColor provides non-premultiplied color components, so we have to premultiply each
	// color component with the alpha value to transform it into a valid
	// mbgl::Color which expects premultiplied color components.
	return { static_cast<float>(r*a), static_cast<float>(g*a), static_cast<float>(b*a), static_cast<float>(a) };
	}

Even if the change in #266 was correct in the usual case when converting expressions from NSExpression to JSON format, for code that uses the feature attribute convenience conversion, the change affected it twice over.

Workaround

A workaround is to circumvent the any color-related conversions in the platform-specific SDK code by specifying a CSS-style rgba(145.0, 174.0, 255.0, 0.5) string directly as the attribute value:

let highlightColor = UIColor(red: 145.0 / 255.0, green: 174.0 / 255.0, blue: 255.0 / 255.0, alpha: 0.5)
let red = UnsafeMutablePointer<CGFloat>.allocate(capacity: 1)
let green = UnsafeMutablePointer<CGFloat>.allocate(capacity: 1)
let blue = UnsafeMutablePointer<CGFloat>.allocate(capacity: 1)
let alpha = UnsafeMutablePointer<CGFloat>.allocate(capacity: 1)
highlightColor.getRed(red, green: green, blue: blue, alpha: alpha)
highlightFeature.attributes["fill"] = "rgba(\(red.pointee * 255.0), \(green.pointee * 255.0), \(blue.pointee * 255.0), \(alpha.pointee))"

Performance-wise, this should be equivalent to the convenience conversion provided by the SDK, or even more performant in the case that the feature is reused multiple times in the same shape source or by multiple shape sources.

/cc @mapbox/maps-ios

[1ec5]

Per #266 (comment), we should also verify that shape and line annotations are still appearing correctly.

[lloydsheng]

@1ec5 Can you see if #536 is the right way to solve this issue?

/cc @knov @julianrex