Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

Commit

Permalink
HARP-17373: Fix shader compilation with Metal in Safari 15 on MacOS M…
Browse files Browse the repository at this point in the history
…onterrey.

Shader compilation errors were caused by the uniform names 'diffuse' and
'bitangent'.

Signed-off-by: Andres Mandado <andres.mandado-almajano@here.com>
  • Loading branch information
atomicsulfate committed Nov 23, 2021
1 parent 9e9d58e commit 19fe4f9
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 34 deletions.
8 changes: 6 additions & 2 deletions @here/harp-lines/lib/Lines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ const LINE_VERTEX_ATTRIBUTES: VertexDescriptor = {
{ name: "extrusionCoord", itemSize: 3, offset: 0 },
{ name: "position", itemSize: 3, offset: 3 },
{ name: "tangent", itemSize: 3, offset: 6 },
{ name: "bitangent", itemSize: 4, offset: 9 }
// HARP-17373: Original uniform name 'bitangent' due to shader compilation errors with Metal
// in Safari 15 on MacOS Monterrey and iPadOS 15.
{ name: "biTangent", itemSize: 4, offset: 9 }
],
stride: 13
};
Expand Down Expand Up @@ -74,7 +76,9 @@ const HP_LINE_VERTEX_ATTRIBUTES: VertexDescriptor = {
{ name: "position", itemSize: 3, offset: 2 },
{ name: "positionLow", itemSize: 3, offset: 5 },
{ name: "tangent", itemSize: 3, offset: 8 },
{ name: "bitangent", itemSize: 4, offset: 11 }
// HARP-17373: Original uniform name 'bitangent' due to shader compilation errors with Metal
// in Safari 15 on MacOS Monterrey and iPadOS 15.
{ name: "biTangent", itemSize: 4, offset: 11 }
],
stride: 15
};
Expand Down
4 changes: 2 additions & 2 deletions @here/harp-mapview/lib/geometry/SolidLineMesh.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020 HERE Europe B.V.
* Copyright (C) 2020-2021 HERE Europe B.V.
* Licensed under Apache 2.0, see full license in LICENSE
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -279,7 +279,7 @@ function intersectFeature(
const geometry = mesh.geometry as THREE.BufferGeometry;
const attributes = geometry.attributes;
const position = attributes.position as THREE.BufferAttribute;
const bitangent = attributes.bitangent;
const bitangent = attributes.biTangent;
const indices = geometry.index!.array;

tmpSphere.copy(bSphere);
Expand Down
2 changes: 1 addition & 1 deletion @here/harp-mapview/lib/geometry/TileGeometry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export abstract class BufferedGeometryAccessorBase implements IGeometryAccessor
const rawShaderMaterial = material as THREE.RawShaderMaterial;

if (rawShaderMaterial.name === "SolidLineMaterial") {
return rawShaderMaterial.uniforms.diffuse.value as THREE.Color;
return rawShaderMaterial.uniforms.diffuseColor.value as THREE.Color;
}

logger.warn(
Expand Down
4 changes: 2 additions & 2 deletions @here/harp-mapview/test/SolidLineMeshTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class BufferGeometryBuilder {
}

withBitangent(values: number[]): BufferGeometryBuilder {
return this.withAttribute("bitangent", values, 3);
return this.withAttribute("biTangent", values, 3);
}

withUv(values: number[]): BufferGeometryBuilder {
Expand Down Expand Up @@ -148,7 +148,7 @@ class SolidLineGeometryBuilder extends BufferGeometryBuilder {

const oldPositions = this.m_attributes.position.array as Float32Array;
const oldIndices = this.indices!.array as Uint32Array;
const oldBitangents = this.m_attributes.bitangent.array as Float32Array;
const oldBitangents = this.m_attributes.biTangent.array as Float32Array;
const indices = SolidLineGeometryBuilder.triangulateLine(polyline, oldPositions.length / 3);

this.withPosition(Array.from(oldPositions).concat(positions));
Expand Down
12 changes: 7 additions & 5 deletions @here/harp-materials/lib/CirclePointsMaterial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const fragmentShader: string = `
precision highp float;
precision highp int;
uniform vec3 diffuse;
uniform vec3 diffuseColor;
uniform float opacity;
void main() {
Expand All @@ -46,7 +46,7 @@ void main() {
float threshold = 1.0 - smoothstep(radius - falloff, radius, len);
alpha *= threshold;
gl_FragColor = vec4(diffuse, alpha);
gl_FragColor = vec4(diffuseColor, alpha);
}`;

/**
Expand Down Expand Up @@ -96,7 +96,9 @@ export class CirclePointsMaterial extends RawShaderMaterial {
shaderParams.uniforms = THREE.UniformsUtils.merge([
{
size: new THREE.Uniform(CirclePointsMaterial.DEFAULT_CIRCLE_SIZE),
diffuse: new THREE.Uniform(defaultColor),
// HARP-17373: Original uniform name 'diffuse' due to shader compilation
// errors with Metal in Safari 15 on MacOS Monterrey and iPadOS 15.
diffuseColor: new THREE.Uniform(defaultColor),
opacity: new THREE.Uniform(defaultOpacity)
},
THREE.UniformsLib.fog
Expand Down Expand Up @@ -142,10 +144,10 @@ export class CirclePointsMaterial extends RawShaderMaterial {
}

get color(): THREE.Color {
return this.uniforms.diffuse.value as THREE.Color;
return this.uniforms.diffuseColor.value as THREE.Color;
}

set color(value: THREE.Color) {
this.uniforms.diffuse.value.copy(value);
this.uniforms.diffuseColor.value.copy(value);
}
}
14 changes: 8 additions & 6 deletions @here/harp-materials/lib/HighPrecisionLineMaterial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const fragmentSource: string = `
precision highp float;
precision highp int;
uniform vec3 diffuse;
uniform vec3 diffuseColor;
uniform float opacity;
#ifdef USE_COLOR
Expand All @@ -48,9 +48,9 @@ varying vec3 color;
void main() {
#ifdef USE_COLOR
gl_FragColor = vec4( diffuse * vColor, opacity );
gl_FragColor = vec4( diffuseColor * vColor, opacity );
#else
gl_FragColor = vec4( diffuse, opacity );
gl_FragColor = vec4( diffuseColor, opacity );
#endif
}`;

Expand Down Expand Up @@ -92,7 +92,9 @@ export class HighPrecisionLineMaterial extends RawShaderMaterial {
vertexShader: vertexSource,
fragmentShader: fragmentSource,
uniforms: {
diffuse: new THREE.Uniform(
// HARP-17373: Original uniform name 'diffuse' due to shader compilation
// errors with Metal in Safari 15 on MacOS Monterrey and iPadOS 15.
diffuseColor: new THREE.Uniform(
new THREE.Color(HighPrecisionLineMaterial.DEFAULT_COLOR)
),
opacity: new THREE.Uniform(HighPrecisionLineMaterial.DEFAULT_OPACITY),
Expand Down Expand Up @@ -126,11 +128,11 @@ export class HighPrecisionLineMaterial extends RawShaderMaterial {
* Line color.
*/
get color(): THREE.Color {
return this.uniforms.diffuse.value as THREE.Color;
return this.uniforms.diffuseColor.value as THREE.Color;
}

set color(value: THREE.Color) {
this.uniforms.diffuse.value.copy(value);
this.uniforms.diffuseColor.value.copy(value);
}

private updateTransparencyFeature() {
Expand Down
6 changes: 5 additions & 1 deletion @here/harp-materials/lib/HighPrecisionPointMaterial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,11 @@ export class HighPrecisionPointMaterial extends THREE.PointsMaterial {
this.fog = false;

this.uniforms = {
diffuse: new THREE.Uniform(new THREE.Color(HighPrecisionPointMaterial.DEFAULT_COLOR)),
// HARP-17373: Original uniform name 'diffuse' due to shader compilation
// errors with Metal in Safari 15 on MacOS Monterrey and iPadOS 15.
diffuseColor: new THREE.Uniform(
new THREE.Color(HighPrecisionPointMaterial.DEFAULT_COLOR)
),
opacity: new THREE.Uniform(HighPrecisionPointMaterial.DEFAULT_OPACITY),
size: new THREE.Uniform(HighPrecisionPointMaterial.DEFAULT_SIZE),
scale: new THREE.Uniform(HighPrecisionPointMaterial.DEFAULT_SCALE),
Expand Down
26 changes: 15 additions & 11 deletions @here/harp-materials/lib/SolidLineMaterial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const vertexSource: string = `
attribute vec3 extrusionCoord;
attribute vec3 position;
attribute vec4 bitangent;
attribute vec4 biTangent;
attribute vec3 tangent;
attribute vec2 uv;
attribute vec3 normal;
Expand Down Expand Up @@ -120,12 +120,12 @@ void main() {
float linePos = mix(segment.x, segment.y, segmentPos);
vec2 extrusionDir = sign(extrusionCoord.xy);
// Precompute to avoid computing multiple times
float tanHalfAngle = tan(bitangent.w / 2.0);
float tanHalfAngle = tan(biTangent.w / 2.0);
float extrusionFactor = extrusionDir.y * tanHalfAngle;
// Calculate the extruded vertex position (and scale the extrusion direction).
vec3 pos = extrudeLine(
position, linePos, extrusionWidth + outlineWidth, bitangent, tangent, tanHalfAngle,
position, linePos, extrusionWidth + outlineWidth, biTangent, tangent, tanHalfAngle,
extrusionDir);
// Store the normalized extrusion coordinates in vCoords (with their ranges in vRange).
Expand Down Expand Up @@ -155,7 +155,7 @@ void main() {
// Note, we need to take the angle into consideration, so we use trigonometry to calculate how
// much we need to extend the offset. Note, orthough this looks complicated we are doing this
// in the vertex shader, so it should not cause a performance issue.
pos += bitangent.xyz * offset * sqrt(1.0 + pow(abs(tanHalfAngle), 2.0));
pos += biTangent.xyz * offset * sqrt(1.0 + pow(abs(tanHalfAngle), 2.0));
vec4 mvPosition = modelViewMatrix * vec4(pos, 1.0);
gl_Position = projectionMatrix * mvPosition;
Expand All @@ -181,7 +181,7 @@ const fragmentSource: string = `
precision highp float;
precision highp int;
uniform vec3 diffuse;
uniform vec3 diffuseColor;
uniform vec3 outlineColor;
uniform float opacity;
uniform float extrusionWidth;
Expand Down Expand Up @@ -220,7 +220,7 @@ varying vec3 vColor;
void main() {
float alpha = opacity;
vec3 outputDiffuse = diffuse;
vec3 outputDiffuse = diffuseColor;
#ifdef USE_TILE_CLIP
tileClip(vPosition.xy, tileSize);
Expand Down Expand Up @@ -264,7 +264,7 @@ void main() {
float dashBlendFactor = 1.0 - smoothstep(-dashWidth, dashWidth, distToDashEdge);
#ifdef USE_DASH_COLOR
outputDiffuse = mix(diffuse, dashColor, dashBlendFactor);
outputDiffuse = mix(diffuseColor, dashColor, dashBlendFactor);
#endif
#endif
Expand All @@ -280,7 +280,7 @@ void main() {
float colorBlendFactor = smoothstep(-1.0, 1.0, dashBlendFactor - outlineBlendFactor);
outputDiffuse = mix(
mix(
mix(outlineColor, diffuse, colorBlendFactor),
mix(outlineColor, diffuseColor, colorBlendFactor),
outputDiffuse,
dashBlendFactor
),
Expand Down Expand Up @@ -312,6 +312,8 @@ void main() {
#ifdef USE_FADING
#include <fading_fragment>
#endif
}`;

/**
Expand Down Expand Up @@ -467,7 +469,9 @@ export class SolidLineMaterial
fragmentShader: fragmentSource,
uniforms: THREE.UniformsUtils.merge([
{
diffuse: new THREE.Uniform(
// HARP-17373: Original uniform name 'diffuse' due to shader compilation
// errors with Metal in Safari 15 on MacOS Monterrey and iPadOS 15.
diffuseColor: new THREE.Uniform(
new THREE.Color(SolidLineMaterial.DEFAULT_COLOR)
),
dashColor: new THREE.Uniform(
Expand Down Expand Up @@ -631,11 +635,11 @@ export class SolidLineMaterial
* Line color.
*/
get color(): THREE.Color {
return this.uniforms.diffuse.value as THREE.Color;
return this.uniforms.diffuseColor.value as THREE.Color;
}

set color(value: THREE.Color) {
this.uniforms.diffuse.value.copy(value);
this.uniforms.diffuseColor.value.copy(value);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions @here/harp-materials/test/CirclePointsMaterialTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ describe("CirclePointsMaterial", function () {
material.color = new THREE.Color(0xfefefe);

expect(material.color.getHex()).to.equal(0xfefefe);
expect(material.uniforms.diffuse.value.getHex()).to.equal(0xfefefe);
expect(material.uniforms.diffuse.value).to.equal(material.color);
expect(material.uniforms.diffuseColor.value.getHex()).to.equal(0xfefefe);
expect(material.uniforms.diffuseColor.value).to.equal(material.color);
});

it("updates color with set", function () {
Expand All @@ -96,8 +96,8 @@ describe("CirclePointsMaterial", function () {
material.color.set(0xfefefe);

expect(material.color.getHex()).to.equal(0xfefefe);
expect(material.uniforms.diffuse.value.getHex()).to.equal(0xfefefe);
expect(material.uniforms.diffuse.value).to.equal(material.color);
expect(material.uniforms.diffuseColor.value.getHex()).to.equal(0xfefefe);
expect(material.uniforms.diffuseColor.value).to.equal(material.color);
});
});
});

0 comments on commit 19fe4f9

Please sign in to comment.