Bug 261651 – SVGTextQuery Performance Optimizations

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: SVG

[Ahmad Saleem]

Hi Team,

While going through Blink's SVG fixes, came across SVGTextQuery optimization and it significantly improves the following test case in local build:

Test Case (NOTE - we might have already bug for this): https://web.archive.org/web/20150712020900/http://output.jsbin.com/imajow/1/quiet

Blink Merges (which at least work - on my level): https://src.chromium.org/viewvc/blink?view=revision&revision=177089 & https://src.chromium.org/viewvc/blink?view=revision&revision=177148 [Partial bits and need to do std::max<int>] & https://src.chromium.org/viewvc/blink?view=revision&revision=177230 & https://src.chromium.org/viewvc/blink?view=revision&revision=177149

I don't know how to merge this: https://src.chromium.org/viewvc/blink?view=revision&revision=177231

Even without the above one, we significantly improve the test case. I think we can land all other and then have the other separately tracked IMO. These are also useful because it is not something tied to LBSE and also we are in sleeping cycle where Safari 17.x will not get updated WebKit soon, so we can experiment with it.

I run it against our outdated WPT SVG and it didn't regress any test case as well.

Thanks!

[Ahmad Saleem]

Manage to sort out other as well:

void SVGTextQuery::modifyStartEndPositionsRespectingLigatures(Data* queryData, const SVGTextFragment& fragment, unsigned& startPosition, unsigned& endPosition) const
{
    SVGTextLayoutAttributes* layoutAttributes = queryData->textRenderer->layoutAttributes();
    Vector<SVGTextMetrics>& textMetricsValues = layoutAttributes->textMetricsValues();

    unsigned textMetricsOffset = fragment.metricsListOffset;

    // Compute the offset of the fragment within the box, since that's the
    // space <startPosition, endPosition> is in (and that's what we need).
    int fragmentOffsetInBox = fragment.characterOffset - queryData->textBox->start();
    int fragmentEndInBox = fragmentOffsetInBox + fragment.length;
    // Find the text metrics cell that start at or contain the character startPosition.
    while (fragmentOffsetInBox < fragmentEndInBox) {
        SVGTextMetrics& metrics = textMetricsValues[textMetricsOffset];
        int glyphEnd = fragmentOffsetInBox + metrics.length();
        if (static_cast<int>(startPosition) < glyphEnd)
            break;
        fragmentOffsetInBox = glyphEnd;
        textMetricsOffset++;
    }
    startPosition = fragmentOffsetInBox;

    // Find the text metrics cell that contain or ends at the character endPosition.
    while (fragmentOffsetInBox < fragmentEndInBox) {
        SVGTextMetrics& metrics = textMetricsValues[textMetricsOffset];
        fragmentOffsetInBox += metrics.length();
        if (fragmentOffsetInBox >= static_cast<int>(endPosition))
            break;
        textMetricsOffset++;
    }

    endPosition = fragmentOffsetInBox;

[Ahmad Saleem]

This is another simplification on top of one from here: https://src.chromium.org/viewvc/blink?view=revision&revision=193017

[Ahmad Saleem]

Created attachment 467724 [details]
SVGTextQuery.cpp local file with changes from Blink Commits

Change in `SVGInlineTextBox.cpp`:

bool SVGInlineTextBox::mapStartEndPositionsIntoFragmentCoordinates(const SVGTextFragment& fragment, unsigned& startPosition, unsigned& endPosition) const
{
    int fragmentOffsetInBox = static_cast<int>(fragment.characterOffset) - start();

    // Compute positions relative to the fragment.
    startPosition -= fragmentOffsetInBox;
    endPosition -= fragmentOffsetInBox;

    // Intersect with the fragment range.
    startPosition = std::max<unsigned>(startPosition, 0);
    endPosition = std::min<unsigned>(endPosition, static_cast<int>(fragment.length));

    return startPosition < endPosition;
}

and attaching 'SVGTextQuery.cpp' file with all my changes from this.

[Radar WebKit Bug Importer]

<rdar://problem/115953998>

[Ahmad Saleem]

Pretty much everything is done here - only pending work is in 'See Also' and also has its own conclusion (issue) in 1-1 merge.

Marking this as 'RESOLVED CONFIGURATION CHANGED'. Thanks!