Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove updateModuleSettings from codebase #9160

Merged
merged 8 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,6 @@ export default class HeatmapLayer<
const moduleSettings = this.getModuleSettings();
this._setModelAttributes(weightsTransform.model, attributes);
weightsTransform.model.setVertexCount(this.getNumInstances());
weightsTransform.model.updateModuleSettings(moduleSettings);

const weightProps: WeightProps = {
radiusPixels,
Expand Down
37 changes: 24 additions & 13 deletions modules/core/src/lib/layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ export default abstract class Layer<PropsT extends {} = {}> extends Component<
/** Update shader module parameters */
setModuleParameters(moduleParameters: any): void {
for (const model of this.getModels()) {
model.updateModuleSettings(moduleParameters);
// HACK as fp64 is not yet ported to UBO
model.uniforms = {ONE: 1};
}
}

Expand Down Expand Up @@ -1081,12 +1082,13 @@ export default abstract class Layer<PropsT extends {} = {}> extends Component<
// @ts-expect-error material is not a Layer prop
const {material, modelMatrix} = this.props;

// Do not pass picking module to avoid crash
// TODO remove `setModuleParameters` from codebase
const {picking: _, ...rest} = moduleParameters;
this.setModuleParameters(rest);
this.setModuleParameters({});

const {
// mask
maskChannels,
maskMap,
maskSources,
// shadow
shadowEnabled,
drawToShadowMap,
Expand All @@ -1108,6 +1110,12 @@ export default abstract class Layer<PropsT extends {} = {}> extends Component<
lightSources
} = moduleParameters;

const maskProps = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type the props?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type is in deck.gl/extensions and it is an open question whether this is the correct place to update the ShaderInputs as in many cases the referenced ShaderModules will not be present. Agree that we should type, will revisit when cleaning up, e.g. moving the shaderInputs.setProps into the extensions

maskChannels,
maskMap,
maskSources
};

const shadowProps = {
viewport,
shadowEnabled,
Expand All @@ -1130,22 +1138,25 @@ export default abstract class Layer<PropsT extends {} = {}> extends Component<
terrainSkipRender
};

const projectProps = {
viewport,
devicePixelRatio,
modelMatrix,
coordinateSystem,
coordinateOrigin
} as ProjectProps;
felixpalmer marked this conversation as resolved.
Show resolved Hide resolved

this.setShaderModuleProps({
// TODO Revisit whether this is necessary once all layers ported to UBO
mask: maskProps,
shadow: shadowProps,
terrain: terrainProps,
layer: {opacity},
lighting: lightSources,
phongMaterial: material,
gouraudMaterial: material,
picking: {isActive, isAttribute} as PickingProps,
project: {
viewport,
devicePixelRatio,
modelMatrix,
coordinateSystem,
coordinateOrigin
} as ProjectProps
picking: {isActive, isAttribute} as const satisfies PickingProps,
project: projectProps
});
}

Expand Down
21 changes: 10 additions & 11 deletions modules/core/src/shaderlib/project/viewport-uniforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
// THE SOFTWARE.
/* eslint-disable complexity, camelcase */

import {mat4, vec4} from '@math.gl/core';
import type {NumberArray16} from '@math.gl/types';
import {mat4, Matrix4Like, vec4} from '@math.gl/core';

import {COORDINATE_SYSTEM, PROJECTION_MODE} from '../../lib/constants';

Expand All @@ -35,8 +34,8 @@ type Vec4 = [number, number, number, number];
// To quickly set a vector to zero
const ZERO_VECTOR: Vec4 = [0, 0, 0, 0];
// 4x4 matrix that drops 4th component of vector
const VECTOR_TO_POINT_MATRIX: NumberArray16 = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0];
const IDENTITY_MATRIX: NumberArray16 = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1];
const VECTOR_TO_POINT_MATRIX: Matrix4Like = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0];
const IDENTITY_MATRIX: Matrix4Like = [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1];
const DEFAULT_PIXELS_PER_UNIT2: Vec3 = [0, 0, 0];
const DEFAULT_COORDINATE_ORIGIN: Vec3 = [0, 0, 0];

