[Webkit-unassigned] [Bug 122458] Simple line layout
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 18 09:23:20 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=122458
--- Comment #25 from Simon Fraser (smfr) <simon.fraser at apple.com> 2013-10-18 09:22:03 PST ---
(From update of attachment 214574)
View in context: https://bugs.webkit.org/attachment.cgi?id=214574&action=review
> Source/WebCore/dom/Position.cpp:650
> + textRenderer->ensureInlineBoxes();
It's unclear how callers can know that they should call ensureInlineBoxes()
> Source/WebCore/editing/TextIterator.cpp:524
> + if (renderer->simpleLineLayout()) {
Might be easier to read as usesSimpleLineLayout() or hasSimpleLineLayout(). It's a bit odd for an object to be called SimpleLineLayout.
> Source/WebCore/rendering/RenderBlock.h:793
> unsigned m_hasMarginBeforeQuirk : 1; // Note these quirk values can't be put in RenderBlockRareData since they are set too frequently.
> unsigned m_hasMarginAfterQuirk : 1;
> unsigned m_beingDestroyed : 1;
> unsigned m_hasMarkupTruncation : 1;
> unsigned m_hasBorderOrPaddingLogicalWidthChanged : 1;
> + unsigned m_forceLineboxInlineLayout : 1;
These can be bools, I think.
> Source/WebCore/rendering/RenderBlockFlow.cpp:2448
> + m_forceLineboxInlineLayout = true;
> +
> + if (!m_simpleLineLayout)
> + return;
> + m_simpleLineLayout = nullptr;
It's odd for something called ensureFoo to blow away existing state and recompute it. This function should be called updateFoo or recomputeFoo or something.
> Source/WebCore/rendering/RenderText.cpp:283
> + // FIXME: These will go away when simple layout can do everything.
But then it will no longer be simple?
> Source/WebCore/rendering/RenderText.cpp:308
> + const_cast<RenderText&>(*this).ensureInlineBoxes();
Again, not sure how I'd know, when writing new code, that I had to call ensureInlineBoxes().
> Source/WebCore/rendering/RenderText.cpp:1200
> + if (auto layout = simpleLineLayout())
> + return layout->caretMinOffset();
> return m_lineBoxes.caretMinOffset();
Shame we can't turn m_lineBoxes into some virtual thing that does both.
> Source/WebCore/rendering/ResolvedSimpleLines.h:38
> +class ResolvedSimpleLines {
The "resolved" doesn't really communicate anything I think.
> Source/WebCore/rendering/SimpleLineLayout.cpp:298
> + if (paintInfo.phase != PaintPhaseForeground)
> + return;
You can probably bail if painting is disabled (updateControlTints).
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list