From d7acc1186138a6da179f62f2764135904128efa4 Mon Sep 17 00:00:00 2001
From: Karim Naaji <karim.naaji@gmail.com>
Date: Wed, 3 Jun 2020 15:14:14 -0700
Subject: [PATCH] Review comments

- Eliminate Math.log2 (not available on ie11)
- Inline and compress evaluation interpolations for line clip by
specializing it for the use case

Thanks @mourner
---
 src/shaders/line_gradient.fragment.glsl |  6 +-----
 src/util/color_ramp.js                  |  8 +++-----
 src/util/util.js                        | 20 ++------------------
 test/unit/util/util.test.js             | 14 +-------------
 4 files changed, 7 insertions(+), 41 deletions(-)

diff --git a/src/shaders/line_gradient.fragment.glsl b/src/shaders/line_gradient.fragment.glsl
index 84beffb410d..d9820c74af2 100644
--- a/src/shaders/line_gradient.fragment.glsl
+++ b/src/shaders/line_gradient.fragment.glsl
@@ -12,10 +12,6 @@ varying highp float v_split_index;
 #pragma mapbox: define lowp float blur
 #pragma mapbox: define lowp float opacity
 
-highp float map(highp float value, highp float start, highp float end, highp float new_start, highp float new_end) {
-    return ((value - start) * (new_end - new_start)) / (end - start) + new_start;
-}
-
 void main() {
     #pragma mapbox: initialize lowp float blur
     #pragma mapbox: initialize lowp float opacity
@@ -32,7 +28,7 @@ void main() {
     highp float texel_height = 1.0 / u_image_height;
     highp float half_texel_height = 0.5 * texel_height;
     highp vec2 uv = vec2(
-        map(v_lineprogress, 0.0, v_line_clip, 0.0, 1.0),
+        v_lineprogress / v_line_clip,
         v_split_index * texel_height - half_texel_height);
 
     // For gradient lines, v_lineprogress is the ratio along the
diff --git a/src/util/color_ramp.js b/src/util/color_ramp.js
index 0ea20617284..24dc0758699 100644
--- a/src/util/color_ramp.js
+++ b/src/util/color_ramp.js
@@ -1,7 +1,7 @@
 // @flow
 
 import {RGBAImage} from './image';
-import {isPowerOfTwo, mapValue} from './util';
+import {isPowerOfTwo} from './util';
 import assert from 'assert';
 
 import type {StylePropertyExpression} from '../style-spec/expression/index';
@@ -50,10 +50,8 @@ export function renderColorRamp(params: ColorRampParams): RGBAImage {
             for (let i = 0, j = 0; i < width; i++, j += 4) {
                 // Remap progress between clips
                 const progress = i / (width - 1);
-                const evaluationProgress = mapValue(progress, 0.0, 1.0,
-                    params.clips[clip].start,
-                    params.clips[clip].end);
-
+                const {start, end} = params.clips[clip];
+                const evaluationProgress = start * (1 - progress) + end * progress;
                 renderPixel(stride, j, evaluationProgress);
             }
         }
diff --git a/src/util/util.js b/src/util/util.js
index 90f4ff02ed1..f5e3d55c58a 100644
--- a/src/util/util.js
+++ b/src/util/util.js
@@ -217,7 +217,7 @@ export function uuid(): string {
  * @private
  */
 export function isPowerOfTwo(value: number): boolean {
-    return Math.log2(value) % 1 === 0;
+    return (Math.log(value) / Math.LN2) % 1 === 0;
 }
 
 /**
@@ -226,23 +226,7 @@ export function isPowerOfTwo(value: number): boolean {
  */
 export function nextPowerOfTwo(value: number): number {
     if (value <= 1) return 1;
-    return Math.pow(2, Math.ceil(Math.log2(value)));
-}
-
-/**
- * Remaps a value from one range to another
- * @param value the value to remap
- * @param start the start of previous range
- * @param end the end of previous range
- * @param newStart the new starting range
- * @param newEnd the new ending range
- * @private
- */
-export function mapValue(value: number, start: number, end: number, newStart: number, newEnd: number): number {
-    if (end - start === 0.0) {
-        return 0.0;
-    }
-    return ((value - start) * (newEnd - newStart)) / (end - start) + newStart;
+    return Math.pow(2, Math.ceil(Math.log(value) / Math.LN2));
 }
 
 /**
diff --git a/test/unit/util/util.test.js b/test/unit/util/util.test.js
index 00e9543bedb..bad799c54bb 100644
--- a/test/unit/util/util.test.js
+++ b/test/unit/util/util.test.js
@@ -2,7 +2,7 @@
 
 import {test} from '../../util/test';
 
-import {easeCubicInOut, mapValue, keysDifference, extend, pick, uniqueId, bindAll, asyncAll, clamp, wrap, bezier, endsWith, mapObject, filterObject, deepEqual, clone, arraysIntersect, isCounterClockwise, isClosedPolygon, parseCacheControl, uuid, validateUuid, nextPowerOfTwo, isPowerOfTwo} from '../../../src/util/util';
+import {easeCubicInOut, keysDifference, extend, pick, uniqueId, bindAll, asyncAll, clamp, wrap, bezier, endsWith, mapObject, filterObject, deepEqual, clone, arraysIntersect, isCounterClockwise, isClosedPolygon, parseCacheControl, uuid, validateUuid, nextPowerOfTwo, isPowerOfTwo} from '../../../src/util/util';
 import Point from '@mapbox/point-geometry';
 
 test('util', (t) => {
@@ -107,18 +107,6 @@ test('util', (t) => {
         t.end();
     });
 
-    t.test('mapValue', (t) => {
-        t.equal(mapValue(0.0, 0.0, 1.0, 1.0, 2.0), 1.0);
-        t.equal(mapValue(1.0, 0.0, 2.0, 10.0, 20.0), 15.0);
-        t.equal(mapValue(5.0, 0.0, 10.0, 50.0, 100.0), 75.0);
-        t.equal(mapValue(0.0, -1.0, 1.0, 2.0, 3.0), 2.5);
-        t.equal(mapValue(-15.0, -10.0, -20.0, 10.0, 20.0), 15.0);
-        t.equal(mapValue(0.6, 0.0, 1.0, 1.0, 0.0), 0.4);
-        t.equal(mapValue(0.0, 0.0, 1.0, 0.0, 0.0), 0.0);
-        t.equal(mapValue(0.0, 0.0, 0.0, 0.0, 1.0), 0.0);
-        t.end();
-    });
-
     t.test('clamp', (t) => {
         t.equal(clamp(0, 0, 1), 0);
         t.equal(clamp(1, 0, 1), 1);