Skip to content

Commit

Permalink
fix(Metal): fix incorrect premultiplication of colors (#5172)
Browse files Browse the repository at this point in the history
`load_color` was multiplying the alpha channel by itself as well, rather
than just the `rgb` channels.

Also made sure to divide alpha out before applying gamma encoding back
to text color when not using linear blending, which was another source
of error.

Probably fixes the problem observed in #5133 -- I couldn't reproduce the
exact color shift, but did see a similar one with some of my test
colors. This most visibly affects dimmed text since it uses an alpha of
less than 1 for the text color.
  • Loading branch information
mitchellh authored Jan 17, 2025
2 parents a185ce3 + 2a1b51e commit 72d0855
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/renderer/shaders/cell.metal
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ float4 load_color(
// already have the correct color here and
// can premultiply and return it.
if (display_p3 && !linear) {
color *= color.a;
color.rgb *= color.a;
return color;
}

Expand Down Expand Up @@ -167,7 +167,7 @@ float4 load_color(
}

// Premultiply our color by its alpha.
color *= color.a;
color.rgb *= color.a;

return color;
}
Expand Down Expand Up @@ -503,12 +503,12 @@ fragment float4 cell_text_fragment(
// If we're not doing linear blending, then we need to
// re-apply the gamma encoding to our color manually.
//
// We do it BEFORE premultiplying the alpha because
// we want to produce the effect of not linearizing
// it in the first place in order to match the look
// of software that never does this.
// Since the alpha is premultiplied, we need to divide
// it out before unlinearizing and re-multiply it after.
if (!uniforms.use_linear_blending) {
color.rgb /= color.a;
color = unlinearize(color);
color.rgb *= color.a;
}

// Fetch our alpha mask for this pixel.
Expand Down

0 comments on commit 72d0855

Please sign in to comment.