Expand Down Expand Up @@ -127,8 +126,8 @@ function calculateMatrixAndOffset(
coordinateSystem: CoordinateSystem,
coordinateOrigin: Vec3
): {
viewMatrix: NumberArray16;
viewProjectionMatrix: NumberArray16;
viewMatrix: Matrix4Like;
viewProjectionMatrix: Matrix4Like;
projectionCenter: Vec4;
originCommon: Vec4;
cameraPosCommon: Vec3;
Expand Down Expand Up @@ -177,8 +176,8 @@ function calculateMatrixAndOffset(
}

return {
viewMatrix: viewMatrix as NumberArray16,
viewProjectionMatrix: viewProjectionMatrix as NumberArray16,
viewMatrix: viewMatrix as Matrix4Like,
viewProjectionMatrix: viewProjectionMatrix as Matrix4Like,
projectionCenter,
originCommon,
cameraPosCommon,
Expand Down Expand Up @@ -209,8 +208,8 @@ export type ProjectUniforms = {
scale: number;
wrapLongitude: boolean;

viewProjectionMatrix: NumberArray16;
modelMatrix: NumberArray16;
viewProjectionMatrix: Matrix4Like;
modelMatrix: Matrix4Like;

// This is for lighting calculations
cameraPosition: Vec3;
Expand All @@ -219,7 +218,7 @@ export type ProjectUniforms = {
export type ProjectProps = {
viewport: Viewport;
devicePixelRatio?: number;
modelMatrix?: NumberArray16 | null;
modelMatrix?: Matrix4Like | null;
coordinateSystem?: CoordinateSystem;
coordinateOrigin?: Vec3;
autoWrapLongitude?: boolean;
Expand Down
3 changes: 2 additions & 1 deletion modules/core/src/types/layer-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {Texture, TextureProps} from '@luma.gl/core';
import type {Buffer, Parameters} from '@luma.gl/core';
import type {Loader} from '@loaders.gl/loader-utils';
import type {LightingModuleSettings} from '../shaderlib/index';
import type {Matrix4Like} from '@math.gl/core';

export type LayerData<T> =
| Iterable<T>
Expand Down Expand Up @@ -169,7 +170,7 @@ export type LayerProps = {
/**
* A 4x4 matrix to transform local coordianates to the world space.
*/
modelMatrix?: NumericArray | null;
modelMatrix?: Matrix4Like | null;
/**
* (Geospatial only) normalize geometries that cross the 180th meridian. Default false.
*/
Expand Down
7 changes: 2 additions & 5 deletions modules/extensions/src/fp64/fp64-extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ export default class Fp64Extension extends LayerExtension {
}

draw(this: Layer, params: any, extension: this): void {
const {moduleParameters} = params;
if (moduleParameters) {
const {viewport} = moduleParameters;
this.setShaderModuleProps({project64: {viewport}});
}
const {viewport} = params.context;
this.setShaderModuleProps({project64: {viewport}});
}
}
4 changes: 2 additions & 2 deletions test/modules/extensions/fp64.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import test from 'tape-promise/tape';
import {Fp64Extension} from '@deck.gl/extensions';
import {COORDINATE_SYSTEM} from '@deck.gl/core';
import {ScatterplotLayer} from '@deck.gl/layers';
import {testLayer} from '@deck.gl/test-utils';
import {getLayerUniforms, testLayer} from '@deck.gl/test-utils';

test('Fp64Extension', t => {
const testCases = [
Expand All @@ -22,7 +22,7 @@ test('Fp64Extension', t => {
extensions: [new Fp64Extension()]
},
onAfterUpdate: ({layer}) => {
const {uniforms} = layer.state.model;
const uniforms = getLayerUniforms(layer);
t.ok(uniforms.viewProjectionMatrix, 'has fp64 uniforms');
t.ok(uniforms.viewProjectionMatrix64Low, 'has fp64 uniforms');
}
Expand Down
Loading