diff --git a/CHANGELOG.md b/CHANGELOG.md index afedc76f78d..c8470686309 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## 3.21.1 + +### Bug fixes 🐞 + +- Fix map crashes on Android Pixel devices (Adreno GPUs) during zoom/pan interactions caused by symbol UBO dynamic indexing. +- Fix `readFloat`/`readUint` in the symbol vertex shader to use Adreno-safe explicit swizzle helpers instead of direct bracket indexing. +- Fix stale default in `getMaxFeatureCount` that could cause incorrect symbol batching limits. +- Guard against potential division by zero in the cutoff fade-range calculation on mobile viewports. + ## 3.20.0-rc.1 ### Features and improvements ✨ diff --git a/src/data/bucket/symbol_properties_ubo.ts b/src/data/bucket/symbol_properties_ubo.ts index 0d06df9c24a..e5f5e5acc45 100644 --- a/src/data/bucket/symbol_properties_ubo.ts +++ b/src/data/bucket/symbol_properties_ubo.ts @@ -166,7 +166,7 @@ export class SymbolPropertiesUBO { * Maximum number of features that fit in one UBO batch given a header. * Returns Infinity when dataDrivenBlockSizeVec4 is 0 (all properties constant). */ - static getMaxFeatureCount(header: SymbolPropertyHeader, propsDwords: number = 4096 - SymbolPropertiesUBO.HEADER_DWORDS): number { + static getMaxFeatureCount(header: SymbolPropertyHeader, propsDwords: number = 4096): number { const dataDrivenBlockSizeDwords = header.dataDrivenBlockSizeVec4 * 4; if (dataDrivenBlockSizeDwords === 0) return Infinity; return Math.floor(propsDwords / dataDrivenBlockSizeDwords); diff --git a/src/gl/context.ts b/src/gl/context.ts index 7137222452e..d1844d73b3b 100644 --- a/src/gl/context.ts +++ b/src/gl/context.ts @@ -153,10 +153,10 @@ class Context { // Force manual rendering for instanced draw calls having gl_InstanceID usage in the shader for PowerVR adapters this.forceManualRenderingForInstanceIDShaders = (options && !!options.forceManualRenderingForInstanceIDShaders) || (this.renderer && this.renderer.indexOf("PowerVR") !== -1); - // Disable symbol UBO batching for PowerVR GPUs. PowerVR drivers do not correctly handle - // dynamic (non-uniform) indexing into UBO arrays, which is technically undefined behavior - // in GLSL ES 3.00. The fallback uses pragma-based paint properties. - this.disableSymbolUBO = (options && !!options.forceDisableSymbolUBO) || (this.renderer && this.renderer.indexOf("PowerVR") !== -1); + // Disable symbol UBO batching for PowerVR and Adreno GPUs. These drivers do not + // correctly handle dynamic (non-uniform) indexing into UBO arrays, which is technically + // undefined behavior in GLSL ES 3.00. The fallback uses pragma-based paint properties. + this.disableSymbolUBO = (options && !!options.forceDisableSymbolUBO) || (this.renderer && (this.renderer.indexOf("PowerVR") !== -1 || this.renderer.indexOf("Adreno") !== -1)); if (!this.options.extTextureFloatLinearForceOff) { this.extTextureFloatLinear = gl.getExtension('OES_texture_float_linear'); diff --git a/src/render/cutoff.ts b/src/render/cutoff.ts index 21084b87002..2c52a7b5664 100644 --- a/src/render/cutoff.ts +++ b/src/render/cutoff.ts @@ -76,7 +76,7 @@ export const getCutoffParams = (painter: Painter, cutoffFadeRange: number): Cuto }; } - const zRange = tr._farZ - tr._nearZ; + const zRange = Math.max(tr._farZ - tr._nearZ, 1.0); const fadeRangePixels = cutoffFadeRange * tr.height * FADE_RANGE_HEIGHT_SCALE; // Half-rate exponential zoom scaling: grows with zoom but stays bounded diff --git a/src/shaders/symbol.vertex.glsl b/src/shaders/symbol.vertex.glsl index a62bf022724..d612ef490cc 100644 --- a/src/shaders/symbol.vertex.glsl +++ b/src/shaders/symbol.vertex.glsl @@ -275,11 +275,11 @@ vec4 readVec4(uint baseOffsetVec4, uint propertyOffsetDwords) { } float readFloat(vec4 slot, uint propertyOffsetDwords) { - return slot[propertyOffsetDwords % DWORDS_PER_VEC4]; + return vec4At(slot, propertyOffsetDwords % DWORDS_PER_VEC4); } uint readUint(uvec4 slot, uint offset) { - return slot[offset % DWORDS_PER_VEC4]; + return uvec4At(slot, offset % DWORDS_PER_VEC4); } vec2 readVec2(vec4 slot, uint propertyOffsetDwords) { diff --git a/test/integration/render-tests/text-color/property-function-no-symbol-ubo/expected.png b/test/integration/render-tests/text-color/property-function-no-symbol-ubo/expected.png new file mode 100644 index 00000000000..eefc3c69c4a Binary files /dev/null and b/test/integration/render-tests/text-color/property-function-no-symbol-ubo/expected.png differ diff --git a/test/integration/render-tests/text-color/property-function-no-symbol-ubo/style.json b/test/integration/render-tests/text-color/property-function-no-symbol-ubo/style.json new file mode 100644 index 00000000000..17d23affa1f --- /dev/null +++ b/test/integration/render-tests/text-color/property-function-no-symbol-ubo/style.json @@ -0,0 +1,75 @@ +{ + "version": 8, + "metadata": { + "test": { + "height": 64, + "width": 64, + "forceDisableSymbolUBO": true + } + }, + "center": [ 0, 0 ], + "zoom": 0, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": { "x": 0 }, + "geometry": { + "type": "Point", + "coordinates": [ + 0, + -10 + ] + } + }, + { + "type": "Feature", + "properties": { "x": 1 }, + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 10 + ] + } + } + ] + } + } + }, + "glyphs": "local://glyphs/{fontstack}/{range}.pbf", + "layers": [ + { + "id": "text", + "type": "symbol", + "source": "geojson", + "layout": { + "text-allow-overlap": true, + "text-field": "Test", + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + }, + "paint": { + "text-color": { + "property": "x", + "stops": [ + [ + 0, + "red" + ], + [ + 1, + "blue" + ] + ] + } + } + } + ] +} diff --git a/test/integration/render-tests/text-halo-color/property-function-no-symbol-ubo/expected.png b/test/integration/render-tests/text-halo-color/property-function-no-symbol-ubo/expected.png new file mode 100644 index 00000000000..28fc63308d0 Binary files /dev/null and b/test/integration/render-tests/text-halo-color/property-function-no-symbol-ubo/expected.png differ diff --git a/test/integration/render-tests/text-halo-color/property-function-no-symbol-ubo/style.json b/test/integration/render-tests/text-halo-color/property-function-no-symbol-ubo/style.json new file mode 100644 index 00000000000..6dc651012ec --- /dev/null +++ b/test/integration/render-tests/text-halo-color/property-function-no-symbol-ubo/style.json @@ -0,0 +1,72 @@ +{ + "version": 8, + "metadata": { + "test": { + "width": 64, + "height": 64, + "forceDisableSymbolUBO": true + } + }, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": { "x": 0 }, + "geometry": { + "type": "Point", + "coordinates": [ + 0, + -10 + ] + } + }, { + "type": "Feature", + "properties": { "x": 1 }, + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 10 + ] + } + } + ] + } + } + }, + "glyphs": "local://glyphs/{fontstack}/{range}.pbf", + "layers": [ + { + "id": "symbol", + "type": "symbol", + "source": "geojson", + "layout": { + "text-field": "ABC", + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + }, + "paint": { + "text-halo-width": 2, + "text-halo-color": { + "property": "x", + "stops": [ + [ + 0, + "red" + ], + [ + 1, + "blue" + ] + ] + } + } + } + ] +} diff --git a/test/integration/render-tests/utils.ts b/test/integration/render-tests/utils.ts index c0a9d6c9cca..e32b9120717 100644 --- a/test/integration/render-tests/utils.ts +++ b/test/integration/render-tests/utils.ts @@ -162,6 +162,7 @@ export async function renderMap(style, options, currentTestName) { extTextureFloatLinearForceOff: options.textureFloatLinear === undefined ? false : !options.textureFloatLinear, // ordinary instancing is enabled by default, manual is disabled forceManualRenderingForInstanceIDShaders: options.forceManualRenderingForInstanceIDShaders, + forceDisableSymbolUBO: options.forceDisableSymbolUBO, }, worldview: options.worldview, maxZoom: options.maxZoom, diff --git a/test/unit/data/symbol_property_binder_ubo.test.ts b/test/unit/data/symbol_property_binder_ubo.test.ts index 0a49ea569f0..9b01830c747 100644 --- a/test/unit/data/symbol_property_binder_ubo.test.ts +++ b/test/unit/data/symbol_property_binder_ubo.test.ts @@ -91,8 +91,8 @@ describe('SymbolPropertiesUBO', () => { test('getMaxFeatureCount returns correct value', () => { // dataDrivenBlockSizeVec4=1 → dataDrivenBlockSizeDwords=4 - // propsDwords = 4096 - 12 (HEADER_DWORDS) = 4084 - // maxFeatures = floor(4084 / 4) = 1021 + // propsDwords = 4096 + // maxFeatures = floor(4096 / 4) = 1024 const header: SymbolPropertyHeader = { dataDrivenMask: 0b00000100, zoomDependentMask: 0, @@ -100,7 +100,7 @@ describe('SymbolPropertiesUBO', () => { dataDrivenBlockSizeVec4: 1, offsets: [0, 0, 0, 0, 0, 0, 0, 0, 0], }; - expect(SymbolPropertiesUBO.getMaxFeatureCount(header)).toEqual(1021); + expect(SymbolPropertiesUBO.getMaxFeatureCount(header)).toEqual(1024); }); test('getMaxFeatureCount returns Infinity when all constant', () => { diff --git a/test/unit/gl/context_gpu_flags.test.ts b/test/unit/gl/context_gpu_flags.test.ts new file mode 100644 index 00000000000..0fb8cdda0af --- /dev/null +++ b/test/unit/gl/context_gpu_flags.test.ts @@ -0,0 +1,93 @@ +import {test, expect, describe} from '../../util/vitest'; +import Context from '../../../src/gl/context'; + +describe('Context GPU flags', () => { + /** + * Returns a WebGL2 context whose WEBGL_debug_renderer_info extension + * reports the given renderer string. This lets us exercise the + * renderer-substring detection in the Context constructor without + * relying on the host GPU. + */ + function createGLWithRenderer(renderer: string): WebGL2RenderingContext { + const el = window.document.createElement('canvas'); + const gl = el.getContext('webgl2'); + if (!gl) throw new Error('WebGL2 context unavailable — cannot run GPU flag tests'); + + const UNMASKED_RENDERER = 0x9246; // WEBGL_debug_renderer_info constant + const fakeExt = {UNMASKED_RENDERER_WEBGL: UNMASKED_RENDERER, UNMASKED_VENDOR_WEBGL: 0x9245}; + + const origGetExtension = gl.getExtension.bind(gl); + const origGetParameter = gl.getParameter.bind(gl); + + gl.getExtension = ((name: string) => { + if (name === 'WEBGL_debug_renderer_info') return fakeExt; + return origGetExtension(name); + }) as typeof gl.getExtension; + + gl.getParameter = ((pname: number) => { + if (pname === UNMASKED_RENDERER) return renderer; + if (pname === fakeExt.UNMASKED_VENDOR_WEBGL) return 'Test Vendor'; + return origGetParameter(pname); + }) as typeof gl.getParameter; + + return gl; + } + + /** + * Returns a WebGL2 context where WEBGL_debug_renderer_info is + * unavailable (getExtension returns null), so Context.renderer + * stays undefined. + */ + function createGLWithoutRendererInfo(): WebGL2RenderingContext { + const el = window.document.createElement('canvas'); + const gl = el.getContext('webgl2'); + if (!gl) throw new Error('WebGL2 context unavailable — cannot run GPU flag tests'); + + const origGetExtension = gl.getExtension.bind(gl); + + gl.getExtension = ((name: string) => { + if (name === 'WEBGL_debug_renderer_info') return null; + return origGetExtension(name); + }) as typeof gl.getExtension; + + return gl; + } + + describe('disableSymbolUBO', () => { + test('enabled when forceDisableSymbolUBO option is set', () => { + const gl = createGLWithRenderer('Generic Desktop GPU'); + const context = new Context(gl, {forceDisableSymbolUBO: true}); + expect(context.disableSymbolUBO).toBe(true); + }); + + test('enabled when renderer contains Adreno', () => { + const gl = createGLWithRenderer('Adreno (TM) 730'); + const context = new Context(gl); + expect(context.disableSymbolUBO).toBe(true); + }); + + test('enabled when renderer contains PowerVR', () => { + const gl = createGLWithRenderer('PowerVR Rogue GE8320'); + const context = new Context(gl); + expect(context.disableSymbolUBO).toBe(true); + }); + + test('disabled for non-matching renderer', () => { + const gl = createGLWithRenderer('ANGLE (Apple, Apple M2 Max, OpenGL 4.1)'); + const context = new Context(gl); + expect(context.disableSymbolUBO).toBe(false); + }); + + test('disabled when WEBGL_debug_renderer_info is unavailable', () => { + const gl = createGLWithoutRendererInfo(); + const context = new Context(gl); + expect(context.disableSymbolUBO).toBeFalsy(); + }); + + test('forceDisableSymbolUBO still works without renderer info', () => { + const gl = createGLWithoutRendererInfo(); + const context = new Context(gl, {forceDisableSymbolUBO: true}); + expect(context.disableSymbolUBO).toBe(true); + }); + }); +}); diff --git a/test/unit/render/cutoff.test.ts b/test/unit/render/cutoff.test.ts new file mode 100644 index 00000000000..038b4b1eece --- /dev/null +++ b/test/unit/render/cutoff.test.ts @@ -0,0 +1,126 @@ +import {test, expect, describe} from '../../util/vitest'; +import {getCutoffParams} from '../../../src/render/cutoff'; + +// getCutoffParams reads specific fields from Painter and Transform. +// We provide minimal fixtures with just the fields the function accesses, +// avoiding mocking internal domain objects per the test guidelines. +function createPainterFixture(overrides: Record = {}) { + return { + terrain: null, + minCutoffZoom: 5, + transform: { + pitch: 60, + _zoom: 10, + _nearZ: 10, + _farZ: 5000, + height: 800, + cameraToCenterDistance: 1000, + isLODDisabled: () => false, + }, + ...overrides, + }; +} + +describe('getCutoffParams', () => { + // -- early-exit paths -- + + test('returns shouldRenderCutoff false when cutoffFadeRange is zero', () => { + const painter = createPainterFixture(); + const result = getCutoffParams(painter as never, 0); + expect(result.shouldRenderCutoff).toBe(false); + }); + + test('returns shouldRenderCutoff false when terrain is active', () => { + const painter = createPainterFixture({terrain: {}}); + const result = getCutoffParams(painter as never, 0.5); + expect(result.shouldRenderCutoff).toBe(false); + }); + + test('returns shouldRenderCutoff false when pitch is below activation threshold', () => { + const painter = createPainterFixture({ + transform: { + pitch: 5, + _zoom: 10, + _nearZ: 10, + _farZ: 5000, + height: 800, + cameraToCenterDistance: 1000, + isLODDisabled: () => false, + }, + }); + const result = getCutoffParams(painter as never, 0.5); + expect(result.shouldRenderCutoff).toBe(false); + }); + + // -- zero/negative zRange guard -- + + test('returns finite cutoff params when zRange is near zero', () => { + const painter = createPainterFixture({ + transform: { + pitch: 60, + _zoom: 10, + _nearZ: 100, + _farZ: 100, // farZ == nearZ → zRange would be 0 without guard + height: 800, + cameraToCenterDistance: 1000, + isLODDisabled: () => false, + }, + }); + const result = getCutoffParams(painter as never, 0.5); + const params = result.uniformValues['u_cutoff_params']; + expect(Number.isFinite(params[2])).toBe(true); + expect(Number.isFinite(params[3])).toBe(true); + }); + + test('returns finite cutoff params when farZ is less than nearZ', () => { + const painter = createPainterFixture({ + transform: { + pitch: 60, + _zoom: 10, + _nearZ: 200, + _farZ: 50, // farZ < nearZ → zRange would be negative without guard + height: 800, + cameraToCenterDistance: 1000, + isLODDisabled: () => false, + }, + }); + const result = getCutoffParams(painter as never, 0.5); + const params = result.uniformValues['u_cutoff_params']; + expect(Number.isFinite(params[2])).toBe(true); + expect(Number.isFinite(params[3])).toBe(true); + }); + + // -- active cutoff invariants -- + + test('returns shouldRenderCutoff true at high pitch with valid fade range', () => { + const painter = createPainterFixture(); + const result = getCutoffParams(painter as never, 0.5); + expect(result.shouldRenderCutoff).toBe(true); + }); + + test('all u_cutoff_params components are finite when cutoff is active', () => { + const painter = createPainterFixture(); + const result = getCutoffParams(painter as never, 0.5); + const params = result.uniformValues['u_cutoff_params']; + for (let i = 0; i < 4; i++) { + expect(Number.isFinite(params[i])).toBe(true); + } + }); + + test('relativeCutoffFadeDistance <= relativeCutoffDistance', () => { + const painter = createPainterFixture(); + const result = getCutoffParams(painter as never, 0.5); + const params = result.uniformValues['u_cutoff_params']; + // params[2] = relativeCutoffDistance, params[3] = relativeCutoffFadeDistance + expect(params[3]).toBeLessThanOrEqual(params[2]); + }); + + test('normalized depth values are non-negative when cutoff is active', () => { + const painter = createPainterFixture(); + const result = getCutoffParams(painter as never, 0.5); + const params = result.uniformValues['u_cutoff_params']; + // params[2] = relativeCutoffDistance, params[3] = relativeCutoffFadeDistance + expect(params[2]).toBeGreaterThanOrEqual(0); + expect(params[3]).toBeGreaterThanOrEqual(0); + }); +});