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

layout: Re-enable parallel layout and refactor boxes significantly #2174

Merged
merged 1 commit into from
May 2, 2014

Conversation

pcwalton
Copy link
Contributor

layout: Re-enable parallel layout by removing all RefCell instances from Flows; in the process, remove InlineInfo in favor of the range-based design that was originally planned and halfway implemented.

Now, the DOM tree structure for inline flows is reflected not by a
series of arrays but instead by a flat list of ranges into the list of
boxes. As part of this, the border and padding fields, which were
incorrect in the case of inlines and necessitated separate
noncontent_inline_foo methods, have been merged into a single
border_padding field that is always guaranteed to be correct after
width assignment, even for inlines.

r? @SimonSapin and/or @larsbergstrom

Closes #1280
Closes #1926
Closes #1999
Closes #2013
Closes #2018

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
  • @pcwalton, please confirm that src/test/html/acid1.html and your favourite wikipedia page still render correctly!

@hoppipolla-critic-bot
Copy link

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

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 pcwalton changed the title Reparallelize layout: Re-enable parallel layout and refactor boxes significantly Apr 18, 2014
@pcwalton
Copy link
Contributor Author

Added a small commit to fix up a safety issue that I realized we have.

@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 19, 2014

Left a few comments on critic

@SimonSapin
Copy link
Member

@pcwalton Does this obsolete #2141? The first commit seems identical.

@pcwalton
Copy link
Contributor Author

@SimonSapin It's meant to be "based on top of" #2141, but I'll just close it.

@SimonSapin
Copy link
Member

I’ve edited the PR message to add "Closes #2018". Hopefully GitHub will do its magic.

/// New-line chracter(\n)'s positions(relative, not absolute)
///
/// FIXME(pcwalton): This is very inefficient; remove.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: #2260

@pcwalton
Copy link
Contributor Author

re-r? @SimonSapin. I had to fix an issue in border styles.

@pcwalton
Copy link
Contributor Author

pcwalton commented May 1, 2014

re-r? @SimonSapin

Filed #2288.

from `Flow`s; in the process, remove `InlineInfo` in favor of the
range-based design that was originally planned and halfway implemented.

Now, the DOM tree structure for inline flows is reflected not by a
series of arrays but instead by a flat list of ranges into the list of
boxes. As part of this, the `border` and `padding` fields, which were
incorrect in the case of inlines and necessitated separate
`noncontent_inline_foo` methods, have been merged into a single
`border_padding` field that is always guaranteed to be correct after
width assignment, even for inlines.
bors-servo pushed a commit that referenced this pull request May 2, 2014
layout: Re-enable parallel layout by removing all `RefCell` instances from `Flow`s; in the process, remove `InlineInfo` in favor of the range-based design that was originally planned and halfway implemented.

Now, the DOM tree structure for inline flows is reflected not by a
series of arrays but instead by a flat list of ranges into the list of
boxes. As part of this, the `border` and `padding` fields, which were
incorrect in the case of inlines and necessitated separate
`noncontent_inline_foo` methods, have been merged into a single
`border_padding` field that is always guaranteed to be correct after
width assignment, even for inlines.

r? @SimonSapin and/or @larsbergstrom

Closes #1280 
Closes #1926
Closes #1999 
Closes #2013
Closes #2018
@bors-servo bors-servo merged commit 27276c0 into servo:master May 2, 2014
@pcwalton pcwalton deleted the reparallelize branch May 2, 2014 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment