Repository navigation
feat(server): advertise wp_alpha_modifier_v1 - #784
Open
isaksamsten wants to merge 1 commit into
Open
isaksamsten wants to merge 1 commit into
isaksamsten wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Talia preliminary review
| Check | Result |
|---|---|
| Merge score | 🟢 5/5 — No issues found; ready for CI and human review |
| Findings | None found |
| Review depth | 4/5 — how thoroughly the changed code could be checked by reading it |
| Coverage | 2 of 2 changed files reviewed |
What Talia checked: Checked the new global creation and filter ordering in Server::start, the 33-entry security-context allowlist and its harness assertion, and the pre-existing alpha-modifier consumers in umbrielfx scene opacity, overview mirroring, and View::fullscreenOpaque; no introduced defect was supported, with only wlroots-internal signal details left uninspected.
Run details
- Commit reviewed: e483847
- Source lookups: 26 file reads, 26 searches
- Prompt version: 859b7490ca3d
Preliminary automated review: Talia read the changed code and related source; nothing was built, run or tested, and AI can make mistakes. It is not a scope acceptance or a promise to merge; a human reviewer makes the final call. See our ethos.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Create the
wp_alpha_modifier_v1global, and expose it to security-context (sandboxed) clients.Umbriel already applies the client's alpha multiplier: umbrielfx multiplies it into the surface's scene opacity, and
View::fullscreenOpaquetreats a multiplier below 1 as translucent. But the global was never created, so no client could use the protocol. This addswlr_alpha_modifier_v1_createnext to the other per-surface protocols.Motivation
Clients that fade or dim their own windows have to redraw every buffer with the alpha baked in. With alpha-modifier they set one multiplier and the compositor applies it while compositing.
Real users:
SetLayeredWindowAttributes/UpdateLayeredWindow) onto alpha-modifier-v1 (wine!11119).Sandboxed apps get the global because it only affects the client's own surfaces. It adds nothing beyond what an ARGB buffer can already do, doesn't change the input region, and sends no events back. That puts it in the same group as viewporter, fractional-scale, and content-type. Wine is often run from Flatpak (Steam, Bottles), so hiding it would mainly cost those users the cheaper path.
Type of Change
Testing
protocol/security_contextpasses.wp_alpha_modifier_v1is now inNORMAL_GLOBALS, so the check confirms the global exists and that both ordinary and security-context clients see it.protocol/security_context_rulealso passes.meson compile -C build-debug umbriel tests/global-client tests/security-context-clientandtests/harness/check.sh ./build-debug/umbriel protocol/security_context.just checkcouldn't buildharness-clientson my machine: the system wayland-protocols is 1.47, which lacks the text-input-v3actionevent thatseat_log_client.cppuses. That's unrelated to this change.Manual Coverage
Checklist
SCOPE.md, or its scope was agreed in an issue or on Discord first.CONTRIBUTING.md.just format, or this PR has no C++, shader, or Markdown changes.docs/andexamples/config.toml, or this PR does not change user-facing configuration or behavior.