search

mapbox / mapbox-scenekit

Other
231 stars 51 forks source link

AddPolyline draws wrong color #54

Closed natalia-osa closed 5 years ago

natalia-osa commented 5 years ago

A minor problem is the fact only 1 method is iOS 8-9 compatible, and that method doesn't contain markers for start/end points and as a result, I had to copy paste my code from my pr to add those start/end dots (why not to expose it here if the fallback is available anyway as far as I read the PR?).

The bigger issue is that the colours do not match. They are very pale. Please look at the line colour in the attached screenshot, the dots which I create by the usual method are of the proper colour, and the colour I get by using your drawPath method is pastel. Because I draw start/end dots, it's so badly visible.

image from ios

natalia-osa commented 5 years ago

Ok, so you can easily replicate it by drawing a route in AR. In DemoPlacement, based on SceneKit only, the colours are perfect. In DemoAR, if you add polyline with a given colour, it is washed out, pastel.

jim-martin commented 5 years ago

@natalia-osa , could you share the input colors you're using for addPolyline? does this appear when using UIColor.red?

natalia-osa commented 5 years ago

That's weird, for me completely any colour (I didn't try uikit basic ones) works like this. Please remember it happens only in AR, not in scenekit only (so on a real device, I'm testing on iOS 11, iPhone SE). I tested in your example project, it happens as well.

Examples of colors which do not work:

UIColor(red: 114/255, green: 13/255, blue: 74/255, alpha: 1)
UIColor(red: 69/255, green: 8/255, blue: 46/255, alpha: 1)
jim-martin commented 5 years ago

Looks like it was a color space issue, thanks for the ticket @natalia-osa!

This shader still doesn't respond to lighting in the same way that the default PBR materials do, so lights in the scene still might cause a color difference. To keep things the same between line/cap in the meantime, you could apply a .consistent lightingModel to your cap nodes.