Bug 202626

Summary: Move font-rendering code from CanvasRenderingContext2D to CanvasRenderingContext2DBase to be used with OffscreenCanvas
Product: WebKit Reporter: Chris Lord <clord>
Component: CanvasAssignee: Chris Lord <clord>
Status: RESOLVED INVALID    
Severity: Normal CC: darin, dino, koivisto, mmaxfield, rniwa, sabouhallawa, simon.fraser, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 182686    
Bug Blocks: 186759, 202793    
Attachments:
Description Flags
Patch rniwa: review-

Chris Lord
Reported 2019-10-07 01:15:32 PDT
Previous work on bug 183720 moves much of Canvas 2d's to a base class. The font functions are skipped mainly I assume due to a reliance on Document to be able to use fonts. This bug is for moving that functionality into the relevant base classes, but not necessarily removing the Document dependency (which is a much larger task that it'd be sensible to split as much work away from as possible).
Attachments
Patch (82.44 KB, patch)
2019-10-10 04:18 PDT, Chris Lord 		rniwa: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chris Lord
Comment 1 2019-10-10 04:18:46 PDT
Created attachment 380631 [details] Patch
Ryosuke Niwa
Comment 2 2019-10-10 12:41:17 PDT
Comment on attachment 380631 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380631&action=review > Source/WebCore/html/canvas/OffscreenCanvasRenderingContext2D.cpp:60 > + auto parsedStyle = MutableStyleProperties::create(); > + CSSParser::parseValue(parsedStyle, CSSPropertyFont, newFont, true, strictToCSSParserMode(!m_usesCSSCompatibilityParseMode)); > + if (parsedStyle->isEmpty()) As far as I know, CSS parser isn't thread safe due to CSSValuePool and other static variables. For example, consumeFontWeightKeywordValue would use CSSValuePool::singleton().createIdentifierValue.
Chris Lord
Comment 3 2019-10-11 01:51:40 PDT
(In reply to Ryosuke Niwa from comment #2) > Comment on attachment 380631 [details] > As far as I know, CSS parser isn't thread safe due to CSSValuePool and other > static variables. > For example, consumeFontWeightKeywordValue would use > CSSValuePool::singleton().createIdentifierValue. This is correct, however I've altered the behaviour of CSSValuePool when used off the main thread in the dependent patch on bug #182686 to not return static variables.
Ryosuke Niwa
Comment 4 2019-10-11 02:45:19 PDT
(In reply to Chris Lord from comment #3) > (In reply to Ryosuke Niwa from comment #2) > > Comment on attachment 380631 [details] > > As far as I know, CSS parser isn't thread safe due to CSSValuePool and other > > static variables. > > For example, consumeFontWeightKeywordValue would use > > CSSValuePool::singleton().createIdentifierValue. > > This is correct, however I've altered the behaviour of CSSValuePool when > used off the main thread in the dependent patch on bug #182686 to not return > static variables. Ah, I didn't see that. I added a comment there.
Chris Lord
Comment 5 2021-01-20 06:56:41 PST
The work from this bug was essentially done in bug 219088
Note You need to log in before you can comment on or make changes to this bug.