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

[Feat] Add autoHighlightBlending prop to Layer #9291

Open
felixpalmer opened this issue Dec 11, 2024 · 6 comments
Open

[Feat] Add autoHighlightBlending prop to Layer #9291

felixpalmer opened this issue Dec 11, 2024 · 6 comments
Assignees
Labels

Comments

@felixpalmer
Copy link
Collaborator

Target Use Case

Currently the picking module always blends the alpha of autohighlightColor with the alpha of the fragment. Layers with partial transparency thus end up with an ugly aliased outline, in particular this is obvious with IconLayer (but also ScatterplotLayer)

Screen.Recording.2024-12-11.at.10.01.24.mov

Proposal

The luma picking module is enhanced with a new prop, blendHighlightAlpha. It defaults to 'normal' to maintain existing behavior, but when it is 'src' the fragment color is blended as before, but the alpha is unchanged.

The Layer class adds a new prop, autoHighlightBlending (also defaulting to 'normal'), with select layers (like IconLayer) changing the default to 'src'.

Why add a new prop?

The highlightColor prop supports both RGB & RGBA colors, so in theory we could only apply 'normal' blending when an RGBA highlightColor is used, and use 'src' with RGB.

However there is value in allowing the user to explicitly use the autoHighlightBlending prop to support different behaviors:

highlightColor autoHighlightBlending behavior
[255, 0, 0, 255] 'normal' Replace all pixels with red (current behavior)
[255, 0, 0, 255] 'src' Tint all pixels red, while preserving transparency
[255, 255, 255, 128] 'src' Lighten all pixels, while preserving transparency
[0, 0, 0, 128] 'src' Darken all pixels, while preserving transparency

The prop is defined as a string constant in order to allow future expansion, for example 'multiply' could compute the output alpha as color.a * picking.highlightColor.a to allow for fading (not sure if this is useful).

Tint all pixels, while preserving transparency

Screen.Recording.2024-12-11.at.10.02.16.mov

Lighten all pixels, while preserving transparency

Screen.Recording.2024-12-11.at.10.34.26.mov

Darken all pixels, while preserving transparency

Screen.Recording.2024-12-11.at.10.35.00.mov
@felixpalmer felixpalmer self-assigned this Dec 11, 2024
@felixpalmer
Copy link
Collaborator Author

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 11, 2024

Thanks for looking into this tricky problem! And for prepping some examples (I always wanted a deck.gl doc page about blending with image / video examples such as these to help demystify blending for users.)

In terms of feedback on the proposal, first I note that luma.gl exposes the WebGPU blend parameter system, see https://luma.gl/docs/api-reference/core/parameters#blending

This system offers a lot of control with separate parameters for color and alpha blending. However in the proposal we are not just passing through these parameters. Instead it looks like we are defining some new prop that offers a few combinations using some "higher level" constants.

That might be fine, but one of the blend functions in the underlying blend system is called src. If we are not exposing the full power of the underlying system, perhaps we should opt for a different constant name? Or if we do use it, at least make sure that we mean the same thing in both "systems".

Also, if we are in effect defining a new higher level blending system, should we make sure we document it in a central place, and use it (or at least plan to use it) across the API - For example, I believe that Mesh and ScenegraphLayers blend their instance colors into the underlying meshes etc and could offer similar control using similarly named props?

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 11, 2024

Side note: Perhaps surprisingly, blending in GPU APIs is not a "frozen" feature. There is a lot of activity in blending right now:

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 11, 2024

Another thing to consider is that we might want to move to premultiplied colors in the future to further improve blending quality.

@felixpalmer
Copy link
Collaborator Author

Thanks for the input. My intention with 'src' was to use a naming scheme that was similar to the standard blend modes. In general blending is configured in parameters using 6 constants and it seems overkill to allow passing them all (and implementing the maths in the picking module).

I would hope we could reduce the number of variables that can be configured to just one, but not by introducing "higher-level" constants, but by mandating that some of the original 6 constants them cannot be configured

@felixpalmer
Copy link
Collaborator Author

Just to make sense of what luma does right now, the blending code looks like this:

float highLightAlpha = picking.highlightColor.a;
float blendedAlpha = highLightAlpha + color.a * (1.0 - highLightAlpha);
float highLightRatio = highLightAlpha / blendedAlpha;

vec3 blendedRGB = mix(color.rgb, picking.highlightColor.rgb, highLightRatio);
return vec4(blendedRGB, blendedAlpha);

If we go by the notation from MDN:

  • color(RGB) = (sourceColor * srcRGB) + (destinationColor * dstRGB)
  • color(A) = (sourceAlpha * srcAlpha) + (destinationAlpha * dstAlpha)

we have:

  • sourceColor as highlightColor.rgb & sourceAlpha as highlightColor.a
  • destinationAlpha as color.rgb & destinationAlpha as color.a

Thus for the alpha calculation the current constants used are:

  • sourceAlpha: ONE
  • destinationAlpha: ONE_MINUS_SRC_ALPHA

However the RGB is computed using highLightRatio which is a function both color.a & highLightAlpha and as far as I can tell is not expressible using the standard bledning equations & constants

To move forward and to align with standards I would propose we re-write the shader to support passing in sourceAlpha & destinationAlpha with the following supported values ZERO, ONE, SRC_ALPHA, DST_ALPHA, ONE_MINUS_SRC_ALPHA, ONE_MINUS_DST_ALPHA. To implement my originally proposed 'src' mode is would then just mean passing ZERO and ONE.

As for the color blending I'm not sure how much value there is in allowing this, as we are always just blending with a constant color anyway & the driving motivation for this proposal is the removal of aliased edges, not to implement some complex hover effects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants