2011-04-04 Eric Seidel <eric@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Apr 2011 02:12:31 +0000 (02:12 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Apr 2011 02:12:31 +0000 (02:12 +0000)
commitd2320c986297551141ab1347b66ad899ce577db4
treefbbe15f7a49fa42c7eac4dbed09e919fbee55ce2
parent5f5e528330f79b5954bb8764409f4d58f2fa8ca6
2011-04-04  Eric Seidel  <eric@webkit.org>

        Reviewed by Ryosuke Niwa.

        Split run storage out from BidiResolver into a new BidiRunList class
        https://bugs.webkit.org/show_bug.cgi?id=57764

        Part of what makes BidiResolver and InlineIterator so difficult to understand
        (and bug 50912 so difficult to fix) is that BidiResolver is both a state machine
        for the Unicode Bidi Algorithm (UBA) as well as storage for the resulting
        BidiRuns from the algorithm.  This patch breaks the storage aspect off
        into its own class BidiRunList.

        This patch is only a start.  It does not actually fix the problematic ownership
        relationship, but it does make it possible to fix such in a second patch.

        The run pointers and addRun/prependRun, etc. were already a tightly coupled
        logical subset of the BidiResolver class, so moving them into their own class
        was both obvious and easy.  The only piece of logic I had to move was that
        deleteRuns() had a side-effect of setting the m_emptyRun bit on the resolver.

        I believe this deleteRuns side-effect was only ever used for one place
        (right after findNextLineBreak) and that it's only needed because
        findNextLineBreak can sometimes create bidi runs.  Run creation appears to be
        an unintentional side-effect of how InlineIterator calls through to BidiResolver
        as part of bidiNext and not a desired effect of the code.  I have added the call
        to markCurrentRunEmpty to both places deleteRuns was called (where the resolver
        could get re-used) as a safety precaution.  We could replace both with ASSERTs
        in a later patch.

        I suspect there may be a small performance win from further refactoring so that
        findNextLineBreak does not need to create BidiRuns.

        As I commented in the code, callers should own their own BidiRunList which they
        pass to BidiResolver::createBidiRunsForLine.  I attempted to implement that in
        an earlier version of this patch, but it was too complicated with the current
        twisted dependencies between InlineIterator/bidiNext and InlineBidiResolver.
        raise/lowerExplicitEmbeddingLevel are called unconditionally
        from commitExplicitEmbedding (which is called by bidiNext) and expect to have
        access to a runs list even in cases where we don't want any runs (findNextLineBreak).

        I also cleaned up some of the callers to pass around BidiRunList objects instead
        of InlineBidiResolvers now that they're separate objects.

        * GNUmakefile.am:
        * WebCore.gypi:
        * WebCore.pro:
        * WebCore.vcproj/WebCore.vcproj:
        * WebCore.xcodeproj/project.pbxproj:
        * platform/graphics/GraphicsContext.cpp:
        (WebCore::GraphicsContext::drawBidiText):
        * platform/text/BidiResolver.h:
        (WebCore::BidiResolver::BidiResolver):
        (WebCore::BidiResolver::runs):
        (WebCore::BidiResolver::markCurrentRunEmpty):
        (WebCore::::appendRun):
        (WebCore::::lowerExplicitEmbeddingLevel):
        (WebCore::::raiseExplicitEmbeddingLevel):
        (WebCore::::reorderRunsFromLevels):
        (WebCore::::createBidiRunsForLine):
        * rendering/InlineIterator.h:
        (WebCore::InlineBidiResolver::appendRun):
        * rendering/RenderBlock.h:
        * rendering/RenderBlockLineLayout.cpp:
        (WebCore::createRun):
        (WebCore::RenderBlock::appendRunsForObject):
        (WebCore::reachedEndOfTextRenderer):
        (WebCore::RenderBlock::handleTrailingSpaces):
        (WebCore::RenderBlock::layoutInlineChildren):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83240 268f45cc-cd09-0410-ab3c-d52691b4dbfc
12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/GNUmakefile.am
Source/WebCore/WebCore.gypi
Source/WebCore/WebCore.pro
Source/WebCore/WebCore.vcproj/WebCore.vcproj
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/platform/graphics/GraphicsContext.cpp
Source/WebCore/platform/text/BidiResolver.h
Source/WebCore/platform/text/BidiRunList.h [new file with mode: 0644]
Source/WebCore/rendering/InlineIterator.h
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderBlockLineLayout.cpp