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: tonemapping effect #148

Merged
merged 14 commits into from
Jan 10, 2025
Merged

Conversation

damienmontastier
Copy link
Collaborator

This component introduces the ToneMapping effect, which provides an abstraction for various tone mapping algorithms from the pmndrs/postprocessing package. The <ToneMapping> effect is specifically designed to improve the visual rendering of HDR content by mapping high-range brightness values to a displayable range, thus enhancing luminance and color balance in the scene.

  • Fixes potential issues with incorrect colors when EffectComposer is added by applying tone mapping manually as an effect.
  • Easy to set up — simply include it in your <EffectComposer> pipeline.
  • Multiple tone mapping modes and options for fine-tuning luminance are available.
  • Fully supports different blend functions for enhanced visual styles.

For more details, see ToneMapping on pmndrs/postprocessing.


Local Playground — pnpm run playground

Local Documentation — pnpm run docs:dev


@alvarosabu @Tinoooo

Copy link
Contributor

@Tinoooo Tinoooo left a comment

Choose a reason for hiding this comment

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

Thank you @damienmontastier for your PR. Very helpful!


The `<ToneMapping>` component is easy to set up and comes with multiple tone mapping modes to suit different visual requirements. Below is an example of how to use it in a Vue application.

```vue{2,4,7-8,32-36}
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: While I like to have a full example on the page, I think it is better to stay uniform with the other effects and only show the usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's a shame that we don't produce slightly advanced demos to present concrete use cases.

What do you think @alvarosabu ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. The downside would be, that there can be increased maintenance effort in case the parameters of the effects change upstream.

I'm fine as long as the docs are uniform.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave it the way it is for now and decide later. The advantage of having the effect outweighs the non-uniform docs.

Copy link
Member

Choose a reason for hiding this comment

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

Hi guys, I agree with Tino here, from a documentation perspective, it's always better to stay simple and focus on the usage of the effect. For advance demos we can always use the Cookbook https://docs.tresjs.org/cookbook/

@alvarosabu
Copy link
Member

Hi @damienmontastier, we will need to update this PR (and probably all the others) to adapt to the breaking change introduced in v2 https://github.com/Tresjs/post-processing/releases/tag/2.0.0

Mostly adding the correct Suffix to the effects if they are from the pmndrs/postprocessing package. Let me know if you have any questions or we can help you out.

Sorry about this

@damienmontastier
Copy link
Collaborator Author

@alvarosabu I've just adapted the ToneMapping component for V2 of the package ✌️

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@Tinoooo Tinoooo merged commit db77fc6 into Tresjs:main Jan 10, 2025
1 check passed
@Tinoooo Tinoooo changed the title feat: add tonemapping effect feat: tonemapping effect Jan 10, 2025
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