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.

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

Output gradientstops in serialization and debug output #2709

Merged
merged 1 commit into from May 2, 2018

Conversation

@Gankra
Copy link
Contributor

Gankra commented Apr 30, 2018

Fixes #2328 (I think?)


This change is Reviewable

@glennw
Copy link
Member

glennw commented Apr 30, 2018

r? @kvark Does this fix up the serialization issue mentioned earlier?

Copy link
Member

kvark left a comment

Thank you for addressing this promptly!
Looks fine in general. One question before we proceed.

return None;
self.next_raw();
if let SetGradientStops = self.cur_item.item {
// SetGradientStops is a dummy item that most consumers should ignore

This comment has been minimized.

@kvark

kvark May 1, 2018

Member

can we leave this decision to the clients, given that they do know which items they need and which they don't?

This comment has been minimized.

@Gankra

Gankra May 1, 2018

Author Contributor

They can just use _raw if they want to make that decision, right? This method is for the "intended" case, where SetGradientStops is really just a quirk of our encoding.

@kvark
Copy link
Member

kvark commented May 1, 2018

I do confirm the PR fixes the issue with capturing gradients 👍 🎉

@kvark
Copy link
Member

kvark commented May 2, 2018

@bors-servo
Copy link
Contributor

bors-servo commented May 2, 2018

📌 Commit 4c187c2 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented May 2, 2018

Testing commit 4c187c2 with merge bd41fa8...

bors-servo added a commit that referenced this pull request May 2, 2018
Output gradientstops in serialization and debug output

Fixes #2328 (I think?)

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

bors-servo commented May 2, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing bd41fa8 to master...

@bors-servo bors-servo merged commit 4c187c2 into servo:master May 2, 2018
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
bors-servo added a commit that referenced this pull request May 2, 2018
Gracefully handle invalid gradient stops

This is not strictly required, just something I found useful when investigating captures on WR versions prior to #2709
I still think that the PR is an improvement, since the errors are recoverable.
r? anyone

<!-- 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/2719)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request May 4, 2018
Revert "Output gradientstops in serialization and debug output"

This reverts #2709 (commit 4c187c2) due to https://bugzilla.mozilla.org/show_bug.cgi?id=1459102
We'll need to investigate and do it again.

r? @staktrace

<!-- 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/2730)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request May 4, 2018
Revert "Output gradientstops in serialization and debug output"

This reverts #2709 (commit 4c187c2) due to https://bugzilla.mozilla.org/show_bug.cgi?id=1459102
We'll need to investigate and do it again.

r? @staktrace

<!-- 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/2730)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.