Bug 238278 - [GPU Process] [GraphicsContextState 4/] Ensure DisplayList::Recorder and its base class are initialized with the same GraphicsContextState
Summary: [GPU Process] [GraphicsContextState 4/] Ensure DisplayList::Recorder and its ...
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: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks: 237728
  Show dependency treegraph
 
Reported: 2022-03-23 12:10 PDT by Said Abou-Hallawa
Modified: 2022-03-28 15:13 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.03 KB, patch)
2022-03-23 12:17 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2022-03-23 12:10:18 PDT
When a glyph run is redisplayed, we tend to cache its drawing for perf gain. We create a DisplayList::RecorderImpl, pass it the current GraphicsContextState and ask it to drawGlyphs(). We then cache the recorded DisplayList.

DisplayList::RecorderImpl passes the initial GraphicsContextState to its base class DisplayList::Recorder which pushes it on its stack. The problem is DisplayList::Recorder does not pass this initial GraphicsContextState to its base class which is GraphicsContext. So DisplayList::Recorder ends up having the initial state but the GraphicsContext ends up having the default state.

DisplayList::Recorder::drawGlyphs() calls DrawGlyphsRecorder::drawGlyphs() which stores the original fillBrush, strokeBrush and dropShadow. It uses these original values to restore the m_owner when it finishes. The m_owner in this case is of type DisplayList::RecorderImpl. The problem is DrawGlyphsRecorder::drawGlyphs() stores the values in the state of the GraphicsContext which are the default. So in some cases we may restore the default state to the drawing GraphicsContext.

For example let's assume the initial GraphicsContextState in the drawing GraphicsContext has fillColor = 'green':

1. DisplayList::RecorderImpl will pass the initial state to DisplayList::Recorder. So the state of its DisplayList::Recorder will have fillColor = 'green' but its GraphicsContext will have fillColor = 'black'
2. DrawGlyphsRecorder::drawGlyphs() will store fillColor = 'black' before recording.
3. DrawGlyphsRecorder::drawGlyphs() will restore fillColor = 'black' to DisplayList::Recorder. So a DisplayList item will be recorded to set the fillColor back to 'black'.
4. When replaying back the glyph DisplayList, the drawing GraphicsContext has fillColor = 'black.
Comment 1 Said Abou-Hallawa 2022-03-23 12:11:19 PDT
rdar://84602660
Comment 2 Said Abou-Hallawa 2022-03-23 12:17:14 PDT
Created attachment 455537 [details]
Patch
Comment 3 EWS 2022-03-23 15:52:19 PDT
Committed r291771 (?): <https://commits.webkit.org/r291771>

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