fix: Multiple layers not including default `shaderModules` and `layer.opacity` not used in fragment shader by rubenthoms · Pull Request #2572 · equinor/webviz-subsurface-components

rubenthoms · 2025-06-05T13:12:35Z

Using super.getShaderModules({}) injects the default layer modules, e.g. layerUniforms which is required to access props.opacity in the fragment shader via layer.opacity.

References:

Layers changed:

Axes2D
GridLayer3D
Hillshading2D
NorthArrow
Map
PieChart
Colormap
Triangle
WellMarkers

Using `super.getShaderModules({})` injects the default layer modules, e.g. `layerUniforms` which is required to access `props.opacity` in the fragment shader via `layer.opacity`

w1nklr

Seems good to me.
Waiting for the upload for other layers, as discussed.

w1nklr · 2025-06-05T14:29:21Z

BTW, shouldn't the 2 last lines of cellProperty.vs.glsl.ts and nodeProperty.vs.glsl.ts be removed ?

   vec4 color = vec4(0.0);
   DECKGL_FILTER_COLOR(color, geometry);

This should not be part of a vertex shader, right ?

rubenthoms · 2025-06-05T19:54:44Z

BTW, shouldn't the 2 last lines of cellProperty.vs.glsl.ts and nodeProperty.vs.glsl.ts be removed ?
   vec4 color = vec4(0.0);
   DECKGL_FILTER_COLOR(color, geometry);
This should not be part of a vertex shader, right ?

Good question. This seems to apply a highlighting to entities. In deck.gl layers, this is also called within the vertex shader. So either we are doing it right or they are doing it wrong. I suggest we look at this later and not as part of this PR.

w1nklr · 2025-06-06T09:07:15Z

BTW, shouldn't the 2 last lines of cellProperty.vs.glsl.ts and nodeProperty.vs.glsl.ts be removed ?
   vec4 color = vec4(0.0);
   DECKGL_FILTER_COLOR(color, geometry);
This should not be part of a vertex shader, right ?
Good question. This seems to apply a highlighting to entities. In deck.gl layers, this is also called within the vertex shader. So either we are doing it right or they are doing it wrong. I suggest we look at this later and not as part of this PR.

I gave a look and understand better.
These DECKGL_FILTER_COLOR, be it in vertex or fragment shader, allow users of the layer to inject custom behavior.
In particular, this is used by deck.gl for picking !
This mean that DECKGL_FILTER_COLOR should be in (nearly) each and every shader file.

I counted 14 shader files without DECKGL_FILTER_COLOR.
We probably don't want to apply that on the 4 axes2D shaders (axis2d is not really meant to display data and should be eventually be implemented not as a layer).
For other shaders (like for the north arrow or piechart ?), it is not sure that we want to pick them.

In any case, all (most) of them should apply the layer.opacity.

Here is the list:

# should support layer.alpha and picking (thus DECKGL_FILTER_COLOR)
src/layers/axes/box.fs.glsl.ts
src/layers/axes/box.vs.glsl.ts
src/layers/gpglLayers/triangleLine.fs.glsl.ts
src/layers/gpglLayers/triangleLine.vs.glsl.ts
src/layers/grid3d/line.vs.glsl.ts
src/layers/map/line.vs.glsl.ts
src/layers/triangle/line.fs.glsl.ts
src/layers/triangle/line.vs.glsl.ts
# should support layer.alpha
# and probably also not picking (thus no DECKGL_FILTER_COLOR)
src/layers/northarrow/northarrow.fs.glsl.ts
src/layers/northarrow/northarrow.vs.glsl.ts
src/layers/piechart/piechart.fs.glsl.ts
src/layers/piechart/piechart.vs.glsl.ts
# should not support picking (thus no DECKGL_FILTER_COLOR)
# and probably no layer.alpha (should be considered like the distance ruler)
src/layers/axes2d/label.fs.glsl.ts
src/layers/axes2d/label.vs.glsl.ts
src/layers/axes2d/line.fs.glsl.ts
src/layers/axes2d/line.vs.glsl.ts
# utility, should not be changed at all
src/layers/shader_modules/decoder.fs.glsl.ts

