Bug 261651 - SVGTextQuery Performance Optimizations
Summary: SVGTextQuery Performance Optimizations
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-09-17 04:33 PDT by Ahmad Saleem
Modified: 2023-10-02 17:56 PDT (History)
4 users (show)

See Also:


Attachments
SVGTextQuery.cpp local file with changes from Blink Commits (18.04 KB, text/plain)
2023-09-17 11:41 PDT, Ahmad Saleem
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-09-17 04:33:55 PDT
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!
Comment 1 Ahmad Saleem 2023-09-17 11:26:00 PDT
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;
Comment 2 Ahmad Saleem 2023-09-17 11:37:57 PDT
This is another simplification on top of one from here: https://src.chromium.org/viewvc/blink?view=revision&revision=193017
Comment 3 Ahmad Saleem 2023-09-17 11:41:28 PDT
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.
Comment 4 Radar WebKit Bug Importer 2023-09-24 04:34:15 PDT
<rdar://problem/115953998>
Comment 5 Ahmad Saleem 2023-10-02 17:56:04 PDT
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!