From e0c9b7b91f8bd9ccb135b3af86a032d46d1d9b4b Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 6 May 2025 08:37:05 +0200 Subject: [PATCH 1/6] fix(16834): Fix issue in parseImageOffsetProperties offsetExpression using the wrong expected type - generating invalid fragment shader --- src/ol/render/webgl/style.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/render/webgl/style.js b/src/ol/render/webgl/style.js index e9034427f8a..222a901e50d 100644 --- a/src/ol/render/webgl/style.js +++ b/src/ol/render/webgl/style.js @@ -184,7 +184,7 @@ function parseImageOffsetProperties( let offsetExpression = expressionToGlsl( context, style[`${prefix}offset`], - NumberArrayType, + SizeType, ); if (`${prefix}offset-origin` in style) { switch (style[`${prefix}offset-origin`]) { From 097b7a5c14a6748db9cbb74543d66fc15cba199f Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 6 May 2025 10:04:57 +0200 Subject: [PATCH 2/6] test: Add test for fill pattern with dynamic offset --- .../spec/ol/render/webgl/style.test.js | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/browser/spec/ol/render/webgl/style.test.js b/test/browser/spec/ol/render/webgl/style.test.js index 4dd88279ca9..3e57ae06c9f 100644 --- a/test/browser/spec/ol/render/webgl/style.test.js +++ b/test/browser/spec/ol/render/webgl/style.test.js @@ -941,6 +941,63 @@ describe('ol/render/webgl/style', () => { `vec4(1.0, 0.0, 0.0, 1.0) * sampleFillPattern(u_texture${uid}, u_texture${uid}_size, vec2(0., u_texture${uid}_size.y) + vec2(5.0, 5.0) * vec2(0., -1.) + vec2(5.0, 10.0) * vec2(1., -1.), vec2(5.0, 5.0), pxOrigin, pxPos)`, ); }); + }); + + describe('fill pattern, dynamic offset', () => { + let result, uid; + + beforeEach(() => { + const style = { + 'fill-color': 'red', + 'fill-pattern-src': + 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==', + 'fill-pattern-offset': ['get', 'patternOffset'], + 'fill-pattern-size': [5, 5], + }; + uid = computeHash(style['fill-pattern-src']); + result = parseLiteralStyle(style); + }); + + it('includes pattern sampling function in the shader', () => { + expect(result.builder.fragmentShaderFunctions_[0]).to.contain( + 'vec4 sampleFillPattern' + ); + }); + + it('registers a vec2 pattern offset attribute', () => { + const patternOffsetAttr = result.builder.attributes_.find(attr => + attr.name === 'a_prop_patternOffset'); + + expect(patternOffsetAttr).not.to.be(undefined); + expect(patternOffsetAttr.type).to.be('vec2'); + expect(patternOffsetAttr.varyingName).to.be('v_prop_patternOffset'); + expect(patternOffsetAttr.varyingType).to.be('vec2'); + expect(patternOffsetAttr.varyingExpression).to.be('a_prop_patternOffset'); + }); + + it('applies pattern offset in the fill color expression', () => { + expect(result.builder.fillColorExpression_).to.contain('a_prop_patternOffset'); + }); + + it('registers pattern offset as a size-2 attribute', () => { + expect(Object.keys(result.attributes)).to.contain('prop_patternOffset'); + expect(result.attributes['prop_patternOffset'].size).to.eql(2); + }); + + it('extracts pattern offset values correctly from features', () => { + const callback = result.attributes['prop_patternOffset'].callback; + const feature = new Feature({ + patternOffset: [15, 25] + }); + expect(callback(feature)).to.eql([15, 25]); + }); + + it('handles missing pattern offset gracefully', () => { + const callback = result.attributes['prop_patternOffset'].callback; + const feature = new Feature({}); + // Should return undefined or default value depending on implementation + expect(callback(feature)).to.eql(undefined); + }); }); }); From f3c66a897d8b867cec3374a931d45eb0b173e890 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 6 May 2025 10:07:37 +0200 Subject: [PATCH 3/6] chore: Remove comment --- test/browser/spec/ol/render/webgl/style.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/browser/spec/ol/render/webgl/style.test.js b/test/browser/spec/ol/render/webgl/style.test.js index 3e57ae06c9f..883dbdb53ab 100644 --- a/test/browser/spec/ol/render/webgl/style.test.js +++ b/test/browser/spec/ol/render/webgl/style.test.js @@ -995,7 +995,6 @@ describe('ol/render/webgl/style', () => { it('handles missing pattern offset gracefully', () => { const callback = result.attributes['prop_patternOffset'].callback; const feature = new Feature({}); - // Should return undefined or default value depending on implementation expect(callback(feature)).to.eql(undefined); }); }); From 7db5e840b88df3701d14a480cc70d8d853b25b94 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 6 May 2025 10:22:27 +0200 Subject: [PATCH 4/6] chore: Fix lint formatting --- .../spec/ol/render/webgl/style.test.js | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/test/browser/spec/ol/render/webgl/style.test.js b/test/browser/spec/ol/render/webgl/style.test.js index 883dbdb53ab..62776631acf 100644 --- a/test/browser/spec/ol/render/webgl/style.test.js +++ b/test/browser/spec/ol/render/webgl/style.test.js @@ -941,11 +941,11 @@ describe('ol/render/webgl/style', () => { `vec4(1.0, 0.0, 0.0, 1.0) * sampleFillPattern(u_texture${uid}, u_texture${uid}_size, vec2(0., u_texture${uid}_size.y) + vec2(5.0, 5.0) * vec2(0., -1.) + vec2(5.0, 10.0) * vec2(1., -1.), vec2(5.0, 5.0), pxOrigin, pxPos)`, ); }); - }); - + }); + describe('fill pattern, dynamic offset', () => { - let result, uid; - + let result; + beforeEach(() => { const style = { 'fill-color': 'red', @@ -954,44 +954,50 @@ describe('ol/render/webgl/style', () => { 'fill-pattern-offset': ['get', 'patternOffset'], 'fill-pattern-size': [5, 5], }; - uid = computeHash(style['fill-pattern-src']); result = parseLiteralStyle(style); }); - + it('includes pattern sampling function in the shader', () => { expect(result.builder.fragmentShaderFunctions_[0]).to.contain( - 'vec4 sampleFillPattern' + 'vec4 sampleFillPattern', ); }); - + it('registers a vec2 pattern offset attribute', () => { - const patternOffsetAttr = result.builder.attributes_.find(attr => - attr.name === 'a_prop_patternOffset'); - + const patternOffsetAttr = result.builder.attributes_.find( + (attr) => attr.name === 'a_prop_patternOffset', + ); + expect(patternOffsetAttr).not.to.be(undefined); expect(patternOffsetAttr.type).to.be('vec2'); expect(patternOffsetAttr.varyingName).to.be('v_prop_patternOffset'); expect(patternOffsetAttr.varyingType).to.be('vec2'); - expect(patternOffsetAttr.varyingExpression).to.be('a_prop_patternOffset'); + expect(patternOffsetAttr.varyingExpression).to.be( + 'a_prop_patternOffset', + ); }); - + it('applies pattern offset in the fill color expression', () => { - expect(result.builder.fillColorExpression_).to.contain('a_prop_patternOffset'); + expect(result.builder.fillColorExpression_).to.contain( + 'a_prop_patternOffset', + ); }); - + it('registers pattern offset as a size-2 attribute', () => { - expect(Object.keys(result.attributes)).to.contain('prop_patternOffset'); + expect(Object.keys(result.attributes)).to.contain( + 'prop_patternOffset', + ); expect(result.attributes['prop_patternOffset'].size).to.eql(2); }); - + it('extracts pattern offset values correctly from features', () => { const callback = result.attributes['prop_patternOffset'].callback; const feature = new Feature({ - patternOffset: [15, 25] + patternOffset: [15, 25], }); expect(callback(feature)).to.eql([15, 25]); }); - + it('handles missing pattern offset gracefully', () => { const callback = result.attributes['prop_patternOffset'].callback; const feature = new Feature({}); From 0a86afd59431422e4b0254cf154406be6c4ab9d5 Mon Sep 17 00:00:00 2001 From: Olivia Guyot Date: Mon, 5 May 2025 12:12:41 +0200 Subject: [PATCH 5/6] WebGL ShaderBuilder / handle uniforms like attributes, improve doc This is mainly for consistency and readability --- src/ol/layer/Heatmap.js | 4 +- src/ol/render/webgl/ShaderBuilder.js | 56 +++++++++++-------- src/ol/render/webgl/compileUtil.js | 2 +- src/ol/render/webgl/style.js | 4 +- src/ol/renderer/webgl/VectorTileLayer.js | 6 +- .../spec/ol/render/webgl/compileUtil.test.js | 4 +- .../ol/render/webgl/shaderbuilder.test.js | 12 ++-- .../spec/ol/render/webgl/style.test.js | 34 +++++------ .../ol/renderer/webgl/VectorTileLayer.test.js | 12 ++-- 9 files changed, 76 insertions(+), 58 deletions(-) diff --git a/src/ol/layer/Heatmap.js b/src/ol/layer/Heatmap.js index 0d1a42edf27..1743374525a 100644 --- a/src/ol/layer/Heatmap.js +++ b/src/ol/layer/Heatmap.js @@ -261,12 +261,12 @@ class Heatmap extends BaseVector { if (typeof this.getBlur() === 'number') { blurCompiled = 'a_blur'; blurRadiusUniforms['a_blur'] = () => this.getBlur(); - builder.addUniform('float a_blur'); + builder.addUniform('a_blur', 'float'); } if (typeof this.getRadius() === 'number') { radiusCompiled = 'a_radius'; blurRadiusUniforms['a_radius'] = () => this.getRadius(); - builder.addUniform('float a_radius'); + builder.addUniform('a_radius', 'float'); } /** @type {import('../render/webgl/VectorStyleRenderer.js').AttributeDefinitions} */ diff --git a/src/ol/render/webgl/ShaderBuilder.js b/src/ol/render/webgl/ShaderBuilder.js index 8d8f540b983..3a96ecaf2e6 100644 --- a/src/ol/render/webgl/ShaderBuilder.js +++ b/src/ol/render/webgl/ShaderBuilder.js @@ -34,11 +34,17 @@ const DEFAULT_STYLE = createDefaultStyle(); /** * @typedef {Object} AttributeDescription - * @property {string} name Attribute name, as will be declared in the headers (including a_) - * @property {string} type Attribute type, either `float`, `vec2`, `vec4`... - * @property {string} varyingName Varying name, as will be declared in the fragment shader (including v_) + * @property {string} name Attribute name, as will be declared in the header of the vertex shader (including a_) + * @property {string} type Attribute GLSL type, either `float`, `vec2`, `vec4`... + * @property {string} varyingName Varying name, as will be declared in the header of both shaders (including v_) * @property {string} varyingType Varying type, either `float`, `vec2`, `vec4`... - * @property {string} varyingExpression GLSL expression to assign to the varying in the vertex shader + * @property {string} varyingExpression GLSL expression to assign to the varying in the vertex shader (e.g. `unpackColor(a_myAttr)`) + */ + +/** + * @typedef {Object} UniformDescription + * @property {string} name Uniform name, as will be declared in the header of the vertex shader (including u_) + * @property {string} type Uniform GLSL type, either `float`, `vec2`, `vec4`... */ /** @@ -48,8 +54,8 @@ const DEFAULT_STYLE = createDefaultStyle(); * * ```js * const shader = new ShaderBuilder() - * .addVarying('v_width', 'float', 'a_width') - * .addUniform('u_time') + * .addAttribute('a_width', 'float') + * .addUniform('u_time', 'float) * .setColorExpression('...') * .setSymbolSizeExpression('...') * .getSymbolFragmentShader(); @@ -63,7 +69,7 @@ export class ShaderBuilder { constructor() { /** * Uniforms; these will be declared in the header (should include the type). - * @type {Array} + * @type {Array} * @private */ this.uniforms_ = []; @@ -202,11 +208,15 @@ export class ShaderBuilder { /** * Adds a uniform accessible in both fragment and vertex shaders. * The given name should include a type, such as `sampler2D u_texture`. - * @param {string} name Uniform name + * @param {string} name Uniform name, including the `u_` prefix + * @param {'float'|'vec2'|'vec3'|'vec4'|'sampler2D'} type GLSL type * @return {ShaderBuilder} the builder object */ - addUniform(name) { - this.uniforms_.push(name); + addUniform(name, type) { + this.uniforms_.push({ + name, + type, + }); return this; } @@ -214,21 +224,21 @@ export class ShaderBuilder { * Adds an attribute accessible in the vertex shader, read from the geometry buffer. * The given name should include a type, such as `vec2 a_position`. * Attributes will also be made available under the same name in fragment shaders. - * @param {string} name Attribute name - * @param {'float'|'vec2'|'vec3'|'vec4'} type Type - * @param {string} [transform] Expression which will be assigned to the varying in the vertex shader, and + * @param {string} name Attribute name, including the `a_` prefix + * @param {'float'|'vec2'|'vec3'|'vec4'} type GLSL type + * @param {string} [varyingExpression] Expression which will be assigned to the varying in the vertex shader, and * passed on to the fragment shader. - * @param {'float'|'vec2'|'vec3'|'vec4'} [transformedType] Type of the attribute after transformation; + * @param {'float'|'vec2'|'vec3'|'vec4'} [varyingType] Type of the attribute after transformation; * e.g. `vec4` after unpacking color components * @return {ShaderBuilder} the builder object */ - addAttribute(name, type, transform, transformedType) { + addAttribute(name, type, varyingExpression, varyingType) { this.attributes_.push({ name, type, varyingName: name.replace(/^a_/, 'v_'), - varyingType: transformedType ?? type, - varyingExpression: transform ?? name, + varyingType: varyingType ?? type, + varyingExpression: varyingExpression ?? name, }); return this; } @@ -463,7 +473,7 @@ export class ShaderBuilder { } return `${COMMON_HEADER} -${this.uniforms_.map((uniform) => `uniform ${uniform};`).join('\n')} +${this.uniforms_.map((uniform) => `uniform ${uniform.type} ${uniform.name};`).join('\n')} attribute vec2 a_position; attribute float a_index; attribute vec4 a_hitColor; @@ -540,7 +550,7 @@ ${this.attributes_ } return `${COMMON_HEADER} -${this.uniforms_.map((uniform) => `uniform ${uniform};`).join('\n')} +${this.uniforms_.map((uniform) => `uniform ${uniform.type} ${uniform.name};`).join('\n')} varying vec2 v_texCoord; varying vec4 v_hitColor; varying vec2 v_centerPx; @@ -584,7 +594,7 @@ ${this.attributes_ } return `${COMMON_HEADER} -${this.uniforms_.map((uniform) => `uniform ${uniform};`).join('\n')} +${this.uniforms_.map((uniform) => `uniform ${uniform.type} ${uniform.name};`).join('\n')} attribute vec2 a_segmentStart; attribute vec2 a_segmentEnd; attribute float a_measureStart; @@ -704,7 +714,7 @@ ${this.attributes_ } return `${COMMON_HEADER} -${this.uniforms_.map((uniform) => `uniform ${uniform};`).join('\n')} +${this.uniforms_.map((uniform) => `uniform ${uniform.type} ${uniform.name};`).join('\n')} varying vec2 v_segmentStart; varying vec2 v_segmentEnd; varying float v_angleStart; @@ -884,7 +894,7 @@ ${this.attributes_ } return `${COMMON_HEADER} -${this.uniforms_.map((uniform) => `uniform ${uniform};`).join('\n')} +${this.uniforms_.map((uniform) => `uniform ${uniform.type} ${uniform.name};`).join('\n')} attribute vec2 a_position; attribute vec4 a_hitColor; @@ -919,7 +929,7 @@ ${this.attributes_ } return `${COMMON_HEADER} -${this.uniforms_.map((uniform) => `uniform ${uniform};`).join('\n')} +${this.uniforms_.map((uniform) => `uniform ${uniform.type} ${uniform.name};`).join('\n')} varying vec4 v_hitColor; ${this.attributes_ .map( diff --git a/src/ol/render/webgl/compileUtil.js b/src/ol/render/webgl/compileUtil.js index c07c76ea613..ee3a56feb63 100644 --- a/src/ol/render/webgl/compileUtil.js +++ b/src/ol/render/webgl/compileUtil.js @@ -99,7 +99,7 @@ export function applyContextToBuilder(builder, context) { // we're not packing colors when they're passed as uniforms glslType = 'vec4'; } - builder.addUniform(`${glslType} ${uniformName}`); + builder.addUniform(uniformName, glslType); } // for each feature attribute used in the fragment shader, define a varying that will be used to pass data diff --git a/src/ol/render/webgl/style.js b/src/ol/render/webgl/style.js index e9034427f8a..0bfa3bc7270 100644 --- a/src/ol/render/webgl/style.js +++ b/src/ol/render/webgl/style.js @@ -157,11 +157,11 @@ function parseImageProperties(style, builder, uniforms, prefix, textureId) { uniforms[`u_texture${textureId}_size`] = () => { return image.complete ? [image.width, image.height] : [0, 0]; }; - builder.addUniform(`vec2 u_texture${textureId}_size`); + builder.addUniform(`u_texture${textureId}_size`, 'vec2'); const size = `u_texture${textureId}_size`; uniforms[`u_texture${textureId}`] = image; - builder.addUniform(`sampler2D u_texture${textureId}`); + builder.addUniform(`u_texture${textureId}`, 'sampler2D'); return size; } diff --git a/src/ol/renderer/webgl/VectorTileLayer.js b/src/ol/renderer/webgl/VectorTileLayer.js index b51e4af9d5b..70a84c67a5c 100644 --- a/src/ol/renderer/webgl/VectorTileLayer.js +++ b/src/ol/renderer/webgl/VectorTileLayer.js @@ -191,8 +191,8 @@ class WebGLVectorTileLayerRenderer extends WebGLBaseTileLayerRenderer { ? `(${exisitingDiscard}) || (${discardFromMask})` : discardFromMask, ); - builder.addUniform(`sampler2D ${Uniforms.TILE_MASK_TEXTURE}`); - builder.addUniform(`float ${Uniforms.TILE_ZOOM_LEVEL}`); + builder.addUniform(Uniforms.TILE_MASK_TEXTURE, 'sampler2D'); + builder.addUniform(Uniforms.TILE_ZOOM_LEVEL, 'float'); } this.styleRenderers_ = this.styles_.map((style) => { @@ -234,7 +234,7 @@ class WebGLVectorTileLayerRenderer extends WebGLBaseTileLayerRenderer { .setFillColorExpression( `vec4(${Uniforms.TILE_ZOOM_LEVEL} / 50., 0., 0., 1.)`, ) - .addUniform(`float ${Uniforms.TILE_ZOOM_LEVEL}`); + .addUniform(Uniforms.TILE_ZOOM_LEVEL, 'float'); this.tileMaskProgram_ = this.helper.getProgram( builder.getFillFragmentShader(), builder.getFillVertexShader(), diff --git a/test/browser/spec/ol/render/webgl/compileUtil.test.js b/test/browser/spec/ol/render/webgl/compileUtil.test.js index 9372d39ce1f..b5e1b09728f 100644 --- a/test/browser/spec/ol/render/webgl/compileUtil.test.js +++ b/test/browser/spec/ol/render/webgl/compileUtil.test.js @@ -87,7 +87,9 @@ describe('ol/render/webgl/compileUtil', () => { applyContextToBuilder(builder, context); - expect(builder.addUniform.calledWith('vec4 u_var_myColor')).to.be(true); + expect(builder.addUniform.calledWith('u_var_myColor', 'vec4')).to.be( + true, + ); expect( builder.addAttribute.calledWith( 'a_prop_colorProp', diff --git a/test/browser/spec/ol/render/webgl/shaderbuilder.test.js b/test/browser/spec/ol/render/webgl/shaderbuilder.test.js index 072682f225e..2ebccda243e 100644 --- a/test/browser/spec/ol/render/webgl/shaderbuilder.test.js +++ b/test/browser/spec/ol/render/webgl/shaderbuilder.test.js @@ -83,7 +83,7 @@ void main(void) { }); it('generates a symbol vertex shader (with uniforms and attributes)', () => { const builder = new ShaderBuilder(); - builder.addUniform('float u_myUniform'); + builder.addUniform('u_myUniform', 'float'); builder.addAttribute('a_myAttr', 'vec2'); builder.setSymbolSizeExpression(`vec2(${numberToGlsl(6)})`); builder.setSymbolOffsetExpression(arrayToGlsl([5, -7])); @@ -325,8 +325,8 @@ void main(void) { }); it('generates a symbol fragment shader (with uniforms)', () => { const builder = new ShaderBuilder(); - builder.addUniform('float u_myUniform'); - builder.addUniform('vec2 u_myUniform2'); + builder.addUniform('u_myUniform', 'float'); + builder.addUniform('u_myUniform2', 'vec2'); builder.setSymbolSizeExpression(`vec2(${numberToGlsl(6)})`); builder.setSymbolOffsetExpression(arrayToGlsl([5, -7])); builder.setSymbolColorExpression(colorToGlsl([255, 255, 255, 1])); @@ -375,7 +375,7 @@ void main(void) { builder.addAttribute('a_opacity', 'float', numberToGlsl(0.4)); builder.addAttribute('a_test', 'vec3', arrayToGlsl([1, 2, 3])); builder.addAttribute('a_myAttr', 'vec2'); - builder.addUniform('float u_myUniform'); + builder.addUniform('u_myUniform', 'float'); builder.setStrokeWidthExpression(numberToGlsl(4)); builder.setStrokeColorExpression(colorToGlsl([80, 0, 255, 1])); builder.setStrokeCapExpression(stringToGlsl('butt')); @@ -679,7 +679,7 @@ void main(void) { builder.addAttribute('a_opacity', 'float', numberToGlsl(0.4)); builder.addAttribute('a_test', 'vec3', arrayToGlsl([1, 2, 3])); builder.addAttribute('a_myAttr', 'vec2'); - builder.addUniform('float u_myUniform'); + builder.addUniform('u_myUniform', 'float'); builder.setFillColorExpression(colorToGlsl([80, 0, 255, 1])); builder.setFragmentDiscardExpression('u_myUniform > 0.5'); @@ -717,7 +717,7 @@ void main(void) { builder.addAttribute('a_opacity', 'float', numberToGlsl(0.4)); builder.addAttribute('a_test', 'vec3', arrayToGlsl([1, 2, 3])); builder.addAttribute('a_myAttr', 'vec2'); - builder.addUniform('float u_myUniform'); + builder.addUniform('u_myUniform', 'float'); builder.setFillColorExpression(colorToGlsl([80, 0, 255, 1])); builder.setFragmentDiscardExpression('u_myUniform > 0.5'); diff --git a/test/browser/spec/ol/render/webgl/style.test.js b/test/browser/spec/ol/render/webgl/style.test.js index 4dd88279ca9..e7e42f5c77a 100644 --- a/test/browser/spec/ol/render/webgl/style.test.js +++ b/test/browser/spec/ol/render/webgl/style.test.js @@ -40,8 +40,8 @@ describe('ol/render/webgl/style', () => { const lowerUniformName = uniformNameForVariable('lower'); const higherUniformName = uniformNameForVariable('higher'); expect(result.builder.uniforms_).to.eql([ - `float ${lowerUniformName}`, - `float ${higherUniformName}`, + {name: lowerUniformName, type: 'float'}, + {name: higherUniformName, type: 'float'}, ]); expect(result.builder.attributes_).to.eql([ { @@ -363,8 +363,8 @@ describe('ol/render/webgl/style', () => { }); it('sets up builder accordingly', () => { expect(result.builder.uniforms_).to.eql([ - `vec2 u_texture${uid}_size`, - `sampler2D u_texture${uid}`, + {name: `u_texture${uid}_size`, type: 'vec2'}, + {name: `u_texture${uid}`, type: 'sampler2D'}, ]); expect(result.builder.attributes_).to.eql([ { @@ -703,10 +703,10 @@ describe('ol/render/webgl/style', () => { }); it('parses style', () => { expect(result.builder.uniforms_).to.eql([ - 'float u_var_width', - 'float u_var_capType', - 'float u_var_joinType', - 'float u_var_miterLimit', + {name: 'u_var_width', type: 'float'}, + {name: 'u_var_capType', type: 'float'}, + {name: 'u_var_joinType', type: 'float'}, + {name: 'u_var_miterLimit', type: 'float'}, ]); expect(result.builder.attributes_).to.eql([ { @@ -872,7 +872,9 @@ describe('ol/render/webgl/style', () => { }, ); - expect(result.builder.uniforms_).to.eql(['float u_var_scale']); + expect(result.builder.uniforms_).to.eql([ + {name: 'u_var_scale', type: 'float'}, + ]); expect(result.builder.attributes_).to.eql([ { name: 'a_prop_intensity', @@ -1114,19 +1116,19 @@ describe('ol/render/webgl/style', () => { color: 'pink', lineType: 'low', lineWidth: 0.5, - fillColor: 'rgba(123, 240, 100, 0.3)', + fillColor: 'rgba(51, 153, 102, 0.3)', transparent: true, }, ); }); it('adds uniforms to the shader builder', () => { expect(parseResult.builder.uniforms_).to.eql([ - 'vec2 u_var_iconSize', - 'vec4 u_var_color', - 'float u_var_lineType', - 'float u_var_lineWidth', - 'float u_var_transparent', - 'vec4 u_var_fillColor', + {name: 'u_var_iconSize', type: 'vec2'}, + {name: 'u_var_color', type: 'vec4'}, + {name: 'u_var_lineType', type: 'float'}, + {name: 'u_var_lineWidth', type: 'float'}, + {name: 'u_var_transparent', type: 'float'}, + {name: 'u_var_fillColor', type: 'vec4'}, ]); }); it('returns uniforms in the result', () => { diff --git a/test/browser/spec/ol/renderer/webgl/VectorTileLayer.test.js b/test/browser/spec/ol/renderer/webgl/VectorTileLayer.test.js index 0bb3552a253..1ec0ca84395 100644 --- a/test/browser/spec/ol/renderer/webgl/VectorTileLayer.test.js +++ b/test/browser/spec/ol/renderer/webgl/VectorTileLayer.test.js @@ -197,13 +197,17 @@ describe('ol/renderer/webgl/VectorTileLayer', function () { it('adds a discard expression and uniforms to the styles', () => { const firstBuilder = spy.firstCall.args[0].builder; const secondBuilder = spy.secondCall.args[0].builder; - expect(firstBuilder.uniforms_).to.contain('sampler2D u_depthMask'); - expect(firstBuilder.uniforms_).to.contain('float u_tileZoomLevel'); + expect(firstBuilder.uniforms_).to.eql([ + {name: 'u_depthMask', type: 'sampler2D'}, + {name: 'u_tileZoomLevel', type: 'float'}, + ]); expect(firstBuilder.getFragmentDiscardExpression()).to.be( 'texture2D(u_depthMask, gl_FragCoord.xy / u_pixelRatio / u_viewportSizePx).r * 50. > u_tileZoomLevel + 0.5', ); - expect(secondBuilder.uniforms_).to.contain('sampler2D u_depthMask'); - expect(secondBuilder.uniforms_).to.contain('float u_tileZoomLevel'); + expect(secondBuilder.uniforms_).to.eql([ + {name: 'u_depthMask', type: 'sampler2D'}, + {name: 'u_tileZoomLevel', type: 'float'}, + ]); expect(secondBuilder.getFragmentDiscardExpression()).to.be( '(!(u_zoom > 10.0)) || (texture2D(u_depthMask, gl_FragCoord.xy / u_pixelRatio / u_viewportSizePx).r * 50. > u_tileZoomLevel + 0.5)', ); From 3650b7b3651e23fd47aa76cdd8e707e0c5d60b45 Mon Sep 17 00:00:00 2001 From: Olivia Guyot Date: Tue, 6 May 2025 09:23:00 +0200 Subject: [PATCH 6/6] webgl / correctly scale color components when provided by a uniform --- src/ol/render/webgl/compileUtil.js | 7 ++++++- test/browser/spec/ol/layer/WebGLVector.test.js | 2 +- test/browser/spec/ol/layer/WebGLVectorTile.test.js | 2 +- test/browser/spec/ol/render/webgl/compileUtil.test.js | 6 +++++- test/browser/spec/ol/render/webgl/style.test.js | 10 ++++++---- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/ol/render/webgl/compileUtil.js b/src/ol/render/webgl/compileUtil.js index ee3a56feb63..be0733e5ebb 100644 --- a/src/ol/render/webgl/compileUtil.js +++ b/src/ol/render/webgl/compileUtil.js @@ -153,7 +153,12 @@ export function generateUniformsFromContext(context, variables) { return value ? 1 : 0; } if (variable.type === ColorType) { - return asArray(value || '#eee'); + const color = [...asArray(value || '#eee')]; + color[0] /= 255; + color[1] /= 255; + color[2] /= 255; + color[3] ??= 1; + return color; } if (typeof value === 'string') { return getStringNumberEquivalent(value); diff --git a/test/browser/spec/ol/layer/WebGLVector.test.js b/test/browser/spec/ol/layer/WebGLVector.test.js index e811b853cfe..e844702fee5 100644 --- a/test/browser/spec/ol/layer/WebGLVector.test.js +++ b/test/browser/spec/ol/layer/WebGLVector.test.js @@ -89,7 +89,7 @@ describe('ol/layer/WebGLVector', function () { expect(layer.styleVariables_['fillColor']).to.be('yellow'); const renderer = layer.getRenderer(); const uniforms = renderer.styleRenderers_[0].uniforms_; - expect(uniforms.u_var_fillColor()).to.eql([255, 255, 0, 1]); + expect(uniforms.u_var_fillColor()).to.eql([1, 1, 0, 1]); }); it('can be called before the layer is rendered', function () { diff --git a/test/browser/spec/ol/layer/WebGLVectorTile.test.js b/test/browser/spec/ol/layer/WebGLVectorTile.test.js index af0a73ff848..140e73e2100 100644 --- a/test/browser/spec/ol/layer/WebGLVectorTile.test.js +++ b/test/browser/spec/ol/layer/WebGLVectorTile.test.js @@ -111,7 +111,7 @@ describe('ol/layer/WebGLVectorTile', function () { expect(layer.styleVariables_['fillColor']).to.be('yellow'); const renderer = layer.getRenderer(); const uniforms = renderer.styleRenderers_[0].uniforms_; - expect(uniforms.u_var_fillColor()).to.eql([255, 255, 0, 1]); + expect(uniforms.u_var_fillColor()).to.eql([1, 1, 0, 1]); }); it('can be called before the layer is rendered', function () { diff --git a/test/browser/spec/ol/render/webgl/compileUtil.test.js b/test/browser/spec/ol/render/webgl/compileUtil.test.js index b5e1b09728f..f85152b478f 100644 --- a/test/browser/spec/ol/render/webgl/compileUtil.test.js +++ b/test/browser/spec/ol/render/webgl/compileUtil.test.js @@ -122,6 +122,7 @@ describe('ol/render/webgl/compileUtil', () => { const context = { variables: { colorVar: {name: 'colorVar', type: ColorType}, + anotherColorVar: {name: 'anotherColorVar', type: ColorType}, stringVar: {name: 'stringVar', type: StringType}, arrayVar: {name: 'arrayVar', type: NumberArrayType}, booleanVar: {name: 'booleanVar', type: BooleanType}, @@ -129,6 +130,7 @@ describe('ol/render/webgl/compileUtil', () => { }; const styleVariables = { colorVar: '#FFF', + anotherColorVar: [51, 102, 0, 0.4], stringVar: 'hello world', arrayVar: [1, 2, 3], booleanVar: true, @@ -136,10 +138,12 @@ describe('ol/render/webgl/compileUtil', () => { const uniforms = generateUniformsFromContext(context, styleVariables); expect(uniforms).to.have.property('u_var_colorVar'); + expect(uniforms).to.have.property('u_var_anotherColorVar'); expect(uniforms).to.have.property('u_var_stringVar'); expect(uniforms).to.have.property('u_var_arrayVar'); expect(uniforms).to.have.property('u_var_booleanVar'); - expect(uniforms.u_var_colorVar()).to.eql([255, 255, 255, 1]); + expect(uniforms.u_var_colorVar()).to.eql([1, 1, 1, 1]); + expect(uniforms.u_var_anotherColorVar()).to.eql([0.2, 0.4, 0, 0.4]); expect(uniforms.u_var_stringVar()).to.eql(stringToGlsl('hello world')); expect(uniforms.u_var_arrayVar()).to.eql([1, 2, 3]); expect(uniforms.u_var_booleanVar()).to.eql(1); diff --git a/test/browser/spec/ol/render/webgl/style.test.js b/test/browser/spec/ol/render/webgl/style.test.js index e7e42f5c77a..8a28fa0dcad 100644 --- a/test/browser/spec/ol/render/webgl/style.test.js +++ b/test/browser/spec/ol/render/webgl/style.test.js @@ -1143,12 +1143,14 @@ describe('ol/render/webgl/style', () => { }); it('processes uniforms according to their types', () => { expect(parseResult.uniforms['u_var_iconSize']()).to.eql([12, 18]); - expect(parseResult.uniforms['u_var_color']()).to.eql(asArray('pink')); + expect(parseResult.uniforms['u_var_color']()).to.eql([ + 1, 0.7529411764705882, 0.796078431372549, 1, + ]); expect(parseResult.uniforms['u_var_lineType']()).to.be.a('number'); expect(parseResult.uniforms['u_var_lineWidth']()).to.eql(0.5); - expect(parseResult.uniforms['u_var_fillColor']()).to.eql( - asArray('rgba(123, 240, 100, 0.3)'), - ); + expect(parseResult.uniforms['u_var_fillColor']()).to.eql([ + 0.2, 0.6, 0.4, 0.3, + ]); expect(parseResult.uniforms['u_var_transparent']()).to.eql(1); }); });