Skip to content

/ webrender

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

Pre-multiplied alpha for the blend pass #1493

Merged
merged 1 commit into from Jul 19, 2017
Merged

Pre-multiplied alpha for the blend pass #1493

merged 1 commit into from Jul 19, 2017

Conversation

@kvark
Copy link
Member

kvark commented Jul 18, 2017 •
edited

Fixes #1449, fixes #1354
Running Servo tests is pending... Done.

This change is Reviewable
@kvark kvark changed the title Pre-multiplied alpha for the blend pass [WIP] Pre-multiplied alpha for the blend pass Jul 18, 2017
@kvark kvark force-pushed the kvark:opacity branch from 70cc535 to babcb1c Jul 18, 2017
@kvark kvark changed the title [WIP] Pre-multiplied alpha for the blend pass Pre-multiplied alpha for the blend pass Jul 18, 2017
@kvark
Copy link
Member Author

kvark commented Jul 18, 2017

r? @rlhunt
@eqrion
eqrion approved these changes Jul 19, 2017
View changes
wrench/reftests/filters/opacity-combined.yaml Outdated
@@ -0,0 +1,10 @@
# this tests that opacity combination respets pre-multiplied alpha

This comment has been minimized.

Sign in to view
@eqrion

eqrion Jul 19, 2017

Contributor

s/respets/respects

ditto in opacity-combined-ref.yaml
wrench/reftests/filters/opacity-combined-ref.yaml
items:
- type: rect
bounds: [10, 10, 100, 100]
color: green

This comment has been minimized.

Sign in to view
@eqrion

eqrion Jul 19, 2017

Contributor

Missing newline at end of file.
@eqrion
Copy link
Contributor

eqrion commented Jul 19, 2017

Thanks for adding tests! 👍
@kvark
Pre-multiplied alpha for the blend pass
Loading status checks…
89dd7ea
@kvark kvark force-pushed the kvark:opacity branch from babcb1c to 89dd7ea Jul 19, 2017
@kvark
Copy link
Member Author

kvark commented Jul 19, 2017

Thanks Ryan!
Merging, per discussion on IRC with Glenn...
@bors-servo r=glennw, rhunt
@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

📌 Commit 89dd7ea has been approved by glennw,
@kvark
Copy link
Member Author

kvark commented Jul 19, 2017

Hmm, maybe the extra space got in the way, trying again:
@bors-servo r=glennw,rhunt
@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #1499
@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

📌 Commit 89dd7ea has been approved by glennw,rhunt
@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

⌛ Testing commit 89dd7ea with merge 831b1e9...
bors-servo added a commit that referenced this pull request Jul 19, 2017
@bors-servo
Auto merge of #1493 - kvark:opacity, r=glennw,rhunt
Loading status checks…
831b1e9 
Pre-multiplied alpha for the blend pass

Fixes #1449, fixes #1354
Running Servo tests is pending... Done.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1493)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 19, 2017

☀️ Test successful - status-travis
Approved by: glennw,rhunt
Pushing 831b1e9 to master...
@bors-servo bors-servo merged commit 89dd7ea into servo:master Jul 19, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@kvark kvark deleted the kvark:opacity branch Jul 19, 2017
@mstange
Copy link
Contributor

mstange commented Jul 19, 2017

Do the other functions in ps_blend.fs.glsl need to be adjusted to work with premultiplied alpha as well? For example the brightness filter may need to unpremultiply the input, apply the brightness factor to the rgb values, and then premultiply the output again.
@mstange
Copy link
Contributor

mstange commented Jul 21, 2017

I've filed #1507 and #1508 about my previous comment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eqrion eqrion

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Opacity group compositing treats premultiplied RGBA as unpremultiplied RGBA
Unexpected behavior when combining filters
4 participants
@kvark @eqrion @bors-servo @mstange
You can’t perform that action at this time.