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

[RFC] ColorFilterExtension #8439

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
126 changes: 126 additions & 0 deletions dev-docs/RFCs/proposals/color-filter-extension-rfc.md
@@ -0,0 +1,126 @@
# RFC: Color Filter and Effects Extension for deck.gl Layers

* Authors: Chris Gervang
* Date: January, 2023
* Status: **Draft**

## Abstract

This RFC proposes a new layer extension for `deck.gl` that allows users to easily apply color filters and effects, such as tint, brightness, and saturation, to any layer. The extension utilizes the `getModels` function from each layer and injects a `DECKGL_FILTER_COLOR` shader extension into each model.

## Background

`deck.gl` currently lacks a straightforward way to apply color filters and effects directly to layers. Users often have to write custom layers to achieve desired visual effects, which can be cumbersome. Alternatively, they are limited to using post-processing effects, which apply to the whole frame buffer rather than per layer.

## Proposals

### Design 1: User-Provided Luma ShaderPass

#### Overview

This design allows users to provide a `luma.gl` ShaderPass, such as the built-in vibrance module, to apply effects. The extension will utilize this ShaderPass for layer rendering.

#### Implementation

- Introduce a `ColorFilterExtension` that accepts a ShaderPass as an input parameter.
- Modify the layer's fragment shader by integrating the provided ShaderPass at render time.

```js
import {
brightnessContrast,
hueSaturation,
} from '@luma.gl/shadertools'

new Tile3DLayer({
extensions: [new ColorFilterExtension(brightnessContrast)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

PostProcessEffect has the props passed directly to the constructor. What do you think of the pattern:

new ColorFilterExtension(brightnessContrast, {
  brightness: 0.5,
  contrast: 0.5
})

Then with types with would get autocompletion on the parameters:

 export default class ColorFilterExtension<T extends ShaderModule> extends LayerExtension {
   constructor(shaderModule: T, shaderModuleProps: T['props']) 
}

brightnessContrast: {
brightness: 0.5,
contrast: 0.5
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

By importing the module you can easily obtain the props required using Typescript:

brightnessContrast: {
  brightness: 0.5,
  contrast: 0.5
} as typeof brightnessContrast.props

})
```

Multiple filters can be applied with more than one instance:

```js
{
extensions: [new ColorFilterExtension(hueSaturation), new ColorFilterExtension(brightnessContrast)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer just one extension and instead a list of effects

new ColorFilterExtension([hueSaturation, brightnessContrast])

hueSaturation: ...,
brightnessContrast: ...,
}
```

#### Pros and Cons

**Pros:**
- Flexibility: Enables users to create custom effects.
- Compatibility: Integrates seamlessly with existing `luma.gl` modules.

**Cons:**
- Usage Complexity: Requires users to have a good understanding of `luma.gl` shaders.
- Typings Complexity: Determining extension properties is complex since the ShaderPass dictates them.

### Design 2: Built-in Shaders in Extension

#### Overview

This design incorporates a set of predefined shaders within the extension, addressing common filters like tint, brightness, and saturation.

#### Implementation

- Equip the `ColorFilterExtension` with built-in shader modules.
- Allow users to select a filter and modify its parameters (e.g., intensity).

```js
new Tile3DLayer({
extensions: [new ColorFilterExtension()]
brightnessContrast: {
brightness: 0.5,
contrast: 0.5
}
})
```

All single pass "filter" shaders would already be built-in: brightnessContrast, hueSaturation, vibrance, tint (mix original colors), fill (set colors to single hue), sepia, colorHalftone, and dotScreen. So, multiple filters can be applied with one instance:

```js
new Tile3DLayer({
extensions: [new ColorFilterExtension()]
hueSaturation: ...,
brightnessContrast: ...,
})
```

##### Props

- shaders (`Array<String>`):

Should a user need to customize the shader order we could have an `shaders` extension prop to specify which shaders to load and in which order.

Usage: `new ColorFilterExtension({shaders: ['hueSaturation', 'tint']})`

Shader order can give a different result, so this may be useful in the event a user wanted to first make their layer greyscale and then tint it as opposed to tinting before desaturating.

The same prop may be useful for cherry picking which shaders to apply when users are concerned with shader overhead.

#### Pros and Cons

**Pros:**
- Ease of Use: Simplifies the process for users unfamiliar with shader programming.
- Consistency: Ensures standardized performance and appearance.

**Cons:**
- Limited Flexibility: Confines users to predefined filters.
Copy link
Collaborator

Choose a reason for hiding this comment

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

For me this is too limiting. I don't find importing a shader module from luma and passing it to the ColorFilterExtension constructor too onerous. It is also what we already have in the PostProcessEffect

- Additional Overhead: Introduces shaders that may not be used consistently unless a lifecycle mechanism for adding/removing active effects is implemented.

## Conclusion

After careful consideration of both designs, I'd prefer Design 2: Built-in Shaders in Extension. This design is easier to adopt for a broader range of users, especially those without extensive shader programming experience.
Copy link
Collaborator

Choose a reason for hiding this comment

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

My vote is for a variation on Design 1, allowing passing custom shader modules. I think being able to modify the shaders is really powerful and fun and not so difficult for basic color filters


While Design 1 offers more flexibility, the complexity it introduces both in terms of usage and implementation may pose barriers for users and maintainers.

### Remarks

We could extend this design in to support custom shaders in the future, akin to what Design 1 proposes.

If performance degrades considerably when applying this extension, we could explore mechanisms for managing the lifecycle of active effects to minimize overhead since most use cases would likely only apply a few effects at a time.