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
Switch from servo/angle to the mozangle crate #20216
Conversation
Heads up! This PR modifies the following files:
|
@bors-servo r+ Thanks! |
📌 Commit aa9f2f1 has been approved by |
Switch from servo/angle to the mozangle crate https://github.com/servo/mozangle <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20216) <!-- Reviewable:end -->
💔 Test failed - linux-dev |
Fixed import order. @bors-servo r=emilio |
📌 Commit 0ba0580 has been approved by |
📌 Commit 0ba0580 has been approved by |
Switch from servo/angle to the mozangle crate https://github.com/servo/mozangle <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20216) <!-- Reviewable:end -->
💔 Test failed - linux-rel-css |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me, with or without that. Thanks!
fn to_name_in_compiled_shader(s: &str) -> String { | ||
map_dot_separated(s, |s, mapped| { | ||
// This closure should match the behavior of the HashName function at: | ||
// https://github.com/servo/mozangle/blob/v0.1.4/gfx/angle/checkout/src/compiler/translator/HashNames.cpp#L56-L64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, the tidy name length lint, right? :/
Maybe move the comment to the line before fn
, saying "The closure below" instead?
// https://github.com/servo/mozangle/blob/v0.1.4/gfx/angle/checkout/src/compiler/translator/HashNames.cpp#L56-L64 | ||
|
||
const IDENTIFIER_MAX_LEN: usize = 1024; | ||
if (s.len() + ANGLE_NAME_PREFIX.len()) <= IDENTIFIER_MAX_LEN { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is still not quite correct, given s
here may contain indices and such, right? If we're going to avoid parsing the names completely, we can't deal with this correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually just realized that we don't even need this. WebGL has its own cap on identifier length, see MAX_UNIFORM_AND_ATTRIBUTE_LEN
.
So let's just remove this condition.
@bors-servo r=emilio |
📌 Commit 0a9ac20 has been approved by |
Switch from servo/angle to the mozangle crate https://github.com/servo/mozangle <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20216) <!-- Reviewable:end -->
💔 Test failed - linux-rel-wpt |
@bors-servo r=emilio |
📌 Commit 67d983c has been approved by |
Switch from servo/angle to the mozangle crate https://github.com/servo/mozangle <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20216) <!-- Reviewable:end -->
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev |
https://github.com/servo/mozangle
This change is