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.

Sign up for GitHub

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

Jump to bottom

Add canvas as a more general version of the shadow primitive. #1807

Merged
merged 1 commit into from
Oct 5, 2017
Merged

Add canvas as a more general version of the shadow primitive. #1807

merged 1 commit into from
Oct 5, 2017

Conversation

glennw
Copy link
Member

@glennw glennw commented Oct 5, 2017 •
edited by larsbergstrom
Loading

This doesn't add any functional changes, but it lays the foundation
for moving box shadows and other render task operations over to
unified as canvas primitives.

This change is Reviewable

@glennw
Copy link
Member Author

glennw commented Oct 5, 2017

r? @nical or @kvark

Pending try:

https://hg.mozilla.org/try/rev/2ec84cb60f7fa2098f25c5eb8e030e877dedad7b
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ec84cb60f7fa2098f25c5eb8e030e877dedad7b

nical
nical requested changes Oct 5, 2017
View reviewed changes
Copy link
Contributor

@nical nical left a comment

Choose a reason for hiding this comment

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

I am not found of the idea of overloading the term canvas. Can we call this LayerEffect, CompositingEffect, FilterTarget or something else that sounds nice but doesn't use "canvas"?

Other than that, looks good to me.

@glennw
Copy link
Member Author

glennw commented Oct 5, 2017

@nical I changed it to Picture - does that sound more reasonable?

@gw3583
Add picture as a more general version of the shadow primitive.
84b0bd1 
This doesn't add any functional changes, but it lays the foundation
for moving box shadows and other render task operations over to
unified as picture primitives.
nical
nical approved these changes Oct 5, 2017
View reviewed changes
Copy link
Contributor

@nical nical left a comment

Choose a reason for hiding this comment

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

Picture is fine by me (could be more descriptive, but at least it doesn't collide with the name of a graphics web API).

@nical
Copy link
Contributor

nical commented Oct 5, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 84b0bd1 has been approved by nical

@bors-servo
Copy link
Contributor

⌛ Testing commit 84b0bd1 with merge cb06be0...

bors-servo pushed a commit that referenced this pull request Oct 5, 2017
Auto merge of #1807 - glennw:canvas1, r=nical
cb06be0 
Add canvas as a more general version of the shadow primitive.

This doesn't add any functional changes, but it lays the foundation
for moving box shadows and other render task operations over to
unified as canvas primitives.

<!-- 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/1807)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: nical
Pushing cb06be0 to master...

@bors-servo bors-servo merged commit 84b0bd1 into servo:master Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nical nical nical approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@glennw @nical @bors-servo @gw3583