Summary: | [GPU Process] [GraphicsContextState 2/] Make GraphicsContextState keep track of changes till they are applied | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||
Severity: | Normal | CC: | annulen, bfulgham, cdumez, changseok, clopez, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, heycam, jonlee, kondapallykalyan, mmaxfield, pdr, ryuan.choi, schenney, sergio, simon.fraser, webkit-bug-importer, youennf, zalan | ||||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||||||
Bug Blocks: | 237728 | ||||||||||||||||||||||||||||
Attachments: |
|
Description
Said Abou-Hallawa
2022-03-17 23:47:34 PDT
Created attachment 455069 [details]
Patch
Created attachment 455070 [details]
Patch
Created attachment 455075 [details]
Patch
Created attachment 455077 [details]
Patch
Created attachment 455079 [details]
Patch
Created attachment 455143 [details]
Investigate Cairo failures
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess Created attachment 455154 [details]
Patch
Created attachment 455190 [details]
Patch
Created attachment 455192 [details]
Patch
Created attachment 455211 [details]
Patch
Comment on attachment 455211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455211&action=review > Source/WebCore/platform/graphics/GraphicsContextState.cpp:89 > +#if USE(CG) Ideally this would be in GraphicsContextCG somewhere. > Source/WebCore/platform/graphics/GraphicsContextState.cpp:100 > +#if USE(CG) > + // CGContextBeginTransparencyLayer() sets the CG global alpha to 1. Resore our alpha now. > + alpha = originalOpacity; > +#else Ditto > Source/WebCore/platform/graphics/GraphicsContextState.cpp:136 > + unsigned index = static_cast<unsigned>(log2(mask)); > + return stateChangeNames[index]; I think a switch would be preferable. It's too easy for this to get out of sync. > Source/WebCore/platform/graphics/GraphicsTypes.cpp:162 > + ts << "DO-NOT-INTERPOLATE"; lowercase please > Source/WebCore/platform/graphics/GraphicsTypes.cpp:171 > + ts << "HEIGH"; spelling > Source/WebCore/platform/graphics/GraphicsTypes.cpp:227 > + ts << "NO-STROKE"; lowercase please > Source/WebCore/platform/graphics/GraphicsTypes.cpp:252 > + ts << "FILL"; lowercase > Source/WebCore/platform/graphics/SourceBrush.h:66 > + bool isPrimitive() const { return !m_brush && m_color.tryGetAsSRGBABytes(); } It's not clear what "primitive" means here. Seems like it's asking if it's a inline sRGB color. > Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:597 > + bool shouldFill = context.fillPattern() || context.fillColor().isVisible(); Should this be calling a function on the SourceBrush? From reading this, it's not clear if a gradient fill will return true. Created attachment 455298 [details]
Patch
Comment on attachment 455211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455211&action=review >> Source/WebCore/platform/graphics/GraphicsContextState.cpp:89 >> +#if USE(CG) > > Ideally this would be in GraphicsContextCG somewhere. But this is also called by DisplayList::Recorder. >> Source/WebCore/platform/graphics/SourceBrush.h:66 >> + bool isPrimitive() const { return !m_brush && m_color.tryGetAsSRGBABytes(); } > > It's not clear what "primitive" means here. Seems like it's asking if it's a inline sRGB color. I called it isInlineColor(). >> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:597 >> + bool shouldFill = context.fillPattern() || context.fillColor().isVisible(); > > Should this be calling a function on the SourceBrush? From reading this, it's not clear if a gradient fill will return true. I added the function SourceBrush::isVisible() to replace these checks. And it is okay to check or not check the gradient because the only caller to this function (i.e. drawPath()) handles the gradient case separately before calling this function. Created attachment 455326 [details]
Patch
Committed r291604 (248697@main): <https://commits.webkit.org/248697@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 455326 [details]. *** Bug 236919 has been marked as a duplicate of this bug. *** *** Bug 237685 has been marked as a duplicate of this bug. *** *** Bug 237686 has been marked as a duplicate of this bug. *** |