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

Merge final outline render into composite step in order to fix blending #1629

Merged
merged 5 commits into from
Mar 21, 2023

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Mar 20, 2023

Previously, blending was done in linear space between the outline layer but then in gamma space with the color target. This led to bad blend color & some artifacts:

Before:
Screenshot 2023-03-20 at 21 19 05

After:
Screenshot 2023-03-20 at 21 19 40

The way I solved this makes things a slightly more messy as the composite now needs to know some internals of outlines, but is almost certainly faster (haven't measured) and the easiest & quickest way at hand to solve the issue!

I hesitated at first, but I think now that we should stick with the "uber postprocessing shader" as long as we can: It makes it trivial to get an overview of what we're compositing, it won't get any faster and if we keep some internal separation it's not that messy.

Related to #1614 - the broken blending looked bad and makes it harder to tweak

Checklist

This makes things a little bit more messy (no separate step for outlines), but is almost certainly faster (haven't measured) and the easiest & quickest way at hand to solve the issue.
@Wumpf Wumpf added the 🔺 re_renderer affects re_renderer itself label Mar 20, 2023
@teh-cmc teh-cmc self-requested a review March 21, 2023 07:19
@Wumpf Wumpf merged commit 068428b into main Mar 21, 2023
@Wumpf Wumpf deleted the andreas/re_renderer/merge-outline-composition branch March 21, 2023 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔺 re_renderer affects re_renderer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants