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

gfx: Use a pattern instead of tiling images manually. #6487

Merged
merged 1 commit into from Jul 7, 2015

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Jun 26, 2015

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jun 26, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5391

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@pcwalton
Copy link
Contributor Author

pcwalton commented Jun 26, 2015

@nickdesaulniers
Copy link

nickdesaulniers commented Jun 26, 2015

I tried applying this patch, building, and running my site. I can scroll a little down the page, and the page stops rendering everything below the first few paragraphs.

➜  servo git:(background-tiling) ✗ ./mach run http://nickdesaulniers.github.io -Z show-parallel-layout
...
thread 'LayoutWorker worker 5/6' panicked at 'Link to containing block not established; perhaps you forgot to call `set_absolute_descendants`?', /Users/Nicholas/Downloads/servo-osx/servo/components/layout/flow.rs:1393
stack backtrace:
   1:        0x1029827e5 - sys::backtrace::write::hab580a18e6ade3dauns
   2:        0x102985e05 - panicking::on_panic::h1844ce5c03c54289BFw
   3:        0x102973922 - rt::unwind::begin_unwind_inner::hbe10c68a88d62d39Nnw
   4:        0x10083ebfc - rt::unwind::begin_unwind::h10098823487077890900
   5:        0x100927d03 - flow::ContainingBlockLink::generated_containing_block_size::h1a5ba2c8a7d5f91f7Gi
   6:        0x10092799b - block::BlockFlow::containing_block_size::h97636cafe1bc91fbHfb
   7:        0x100951e9c - table_row::TableRowFlow.Flow::assign_inline_sizes::hd9086e5a721ee500LPt
   8:        0x100b4be80 - traversal::AssignISizes<'a>.PreorderFlowTraversal::process::h6192444e4c0168e46Uv
   9:        0x100b4af44 - parallel::AssignISizes<'a>.ParallelPreorderFlowTraversal::run_parallel::had59bbd2b3292c4397q
  10:        0x100b513f5 - parallel::assign_inline_sizes::ha687bdbbd05502cffbr
  11:        0x100ac895b - workqueue::WorkerThread<QueueData, WorkData>::start::h13553608466671330882
  12:        0x100ac8044 - workqueue::WorkQueue<QueueData, WorkData>::new::closure.43248
  13:        0x100ac7eaf - task::spawn_named::closure.43242
  14:        0x100ac7dbe - boxed::F.FnBox<A>::call_box::h13516651952116030504
  15:        0x100a9d624 - boxed::Box<FnBox<A, Output $u3d$$u20$R$GT$$u2b$$u20$Send$u20$$u2b$$u20$$u27$a$GT$.FnOnce$LT$A$GT$::call_once::h9166918206100653573
  16:        0x100a9d092 - thread::Builder::spawn_inner::closure.42472
  17:        0x100a9d00e - rt::unwind::try::try_fn::__rust_abi::h6837061623529591365
  18:        0x100a9cfa9 - rt::unwind::try::try_fn::h6837061623529591365
  19:        0x102987818 - rust_try_inner
  20:        0x102987805 - rust_try
  21:        0x102980d85 - rt::unwind::try::inner_try::h952058f93415498bGjw
  22:        0x100a9cf18 - rt::unwind::try::h15725383742404244903
  23:        0x100a9cd61 - thread::Builder::spawn_inner::closure.42424
  24:        0x100a9d93d - boxed::F.FnBox<A>::call_box::h9288818815509458392
  25:        0x1029847cd - sys::thread::Thread::new::thread_start::hce4e8f4a53df8c160Hv
  26:     0x7fff8e539267 - _pthread_body
  27:     0x7fff8e5391e4 - _pthread_start
thread 'LayoutWorker worker 5/6' panicked at 'Link to containing block not established; perhaps you forgot to call `set_absolute_descendants`?', /Users/Nicholas/Downloads/servo-osx/servo/components/layout/flow.rs:1393
stack backtrace:
   1:        0x1029827e5 - sys::backtrace::write::hab580a18e6ade3dauns
   2:        0x102985e05 - panicking::on_panic::h1844ce5c03c54289BFw
   3:        0x102973922 - rt::unwind::begin_unwind_inner::hbe10c68a88d62d39Nnw
   4:        0x10083ebfc - rt::unwind::begin_unwind::h10098823487077890900
   5:        0x100927d03 - flow::ContainingBlockLink::generated_containing_block_size::h1a5ba2c8a7d5f91f7Gi
   6:        0x10092799b - block::BlockFlow::containing_block_size::h97636cafe1bc91fbHfb
   7:        0x100951e9c - table_row::TableRowFlow.Flow::assign_inline_sizes::hd9086e5a721ee500LPt
   8:        0x100b4be80 - traversal::AssignISizes<'a>.PreorderFlowTraversal::process::h6192444e4c0168e46Uv
   9:        0x100b4af44 - parallel::AssignISizes<'a>.ParallelPreorderFlowTraversal::run_parallel::had59bbd2b3292c4397q
  10:        0x100b513f5 - parallel::assign_inline_sizes::ha687bdbbd05502cffbr
  11:        0x100ac895b - workqueue::WorkerThread<QueueData, WorkData>::start::h13553608466671330882
  12:        0x100ac8044 - workqueue::WorkQueue<QueueData, WorkData>::new::closure.43248
  13:        0x100ac7eaf - task::spawn_named::closure.43242
  14:        0x100ac7dbe - boxed::F.FnBox<A>::call_box::h13516651952116030504
  15:        0x100a9d624 - boxed::Box<FnBox<A, Output $u3d$$u20$R$GT$$u2b$$u20$Send$u20$$u2b$$u20$$u27$a$GT$.FnOnce$LT$A$GT$::call_once::h9166918206100653573
  16:        0x100a9d092 - thread::Builder::spawn_inner::closure.42472
  17:        0x100a9d00e - rt::unwind::try::try_fn::__rust_abi::h6837061623529591365
  18:        0x100a9cfa9 - rt::unwind::try::try_fn::h6837061623529591365
  19:        0x102987818 - rust_try_inner
  20:        0x102987805 - rust_try
  21:        0x102980d85 - rt::unwind::try::inner_try::h952058f93415498bGjw
  22:        0x100a9cf18 - rt::unwind::try::h15725383742404244903
  23:        0x100a9cd61 - thread::Builder::spawn_inner::closure.42424
  24:        0x100a9d93d - boxed::F.FnBox<A>::call_box::h9288818815509458392
  25:        0x1029847cd - sys::thread::Thread::new::thread_start::hce4e8f4a53df8c160Hv
  26:     0x7fff8e539267 - _pthread_body
  27:     0x7fff8e5391e4 - _pthread_start
libpng warning: IDAT: Too much image data
ERROR:script::dom::htmlscriptelement: error loading script Invalid Header provided

screen shot 2015-06-26 at 3 22 21 pm

@pcwalton
Copy link
Contributor Author

pcwalton commented Jun 26, 2015

@nickdesaulniers Yeah, that's a preexisting issue. Please file a new bug :)

@SimonSapin
Copy link
Member

SimonSapin commented Jun 26, 2015

@bors-servo r+


Reviewed 3 of 3 files at r1.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2015

📌 Commit 7546efc has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2015

Testing commit 7546efc with merge c3ea26f...

bors-servo pushed a commit that referenced this pull request Jun 27, 2015
gfx: Use a pattern instead of tiling images manually.

r? @SimonSapin

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6487)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2015

💔 Test failed - mac2

@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2015

The latest upstream changes (presumably #6493) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm
Copy link
Member

jdm commented Jul 6, 2015

failures:
    acid2_noscroll.html == acid2_ref_broken.html
    background_image_position_a.html == background_image_position_ref.html
    background_repeat_both_a.html == background_repeat_both_b.html
    background_repeat_y_a.html == background_repeat_y_b.html
    background_size_a.html == background_size_ref.html
@jdm jdm added S-tests-failed and removed S-awaiting-review labels Jul 6, 2015
@pcwalton pcwalton force-pushed the pcwalton:background-tiling branch from 7546efc to ae5965a Jul 7, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 7, 2015

OK, the tests should be fixed. Now depends on servo/rust-azure#177. r? @glennw

@pcwalton pcwalton force-pushed the pcwalton:background-tiling branch from ae5965a to c54a671 Jul 7, 2015
@glennw
Copy link
Member

glennw commented Jul 7, 2015

@pcwalton Fix the test-tidy failure, then r=me (once the cargo.lock files are all updated)

@pcwalton pcwalton force-pushed the pcwalton:background-tiling branch from c54a671 to 3500af3 Jul 7, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 7, 2015

@bors-servo: r=glennw

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

📌 Commit 3500af3 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

Testing commit 3500af3 with merge ae01e97...

bors-servo pushed a commit that referenced this pull request Jul 7, 2015
gfx: Use a pattern instead of tiling images manually.

r? @SimonSapin

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6487)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 3500af3 into servo:master Jul 7, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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

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