blender/blender

Compositor: Add Bloom option to Glare node #119128

Merged

Omar Emara merged 5 commits from OmarEmaraDev/blender:cpu-bloom-glare into main

2024-03-11 08:44:03 +01:00

Omar Emara commented

2024-03-06 16:28:20 +01:00

Member

This patch implements the GPU Bloom glare for the CPU compositor, and
adds a new option for it, leaving the Fog Glow option unimplemented once
again for the GPU compositor.

This patch implements the GPU Bloom glare for the CPU compositor, and adds a new option for it, leaving the Fog Glow option unimplemented once again for the GPU compositor.

Omar Emara added the

labels 2024-03-06 16:28:20 +01:00

Omar Emara added 1 commit 2024-03-06 16:28:32 +01:00

Compositor: Add Bloom option to Glare node 87624c8623

This patch implements the GPU Bloom glare for the CPU compositor, and
adds a new option for it, leaving the Fog Glow option unimplemented once
again for the GPU compositor.

Omar Emara added 3 commits 2024-03-07 11:59:08 +01:00

Use proper boundary condition 1bfa485694

Correct weighted sum 96c3cc975f

Correct inverted Karis average condition 3614d66b31

Omar Emara changed title from ~~WIP: Compositor: Add Bloom option to Glare node~~ to Compositor: Add Bloom option to Glare node

2024-03-07 15:59:04 +01:00

Omar Emara added 1 commit 2024-03-07 15:59:08 +01:00

Merge branch 'main' into cpu-bloom-glare

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-coordinator Build done.

Details

Omar Emara commented

2024-03-07 15:59:24 +01:00

Author

Member

@blender-bot build

@blender-bot build

Omar Emara requested review from Sergey Sharybin 2024-03-07 16:51:03 +01:00

Sergey Sharybin commented

2024-03-07 17:56:35 +01:00

Owner

It is definitely great to implement Bloom on CPU. The code for this looks good.

What I wonder is whether we should have both Fog Glow and Bloom. The Fog Glow is tricky for GPU, and it seems to be possible to configure Bloom to kind of match the Fog Glow by tweaking Threshold.

Maybe we can land this PR, and then make another one to do-version Fog Glow to Bloom, hide the option from the UI and not worry about it until there is a clear case where Fog Glare is really missing and Bloom can't handle specific setup?

It is definitely great to implement Bloom on CPU. The code for this looks good. What I wonder is whether we should have both Fog Glow and Bloom. The Fog Glow is tricky for GPU, and it seems to be possible to configure Bloom to kind of match the Fog Glow by tweaking Threshold. Maybe we can land this PR, and then make another one to do-version Fog Glow to Bloom, hide the option from the UI and not worry about it until there is a clear case where Fog Glare is really missing and Bloom can't handle specific setup?

Omar Emara commented

2024-03-07 18:23:10 +01:00

Author

Member

@Sergey I was planning to make a DevTalk post once this lands, where users can test both methods, and hopefully reach a decision through user feedback then.

However, I think part of the reason why Fog Glow seems useless is because it uses a mediocre PSF. And using a proper SPF would make it much more useful and even preferred over bloom for most realistic renders. So the right decision is probably to replace fog glow with a proper glare implementation. But it is of course a rather technically challenging target for GPU.

Here is an exaggerated comparison between, from left to right, bloom glare, fog glow glare, photopic glare, mesopic glare, and scotopic glare. The latter 3 are an implementation I did years back for the ecycles fork based on the SPF described in the paper "Physically-Based Glare Effects for Digital Images". Which shows that a convolution-based glare can be rather attractive and useful for users.

In any case, I think regardless of what we decide, having an option for bloom is good to have nonetheless.

@Sergey I was planning to make a DevTalk post once this lands, where users can test both methods, and hopefully reach a decision through user feedback then. However, I think part of the reason why Fog Glow seems useless is because it uses a mediocre PSF. And using a proper SPF would make it much more useful and even preferred over bloom for most realistic renders. So the right decision is probably to replace fog glow with a proper glare implementation. But it is of course a rather technically challenging target for GPU. Here is an exaggerated comparison between, from left to right, bloom glare, fog glow glare, photopic glare, mesopic glare, and scotopic glare. The latter 3 are an implementation I did years back for the ecycles fork based on the SPF described in the paper "Physically-Based Glare Effects for Digital Images". Which shows that a convolution-based glare can be rather attractive and useful for users. ![glareComparison.png](/attachments/a2d74272-f0b4-43ed-8d75-2ebaa097cf04) In any case, I think regardless of what we decide, having an option for bloom is good to have nonetheless.

glareComparison.png

423 KiB

Sergey Sharybin commented

2024-03-08 10:35:25 +01:00

Owner

For the Fog Glow, surely improvements are possible, as you've described. I'm just not sure what the best plan would be. Within its current limitations I do not see huge advantage of it over the Bloom. So maybe what we can do is:

Land Bloom (as it is undeniably useful addition, and, I think, the PR is actually ready)
Keep Fog Glow as it is in the code
In the release notes mention that the Bloom is what has closest behavior to the Fog Glow, and that it is supported on both CPU and GPU
But also mention that there are plans to replace Fog Glow with a physically based implementation, which will give better results but will be seen as breaking change

What do you think of this plan?

For the Fog Glow, surely improvements are possible, as you've described. I'm just not sure what the best plan would be. Within its current limitations I do not see huge advantage of it over the Bloom. So maybe what we can do is: - Land Bloom (as it is undeniably useful addition, and, I think, the PR is actually ready) - Keep Fog Glow as it is in the code - In the release notes mention that the Bloom is what has closest behavior to the Fog Glow, and that it is supported on both CPU and GPU - But also mention that there are plans to replace Fog Glow with a physically based implementation, which will give better results but will be seen as breaking change What do you think of this plan?

Omar Emara commented

2024-03-08 10:41:27 +01:00

Author

Member

@Sergey That sounds good.

One thing I forgot to mention is that the Bloom implementation needs to be adjusted to be more "energy conserving", to make it more like Fog Glow, since currently if you give bloom a highlight with a value of 10, it will create a bloom with a value of ~35. Users didn't notice that for EEVEE, but it will be a problem for compositing I think. So I will think about that once this lands.

@Sergey That sounds good. One thing I forgot to mention is that the Bloom implementation needs to be adjusted to be more "energy conserving", to make it more like Fog Glow, since currently if you give bloom a highlight with a value of 10, it will create a bloom with a value of ~35. Users didn't notice that for EEVEE, but it will be a problem for compositing I think. So I will think about that once this lands.

Sergey Sharybin approved these changes 2024-03-08 10:54:36 +01:00

Sergey Sharybin commented

2024-03-08 10:55:47 +01:00

Owner

@OmarEmaraDev Lets land it then! I've marked the PR as green now. Probably should have done it a while ago, as the change itself was good, and the discussion was more about plans of how to move forward after that.

@OmarEmaraDev Lets land it then! I've marked the PR as green now. Probably should have done it a while ago, as the change itself was good, and the discussion was more about plans of how to move forward after that.

Omar Emara referenced this issue from a commit

2024-03-11 08:44:03 +01:00

Compositor: Add Bloom option to Glare node

Omar Emara merged commit f4f22b64eb into main

2024-03-11 08:44:03 +01:00

Omar Emara deleted branch cpu-bloom-glare

2024-03-11 08:44:05 +01:00

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Automated Testing

Blender Asset Bundle

Code Documentation

Dependency Graph

Development Management

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Viewport & EEVEE

Virtual Reality

Asset Browser Project

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Viewport & EEVEE

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#119128

No description provided.