Bug 230648

Summary: [GPU Process] support rendering Apple Pay buttons
Product: WebKit Reporter: Devin Rousso <hi>
Component: New BugsAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, changseok, esprehn+autocc, ews-watchlist, glenn, hi, kondapallykalyan, pdr, sam, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=230647
Attachments:
Description Flags
Patch none

Devin Rousso
Reported 2021-09-22 16:05:05 PDT
.
Attachments
Patch (4.27 KB, patch)
2021-09-22 16:19 PDT, Devin Rousso
no flags
Devin Rousso
Comment 1 2021-09-22 16:07:26 PDT
Devin Rousso
Comment 2 2021-09-22 16:19:44 PDT
Sam Weinig
Comment 3 2021-09-24 12:00:52 PDT
Comment on attachment 438987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438987&action=review > Source/WebCore/ChangeLog:16 > + While it is possible to create a dedicated display list item for this, we don't want to do > + that because `PKDrawApplePayButtonWithCornerRadius` involves dealing with PDFs, which are > + not as secure as we'd like for use in the GPUProcess. What about doing something like we do for DrawGlyphsRecorder and use a CGContextDelegate to get the underlying draw calls and serialize those? (This is of course assuming CGContextDelegate doesn't have dedicate DrawPDFPage delegation, but I didn't think it did)?
Devin Rousso
Comment 4 2021-09-24 18:20:06 PDT
Comment on attachment 438987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438987&action=review >> Source/WebCore/ChangeLog:16 >> + not as secure as we'd like for use in the GPUProcess. > > What about doing something like we do for DrawGlyphsRecorder and use a CGContextDelegate to get the underlying draw calls and serialize those? (This is of course assuming CGContextDelegate doesn't have dedicate DrawPDFPage delegation, but I didn't think it did)? Interesting! I had no idea that existed =D And yeah I dont see anything related to PDF. Frankly though I'm not really sure I have the time to investigate this right now. I'm happy to file a followup to investigate this further if I get a chance in a few weeks. The initial desire for this change is to just get this working in a GPUProcess world, which I think this achieves.
Tim Horton
Comment 5 2021-09-30 12:03:58 PDT
Comment on attachment 438987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438987&action=review >>> Source/WebCore/ChangeLog:16 >>> + not as secure as we'd like for use in the GPUProcess. >> >> What about doing something like we do for DrawGlyphsRecorder and use a CGContextDelegate to get the underlying draw calls and serialize those? (This is of course assuming CGContextDelegate doesn't have dedicate DrawPDFPage delegation, but I didn't think it did)? > > Interesting! I had no idea that existed =D And yeah I dont see anything related to PDF. > > Frankly though I'm not really sure I have the time to investigate this right now. I'm happy to file a followup to investigate this further if I get a chance in a few weeks. > > The initial desire for this change is to just get this working in a GPUProcess world, which I think this achieves. We don't have a general purpose recorder, only drawglyphs, so I think this is OK for now (you could imagine us having implemented the whole of GPUP that way instead, but we did not).
EWS
Comment 6 2021-09-30 13:21:53 PDT
Committed r283331 (242352@main): <https://commits.webkit.org/242352@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 438987 [details].
Sam Weinig
Comment 7 2021-09-30 16:02:08 PDT
Please at least make sure there are tests for drawing and scaling these at different sizes to ensure we don't regress that.
Sam Weinig
Comment 8 2021-09-30 16:02:17 PDT Comment hidden (obsolete)
Note You need to log in before you can comment on or make changes to this bug.