Do you feel to handle that in this PR, or do you prefer to handle that appart ?

typescript/packages/subsurface-viewer/src/layers/grid3d/nodeProperty.fs.glsl.ts

typescript/packages/subsurface-viewer/src/layers/grid3d/nodeProperty.vs.glsl.ts

typescript/packages/subsurface-viewer/src/layers/well_markers/marker.vs.glsl.ts

typescript/packages/subsurface-viewer/src/layers/map/map.vs.glsl.ts

fix: Grid3DLayer not including default shaderModules

3b77faf

Using `super.getShaderModules({})` injects the default layer modules, e.g. `layerUniforms` which is required to access `props.opacity` in the fragment shader via `layer.opacity`

rubenthoms requested review from hkfb and w1nklr June 5, 2025 13:12

rubenthoms self-assigned this Jun 5, 2025

rubenthoms added the bug Something isn't working label Jun 5, 2025

fix: Revert removal of react import

69fbfb2

w1nklr reviewed Jun 5, 2025

View reviewed changes

rubenthoms added 2 commits June 5, 2025 17:11

wip

4421f92

fix: adjusted and applied layer opacity to all layers with custom shader

90ab7b4

rubenthoms changed the title ~~fix: Grid3DLayer not including default shaderModules and layer.opacity not used in fragment shader~~ fix: Multiple layer not including default shaderModules and layer.opacity not used in fragment shader Jun 5, 2025

rubenthoms changed the title ~~fix: Multiple layer not including default shaderModules and layer.opacity not used in fragment shader~~ fix: Multiple layers not including default shaderModules and layer.opacity not used in fragment shader Jun 5, 2025

Further adjustments

0125afb

rubenthoms requested a review from w1nklr June 5, 2025 19:54

rubenthoms added 2 commits June 6, 2025 15:31

Adjustments according to review discussion

842c07b

Minor adjustment

bf6192b

w1nklr requested changes Jun 6, 2025

View reviewed changes

typescript/packages/subsurface-viewer/src/layers/grid3d/nodeProperty.fs.glsl.ts Show resolved Hide resolved

typescript/packages/subsurface-viewer/src/layers/grid3d/nodeProperty.vs.glsl.ts Outdated Show resolved Hide resolved

fix: reverted shader code changes for grid3d nodeProperty

901a9ba

w1nklr approved these changes Jun 10, 2025

View reviewed changes

fix: MapLayer shaders

a9864a8

rubenthoms commented Jun 10, 2025

View reviewed changes

typescript/packages/subsurface-viewer/src/layers/well_markers/marker.vs.glsl.ts Outdated Show resolved Hide resolved

fix: double application of layer.opacity to final fragColor

06bf53f

rubenthoms commented Jun 10, 2025

View reviewed changes

typescript/packages/subsurface-viewer/src/layers/map/map.vs.glsl.ts Show resolved Hide resolved

rubenthoms added 2 commits June 10, 2025 11:24

fix: storybook snapshots

4de315a

fix: adjusted shaders and snapshot

a9a9a16

rubenthoms merged commit 7083782 into equinor:master Jun 12, 2025
13 of 14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Multiple layers not including default `shaderModules` and `layer.opacity` not used in fragment shader#2572

fix: Multiple layers not including default `shaderModules` and `layer.opacity` not used in fragment shader#2572
rubenthoms merged 12 commits intoequinor:masterfrom
CeetronSolutions:subsurface-viewer/support-opacity

rubenthoms commented Jun 5, 2025 •

edited

Loading

Uh oh!

w1nklr left a comment

Uh oh!

w1nklr commented Jun 5, 2025 •

edited

Loading

Uh oh!

rubenthoms commented Jun 5, 2025 •

edited

Loading

Uh oh!

w1nklr commented Jun 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rubenthoms commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w1nklr left a comment

Choose a reason for hiding this comment

Uh oh!

w1nklr commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rubenthoms commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w1nklr commented Jun 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rubenthoms commented Jun 5, 2025 •

edited

Loading

w1nklr commented Jun 5, 2025 •

edited

Loading

rubenthoms commented Jun 5, 2025 •

edited

Loading