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

Split background.rs file in background, border and gradient #21972

Merged
merged 2 commits into from Oct 23, 2018

Conversation

@pyfisch
Copy link
Contributor

pyfisch commented Oct 17, 2018

I happened to read a style guide and found that many people use short function names but import only the parent module so the function is called gradient::linear instead of create_linear_gradient. (I usually prefix my function names with a verb but this does not appear to be the preferred style)

In the layout crate files quite often contain the line #![deny(unsafe_code)] but writing it once in lib.rs should suffice.

Finally the Webrender DisplayListBuilder is now bypassed for some items and they are directly pushed to the display list. (See #19676)


This change is Reviewable

@highfive
Copy link

highfive commented Oct 17, 2018

Heads up! This PR modifies the following files:

  • @emilio: components/layout/table_rowgroup.rs, components/layout/text.rs, components/layout/block.rs, components/layout/table_row.rs, components/layout/display_list/background.rs and 17 more
@highfive
Copy link

highfive commented Oct 17, 2018

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
@jdm
Copy link
Member

jdm commented Oct 17, 2018

@bors-servo r+
Thanks!

@bors-servo
Copy link
Contributor

bors-servo commented Oct 17, 2018

📌 Commit b2fbb04 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 17, 2018

Testing commit b2fbb04 with merge fc18070...

bors-servo added a commit that referenced this pull request Oct 17, 2018
Split background.rs file in background, border and gradient

I happened to read a style guide and found that many people use short function names but import only the parent module so the function is called gradient::linear instead of create_linear_gradient. (I usually prefix my function names with a verb but this does not appear to be the preferred style)

In the layout crate files quite often contain the line `#![deny(unsafe_code)]` but writing it once in `lib.rs` should suffice.

Finally the Webrender DisplayListBuilder is now bypassed for some items and they are directly pushed to the display list. (See #19676)

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

bors-servo commented Oct 17, 2018

💔 Test failed - linux-rel-css

@jdm
Copy link
Member

jdm commented Oct 17, 2018

A lot of output like:

  ▶ TIMEOUT [expected PASS] /css/css-flexbox/justify-content-004.htm
  │ 
  │ VMware, Inc.
  │ softpipe
  │ 3.3 (Core Profile) Mesa 18.1.0-devel
  │ MEH: malicious process?: Custom("invalid value: integer `160`, expected variant index 0 <= i < 22") (thread WRSceneBuilder#0, at libcore/result.rs:1009)
  │ stack backtrace:
  │    0:     0x7fb3f8da819c - backtrace::backtrace::trace::h34a5004b5e0f1dc7
  │    1:     0x7fb3f8da7432 - <backtrace::capture::Backtrace as core::default::Default>::default::hae829d87a5db9150
  │    2:     0x7fb3f8da74a8 - backtrace::capture::Backtrace::new::hcbbf9ea3df342ffc
  │    3:     0x7fb3f5c788c9 - servo::main::{{closure}}::h21eebb055abb3229
  │    4:     0x7fb3f8dd37d3 - std::panicking::rust_panic_with_hook::hd470ff3b3e7a1b18
  │                         at libstd/panicking.rs:480
  │    5:     0x7fb3f8dd3339 - std::panicking::continue_panic_fmt::hf0cf39ae1e114602
  │                         at libstd/panicking.rs:390
  │    6:     0x7fb3f8dd3235 - rust_begin_unwind
  │                         at libstd/panicking.rs:325
  │    7:     0x7fb3f8df443b - core::panicking::panic_fmt::hf53111ba26d35bb0
  │                         at libcore/panicking.rs:77
  │    8:     0x7fb3f819c67d - core::result::unwrap_failed::h83bba07137c7fd88
  │    9:     0x7fb3f81caf7b - webrender_api::display_list::BuiltDisplayListIter::next_raw::ha791545b37259620
  │   10:     0x7fb3f81cabb7 - webrender_api::display_list::BuiltDisplayListIter::next::hbead1b38a2f07408
  │   11:     0x7fb3f7c1c4b8 - webrender::display_list_flattener::DisplayListFlattener::flatten_item::hf49a498ee0f8cf64
  │   12:     0x7fb3f7c1911e - webrender::display_list_flattener::DisplayListFlattener::flatten_root::h196542530b9e355c
  │   13:     0x7fb3f7c18a12 - webrender::display_list_flattener::DisplayListFlattener::create_frame_builder::h4be3121894bf2322
  │   14:     0x7fb3f7c8e963 - webrender::scene_builder::SceneBuilder::run::h68b2b9ded54f78e0
  │   15:     0x7fb3f7be9c7e - std::sys_common::backtrace::__rust_begin_short_backtrace::h7a25c4c7a11a8494
  │   16:     0x7fb3f7c2fa35 - _ZN3std9panicking3try7do_call17hfb637ac78c682c6aE.llvm.17109957267228112710
  │   17:     0x7fb3f8de5649 - __rust_maybe_catch_panic
  │                         at libpanic_unwind/lib.rs:102
  │   18:     0x7fb3f7c7d240 - <F as alloc::boxed::FnBox<A>>::call_box::h4805a8f1f30ac19a
  │   19:     0x7fb3f8dd072a - <alloc::boxed::Box<(dyn alloc::boxed::FnBox<A, Output$u3d$R$GT$$u20$$u2b$$u20$$u27$a$RP$$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h9d1cd498499ed135
  │                         at liballoc/boxed.rs:682
  │                          - std::sys_common::thread::start_thread::ha4bfe77455e6e434
  │                         at libstd/sys_common/thread.rs:24
  │   20:     0x7fb3f8dbdcb5 - std::sys::unix::thread::Thread::new::thread_start::hebe133da3ea5230a
  │                         at libstd/sys/unix/thread.rs:90
  │   21:     0x7fb3f3fef183 - start_thread
  │   22:     0x7fb3f227b03c - clone
  │   23:                0x0 - <unknown>
@jdm
Copy link
Member

jdm commented Oct 17, 2018

@pyfisch pyfisch force-pushed the pyfisch:split-background-rs branch from b2fbb04 to c21d7f8 Oct 17, 2018
@pyfisch
Copy link
Contributor Author

pyfisch commented Oct 17, 2018

@bors-servo test=wpt

Had made an embarrassing mistake, should be fine now.

@jdm
Copy link
Member

jdm commented Oct 17, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Oct 17, 2018

📌 Commit c21d7f8 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 17, 2018

Testing commit c21d7f8 with merge 349dcc9...

bors-servo added a commit that referenced this pull request Oct 17, 2018
Split background.rs file in background, border and gradient

I happened to read a style guide and found that many people use short function names but import only the parent module so the function is called gradient::linear instead of create_linear_gradient. (I usually prefix my function names with a verb but this does not appear to be the preferred style)

In the layout crate files quite often contain the line `#![deny(unsafe_code)]` but writing it once in `lib.rs` should suffice.

Finally the Webrender DisplayListBuilder is now bypassed for some items and they are directly pushed to the display list. (See #19676)

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

bors-servo commented Oct 17, 2018

💔 Test failed - linux-rel-css

@pyfisch
Copy link
Contributor Author

pyfisch commented Oct 17, 2018

(@bors-servo retry) Not sure how to restart bors. Failures are AFAIK unrelated.

@jdm
Copy link
Member

jdm commented Oct 17, 2018

@bors-servo retry

Add license to two files.

Bypass DisplayListBuilder for some items.
@pyfisch pyfisch force-pushed the pyfisch:split-background-rs branch from c21d7f8 to d9b1950 Oct 22, 2018
@pyfisch
Copy link
Contributor Author

pyfisch commented Oct 22, 2018

@bors-servo try=wpt

Bug fixed, rebased.

@pyfisch
Copy link
Contributor Author

pyfisch commented Oct 22, 2018

rebased, bug fixed

@bors-servo try=wpt

@pyfisch
Copy link
Contributor Author

pyfisch commented Oct 22, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2018

Trying commit d9b1950 with merge ed6a15e...

bors-servo added a commit that referenced this pull request Oct 22, 2018
Split background.rs file in background, border and gradient

I happened to read a style guide and found that many people use short function names but import only the parent module so the function is called gradient::linear instead of create_linear_gradient. (I usually prefix my function names with a verb but this does not appear to be the preferred style)

In the layout crate files quite often contain the line `#![deny(unsafe_code)]` but writing it once in `lib.rs` should suffice.

Finally the Webrender DisplayListBuilder is now bypassed for some items and they are directly pushed to the display list. (See #19676)

<!-- 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/21972)
<!-- Reviewable:end -->
@jdm
Copy link
Member

jdm commented Oct 22, 2018

{"status": "FAIL", "group": "default", "message": "/css/CSS2/bidi-text/bidi-box-model-030.xht 4e6ec2a4e7504e5c4bb739eb4fad793251d3180f\n/css/CSS2/bidi-text/bidi-box-model-010-ref.xht ac8671852f0ec4ba6e1989b4cd260b24c9b9f9ec\nTesting 4e6ec2a4e7504e5c4bb739eb4fad793251d3180f == ac8671852f0ec4ba6e1989b4cd260b24c9b9f9ec", "stack": null, "subtest": null, "test": "/css/CSS2/bidi-text/bidi-box-model-030.xht", "line": 33242, "action": "test_result", "expected": "PASS"}

That was the only failure.

@bors-servo
Copy link
Contributor

bors-servo commented Oct 23, 2018

Trying commit d9b1950 with merge ec7aeb3...

bors-servo added a commit that referenced this pull request Oct 23, 2018
Split background.rs file in background, border and gradient

I happened to read a style guide and found that many people use short function names but import only the parent module so the function is called gradient::linear instead of create_linear_gradient. (I usually prefix my function names with a verb but this does not appear to be the preferred style)

In the layout crate files quite often contain the line `#![deny(unsafe_code)]` but writing it once in `lib.rs` should suffice.

Finally the Webrender DisplayListBuilder is now bypassed for some items and they are directly pushed to the display list. (See #19676)

<!-- 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/21972)
<!-- Reviewable:end -->
@jdm
Copy link
Member

jdm commented Oct 23, 2018

This time it was:

  ▶ FAIL [expected PASS] /_mozilla/css/position_relative_stacking_context_a.html
  └   → /_mozilla/css/position_relative_stacking_context_a.html 4e6ec2a4e7504e5c4bb739eb4fad793251d3180f
/_mozilla/css/position_relative_stacking_context_ref.html 314d194a3928aeb71b6b37421f080e1d509c7930
Testing 4e6ec2a4e7504e5c4bb739eb4fad793251d3180f == 314d194a3928aeb71b6b37421f080e1d509c7930
@jdm
Copy link
Member

jdm commented Oct 23, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Oct 23, 2018

📌 Commit d9b1950 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 23, 2018

Testing commit d9b1950 with merge 5422ca4...

bors-servo added a commit that referenced this pull request Oct 23, 2018
Split background.rs file in background, border and gradient

I happened to read a style guide and found that many people use short function names but import only the parent module so the function is called gradient::linear instead of create_linear_gradient. (I usually prefix my function names with a verb but this does not appear to be the preferred style)

In the layout crate files quite often contain the line `#![deny(unsafe_code)]` but writing it once in `lib.rs` should suffice.

Finally the Webrender DisplayListBuilder is now bypassed for some items and they are directly pushed to the display list. (See #19676)

<!-- 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/21972)
<!-- Reviewable:end -->
6 similar comments
@bors-servo bors-servo merged commit d9b1950 into servo:master Oct 23, 2018
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
@pyfisch pyfisch deleted the pyfisch:split-background-rs branch Oct 23, 2018
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.