Bug 239829 - DisplayList::Recorder should lazily create the DrawGlyphsRecorder
Summary: DisplayList::Recorder should lazily create the DrawGlyphsRecorder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-27 15:37 PDT by Simon Fraser (smfr)
Modified: 2022-04-28 08:13 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.34 KB, patch)
2022-04-27 15:38 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].