Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
- Eliminate Math.log2 (not available on ie11)
- Inline and compress evaluation interpolations for line clip by
specializing it for the use case

Thanks @mourner
  • Loading branch information
karimnaaji committed Jun 3, 2020
1 parent add4892 commit d7acc11
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 41 deletions.
6 changes: 1 addition & 5 deletions src/shaders/line_gradient.fragment.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 3 additions & 5 deletions src/util/color_ramp.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
}
}
Expand Down
20 changes: 2 additions & 18 deletions src/util/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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));
}

/**
Expand Down
14 changes: 1 addition & 13 deletions test/unit/util/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit d7acc11

Please sign in to comment.