Skip to content

Commit

Permalink
[LayoutNG] Add initial support for collecting atomic inlines
Browse files Browse the repository at this point in the history
Change NGInlineBox::PrepareLayout to collect all child nodes instead of
just non-atomic inlines and update the CollectNode method to insert the
object replacement unicode character in place of atomic inline objects.
This allows bidi resolution and text shaping to do the right thing when
operating on the entire paragraph.

Also removes the ComputedStyle pointer from NGLayoutInlineItem as it is
not needed at this point.

R=ikilpatrick@chromium.org
BUG=636993
TEST=Source/web/tests/NGInlineLayoutTest.cpp

Review-Url: https://codereview.chromium.org/2488163003
Cr-Commit-Position: refs/heads/master@{#431214}
  • Loading branch information
eaenet authored and Commit bot committed Nov 10, 2016
1 parent 6865095 commit ce75c6c
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 23 deletions.
23 changes: 14 additions & 9 deletions third_party/WebKit/Source/core/layout/ng/ng_inline_box.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "core/layout/ng/ng_fragment_builder.h"
#include "core/layout/ng/ng_length_utils.h"
#include "core/layout/ng/ng_writing_mode.h"
#include "wtf/text/CharacterNames.h"

namespace blink {

Expand All @@ -29,14 +30,8 @@ void NGInlineBox::PrepareLayout() {
// Scan list of siblings collecting all in-flow non-atomic inlines. A single
// NGInlineBox represent a collection of adjacent non-atomic inlines.
last_inline_ = start_inline_;
for (LayoutObject* curr = start_inline_; curr; curr = curr->nextSibling()) {
// TODO(layout-dev): We may want to include floating and OOF positioned
// objects.
if (curr->isAtomicInlineLevel() && !curr->isFloating() &&
!curr->isOutOfFlowPositioned())
break;
for (LayoutObject* curr = start_inline_; curr; curr = curr->nextSibling())
last_inline_ = curr;
}

CollectInlines(start_inline_, last_inline_);
CollapseWhiteSpace();
Expand All @@ -49,14 +44,24 @@ void NGInlineBox::PrepareLayout() {
// parent LayoutInline where possible, and joining all text content in a single
// string to allow bidi resolution and shaping of the entire block.
void NGInlineBox::CollectNode(LayoutObject* node, unsigned* offset) {
unsigned length = 0;
if (node->isText()) {
const String& text = toLayoutText(node)->text();
unsigned length = text.length();
length = text.length();
text_content_.append(text);
}

// For atomic inlines add a unicode "object replacement character" to signal
// the presence of a non-text object to the unicode bidi algorithm.
else if (node->isAtomicInlineLevel()) {
text_content_.append(objectReplacementCharacter);
length = 1;
}

if (length) {
unsigned start_offset = *offset;
*offset = *offset + length;
items_.append(NGLayoutInlineItem(node->style(), start_offset, *offset));
items_.append(NGLayoutInlineItem(start_offset, *offset));
}
}

Expand Down
6 changes: 2 additions & 4 deletions third_party/WebKit/Source/core/layout/ng/ng_inline_box.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,11 @@ class CORE_EXPORT NGInlineBox final
// element where possible.
class NGLayoutInlineItem {
public:
NGLayoutInlineItem(const ComputedStyle* style, unsigned start, unsigned end)
NGLayoutInlineItem(unsigned start, unsigned end)
: start_offset_(start),
end_offset_(end),
direction_(LTR),
script_(USCRIPT_INVALID_CODE),
style_(style) {
script_(USCRIPT_INVALID_CODE) {
DCHECK(end >= start);
}

Expand All @@ -104,7 +103,6 @@ class NGLayoutInlineItem {
UScriptCode script_;
// FontFallbackPriority fallback_priority_;
// bool rotate_sideways_;
const ComputedStyle* style_;
RefPtr<ShapeResult> shape_result_;

friend class NGInlineBox;
Expand Down
54 changes: 44 additions & 10 deletions third_party/WebKit/Source/web/tests/NGInlineLayoutTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,24 @@
#include "web/tests/sim/SimRequest.h"
#include "web/tests/sim/SimTest.h"
#include "wtf/CurrentTime.h"
#include "wtf/text/CharacterNames.h"

namespace blink {

class NGInlineLayoutTest : public SimTest {};
class NGInlineLayoutTest : public SimTest {
public:
NGConstraintSpace* constraintSpaceForElement(LayoutNGBlockFlow* blockFlow) {
NGConstraintSpaceBuilder builder(
FromPlatformWritingMode(blockFlow->style()->getWritingMode()));
builder.SetAvailableSize(NGLogicalSize(LayoutUnit(), LayoutUnit()));
builder.SetPercentageResolutionSize(
NGLogicalSize(LayoutUnit(), LayoutUnit()));
NGConstraintSpace* constraintSpace = new NGConstraintSpace(
FromPlatformWritingMode(blockFlow->style()->getWritingMode()),
blockFlow->style()->direction(), builder.ToConstraintSpace());
return constraintSpace;
}
};

TEST_F(NGInlineLayoutTest, BlockWithSingleTextNode) {
RuntimeEnabledFeatures::setLayoutNGEnabled(true);
Expand All @@ -33,15 +47,7 @@ TEST_F(NGInlineLayoutTest, BlockWithSingleTextNode) {

Element* target = document().getElementById("target");
LayoutNGBlockFlow* blockFlow = toLayoutNGBlockFlow(target->layoutObject());

NGConstraintSpaceBuilder builder(
FromPlatformWritingMode(blockFlow->style()->getWritingMode()));
builder.SetAvailableSize(NGLogicalSize(LayoutUnit(), LayoutUnit()));
builder.SetPercentageResolutionSize(
NGLogicalSize(LayoutUnit(), LayoutUnit()));
NGConstraintSpace* constraintSpace = new NGConstraintSpace(
FromPlatformWritingMode(blockFlow->style()->getWritingMode()),
blockFlow->style()->direction(), builder.ToConstraintSpace());
NGConstraintSpace* constraintSpace = constraintSpaceForElement(blockFlow);

NGInlineBox* inlineBox = new NGInlineBox(blockFlow->firstChild());
NGInlineLayoutAlgorithm* layoutAlgorithm = new NGInlineLayoutAlgorithm(
Expand All @@ -54,4 +60,32 @@ TEST_F(NGInlineLayoutTest, BlockWithSingleTextNode) {
layoutAlgorithm->Layout(&fragment);
}

TEST_F(NGInlineLayoutTest, BlockWithTextAndAtomicInline) {
RuntimeEnabledFeatures::setLayoutNGEnabled(true);
RuntimeEnabledFeatures::setLayoutNGInlineEnabled(true);

SimRequest mainResource("https://example.com/", "text/html");
loadURL("https://example.com/");
mainResource.complete("<div id=\"target\">Hello <img>.</div>");

compositor().beginFrame();
ASSERT_FALSE(compositor().needsBeginFrame());

Element* target = document().getElementById("target");
LayoutNGBlockFlow* blockFlow = toLayoutNGBlockFlow(target->layoutObject());
NGConstraintSpace* constraintSpace = constraintSpaceForElement(blockFlow);

NGInlineBox* inlineBox = new NGInlineBox(blockFlow->firstChild());
NGInlineLayoutAlgorithm* layoutAlgorithm = new NGInlineLayoutAlgorithm(
blockFlow->style(), inlineBox, constraintSpace);

String expectedText("Hello ");
expectedText.append(objectReplacementCharacter);
expectedText.append(".");
EXPECT_EQ(expectedText, inlineBox->Text(0, 8));

NGPhysicalFragmentBase* fragment;
layoutAlgorithm->Layout(&fragment);
}

} // namespace blink

0 comments on commit ce75c6c

Please sign in to comment.