Skip to content

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Aug 29, 2025

Supersedes #1121

cc @Keavon does this address your need/use case?

cc @mTvare6, as this impacts your PRs

@DJMcNab DJMcNab added enhancement New feature or request C-classic Applies to the classic `vello` crate labels Aug 29, 2025
@nicoburns
Copy link
Contributor

Would it make sense to generalise this new method to allow alpha masks too? I suspect that might be quite convenient for implementing https://developer.mozilla.org/en-US/docs/Web/CSS/mask-image...

@DJMcNab
Copy link
Member Author

DJMcNab commented Sep 1, 2025

I suppose so; it's not entirely clear to me whether that is already covered by an existing blend mode. But this operation has been discussed as blocking for Graphite, whereas alpha masks have not been.

Either way, that should be a separate PR to this (but before 0.6.0, probably).

@DJMcNab DJMcNab force-pushed the luminance-mask-layer branch from 1903894 to 2a6e445 Compare September 1, 2025 09:38
@mTvare6
Copy link
Contributor

mTvare6 commented Sep 1, 2025

Supersedes #1121

cc @Keavon does this address your need/use case?

cc @mTvare6, as this impacts your PRs

It does address the use case and works correctly, thanks.

if end_clip.blend == LUMINANCE_MASK_LAYER {
// TODO: Does this case apply more generally?
// See https://github.com/linebender/vello/issues/1061
// TODO: How do we handle anti-aliased edges here?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Presumably they're accounted for in the multiplication of rgba[i] and area[i] above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Kind of; area[i] indicates "how much the blending should apply to this pixel".

However, we're instead using it as an alpha mask over the foreground. For Porter-Duff Over, this is equivalent. However, for the "destructive" blend modes, that doesn't give anywhere near the same results.
In the extreme case, consider the case where area[i] is zero; that is the cause of #1061.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense. That seems unrelated to this PR and shouldn't block landing IMO. Let's try to resolve that separately.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As an aside, that issue is interesting because I don't think we anticipated that someone would push/pop an empty layer simply for the side effects of the blend operation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahah:

Note: Shape is defined by the mathematical description of the shape. A particular point is either inside the shape or it is not. There are no gradations.

https://drafts.fxtf.org/compositing/#whatiscompositing

Copy link
Collaborator

@dfrg dfrg left a comment

Choose a reason for hiding this comment

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

I didn't do a deep dive on the math but the pipeline changes LGTM and everything else seems reasonable with a few nits.

@DJMcNab DJMcNab force-pushed the luminance-mask-layer branch from cd611c4 to 09eea7d Compare September 4, 2025 14:16
@DJMcNab DJMcNab enabled auto-merge September 4, 2025 14:17
@DJMcNab DJMcNab added this pull request to the merge queue Sep 4, 2025
Merged via the queue into linebender:main with commit 0cb4afd Sep 4, 2025
17 checks passed
@DJMcNab DJMcNab deleted the luminance-mask-layer branch September 4, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-classic Applies to the classic `vello` crate enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants