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

Add MaskExtension #6554

Merged
merged 78 commits into from
Jan 26, 2022
Merged

Add MaskExtension #6554

merged 78 commits into from
Jan 26, 2022

Conversation

felixpalmer
Copy link
Collaborator

For #6552

Change List

  • Implementation of MaskExtension
  • Example app for website

Tests & documentation in future PR, publishing this minimal WIP to gather feedback on API and to allow downstream implementation in CARTO for React

@felixpalmer felixpalmer self-assigned this Jan 10, 2022
@felixpalmer felixpalmer marked this pull request as draft January 10, 2022 17:47
@felixpalmer
Copy link
Collaborator Author

@Pessimistress I followed the code in the other extension as well as the shadow module to implement this.

One thing I would appreciate you commenting on the the maths around the offset projection. I think what I have is close, but not quite right, and I see precision errors when zoomed in close. Relevant code: https://github.com/visgl/deck.gl/pull/6554/files#diff-e7aa5378c14ee05c714d147abc0b53e860e4f422f9fc85cae7d44bb4f3e9971aR56-R93

@felixpalmer felixpalmer force-pushed the felix/mask-extension-wip branch 2 times, most recently from 8d17a1c to 48d1468 Compare January 13, 2022 09:07
Base automatically changed from felix/layer-pass-framebuffer to master January 13, 2022 11:37
@coveralls
Copy link

coveralls commented Jan 13, 2022

Coverage Status

Coverage decreased (-0.04%) to 80.26% when pulling 97be55a on felix/mask-extension-wip into 29373c6 on master.

Copy link
Collaborator

@Pessimistress Pessimistress left a comment

Choose a reason for hiding this comment

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

An extension is the wrong place to hold a FBO because it is run per layer. Mask(s) are global and shared and should be implemented in an Effect, which can be executed before and after any render and inject resources into all draw calls. Since we are adding a core operation prop it is not inappropriate to always load a MaskEffect by default. See how shadow FBOs are handled in https://github.com/visgl/deck.gl/blob/master/modules/core/src/effects/lighting/lighting-effect.js#L62

examples/website/mask/README.md Outdated Show resolved Hide resolved
modules/core/src/lib/deck.js Outdated Show resolved Hide resolved
modules/core/src/lib/deck.js Outdated Show resolved Hide resolved
modules/core/src/lib/composite-layer.js Outdated Show resolved Hide resolved
modules/core/src/lib/composite-layer.js Outdated Show resolved Hide resolved
modules/core/src/lib/layer.js Outdated Show resolved Hide resolved
modules/core/src/effects/mask/utils.js Show resolved Hide resolved
modules/core/src/effects/mask/mask-effect.js Show resolved Hide resolved
modules/core/src/passes/draw-layers-pass.ts Show resolved Hide resolved
modules/core/src/passes/mask-pass.ts Outdated Show resolved Hide resolved
@felixpalmer felixpalmer merged commit cba5ddf into master Jan 26, 2022
@felixpalmer felixpalmer deleted the felix/mask-extension-wip branch January 26, 2022 15:08
@felixpalmer felixpalmer changed the title Mask Extension [WIP] Add MaskExtension Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants