Update style spec for sky properties

[thefifthisa]

This PR updates sky-gradient-center and sky-atmosphere-sun with the appropriate units. Also see inline comment about min/max values.

Closes #10269
Closes #10270

cc @samanpwbb

---

Launch Checklist

• briefly describe the changes in this PR
• include before/after visuals or gifs if this PR includes visual changes
• write tests for all new functionality
• document any changes to public APIs
• post benchmark scores
• manually test the debug page
• tagged @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes
• tagged @mapbox/gl-native if this PR includes shader changes or needs a native port
• apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
• add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog></changelog>

[thefifthisa]

Realizing that polar ranges 0-90 but azimuthal doesn't have the same constraint. Is there a way to specify the min/max of each number in the array, or would it be better to remove this altogether?

[karimnaaji]

The altitude ranges in [0,180] (0 at top/zenith, 180 below) while the azimuth in [0,360] (0 north and increasing clock-wise). I don't think our spec allows to untie these to only accept a range for one of the value but not the other, but I might be wrong. This spec is matching to the light "position" spec, which does not have min and max.

Unless we can assign separate values I'm in favor of removing.

[karimnaaji]

@thefifthisa did you have a path forward for this? We could add validation of multiple min/max values here https://github.com/mapbox/mapbox-gl-js/blob/main/src/style-spec/validate/validate_number.js#L20, if not done in this PR, we can remove these values for the time being and address support for that separately (Otherwise we'd be constraining with an incorrect range).

[thefifthisa]

@karimnaaji thanks and sorry I missed this earlier! I've updated the spec and validation to accept an array for minimum and maximum, and added test cases with the corresponding error messages.

Looks like some integration tests are timing out though, starting from 14c511a. Did I miss something here?

[karimnaaji]

This looks good to me! Thanks for the change @thefifthisa

[karimnaaji]

Looks like some integration tests are timing out though, starting from 14c511a. Did I miss something here?

I'm re-running the render tests as it seems that none failed but still reported a failed result, should be passing in a few.

[karimnaaji]

The render test is actually failing on line-dasharray/unusual-cases/negative-values. I think there's something that breaks this test introduced in this PR. The test is timed out because it does not finish and gets stuck.

[karimnaaji]

The failure in the render test was not being reported appropriately, but it was a useful issue that you fixed with this PR. We were not correctly validating dash arrays such as [1, -2, -1, 1] until now, where only the first value was accounted for the bounds.

The render test line-dasharray/unusual-cases/negative-values was using such values and the spec of dasharray has a limit of a minimum of 0, which wasn't correctly validated until your change; it was only checking that range for the very first value.

I've removed that test and replaced it with a unit test, and also split the validation file in their own separate files.

This change will need to be ported to native, cc @mapbox/gl-native .

[thefifthisa]

Thanks @karimnaaji!