Bug 239829

Summary:

DisplayList::Recorder should lazily create the DrawGlyphsRecorder

Product:

WebKit

Reporter:

Simon Fraser (smfr) <simon.fraser>

Component:

Layout and Rendering

Assignee:

Simon Fraser (smfr) <simon.fraser>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, mmaxfield, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description Simon Fraser (smfr) 2022-04-27 15:37:11 PDT

DisplayList::Recorder should lazily create the DrawGlyphsRecorder

Comment 1 Simon Fraser (smfr) 2022-04-27 15:38:25 PDT

Created attachment 458475 [details]
Patch

Comment 2 Myles C. Maxfield 2022-04-27 15:42:16 PDT

Comment on attachment 458475 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=458475&action=review

> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:166
> +    if (!m_drawGlyphsRecorder)
> +        m_drawGlyphsRecorder = makeUnique<DrawGlyphsRecorder>(*this, m_initialScale, m_deconstructDrawGlyphs);

Can we pull this out into an ensure() function?

> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:275
> +    float m_initialScale { 1 };
> +    DrawGlyphsRecorder::DeconstructDrawGlyphs m_deconstructDrawGlyphs { DrawGlyphsRecorder::DeconstructDrawGlyphs::No };

Should these be batched up into a struct? Are we polluting the contents of the Recorder object?

Comment 3 Simon Fraser (smfr) 2022-04-27 15:53:52 PDT

(In reply to Myles C. Maxfield from comment #2)
> Comment on attachment 458475 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=458475&action=review
> 
> > Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:166
> > +    if (!m_drawGlyphsRecorder)
> > +        m_drawGlyphsRecorder = makeUnique<DrawGlyphsRecorder>(*this, m_initialScale, m_deconstructDrawGlyphs);
> 
> Can we pull this out into an ensure() function?
> 
> > Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:275
> > +    float m_initialScale { 1 };
> > +    DrawGlyphsRecorder::DeconstructDrawGlyphs m_deconstructDrawGlyphs { DrawGlyphsRecorder::DeconstructDrawGlyphs::No };
> 
> Should these be batched up into a struct? Are we polluting the contents of
> the Recorder object?

Both true, but I'm not sure either are worth it.

Comment 4 Radar WebKit Bug Importer 2022-04-27 21:21:15 PDT

<rdar://problem/92438193>

Comment 5 EWS 2022-04-28 08:13:33 PDT

Committed r293572 (250086@main): <https://commits.webkit.org/250086@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458475 [details].