Bug 234052

Summary: Use thread safe initialization in rotateLeftTransform
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: Layout and RenderingAssignee: Cameron McCormack (:heycam) <heycam>
Status: NEW    
Severity: Normal CC: bfulgham, darin, mmaxfield, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 233488    
Attachments:
Description Flags
Patch
none
Patch
none
Patch mmaxfield: review+

Cameron McCormack (:heycam)
Reported 2021-12-08 18:22:49 PST
Use thread safe initialization in rotateLeftTransform
Attachments
Patch (2.00 KB, patch)
2021-12-08 18:24 PST, Cameron McCormack (:heycam) 		no flags
DetailsFormatted DiffDiff
Patch (5.93 KB, patch)
2021-12-08 20:11 PST, Cameron McCormack (:heycam) 		no flags
DetailsFormatted DiffDiff
Patch (7.18 KB, patch)
2021-12-08 20:18 PST, Cameron McCormack (:heycam) 		mmaxfield: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Cameron McCormack (:heycam)
Comment 1 2021-12-08 18:24:00 PST
Created attachment 446471 [details] Patch
Cameron McCormack (:heycam)
Comment 2 2021-12-08 18:24:11 PST
I'd be open to the argument we shouldn't bother caching this object at all.
Darin Adler
Comment 3 2021-12-08 19:05:16 PST
Comment on attachment 446471 [details] Patch Another solution here would be to make the constructor for AffineTransform constexpr, then we could use a constexpr static and there is no thread issue there.
Cameron McCormack (:heycam)
Comment 4 2021-12-08 19:12:29 PST
(In reply to Darin Adler from comment #3) > Another solution here would be to make the constructor for AffineTransform > constexpr, then we could use a constexpr static and there is no thread issue > there. Good idea!
Cameron McCormack (:heycam)
Comment 5 2021-12-08 20:11:29 PST
Created attachment 446493 [details] Patch
Cameron McCormack (:heycam)
Comment 6 2021-12-08 20:18:35 PST
Created attachment 446494 [details] Patch
Darin Adler
Comment 7 2021-12-09 12:33:55 PST
Comment on attachment 446494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446494&action=review > Source/WebCore/platform/graphics/transforms/AffineTransform.h:64 > + WEBCORE_EXPORT constexpr AffineTransform(); > + WEBCORE_EXPORT constexpr AffineTransform(double a, double b, double c, double d, double e, double f); Should not need WEBCORE_EXPORT on constexpr functions, but if it builds, then I guess that proves me wrong.
Myles C. Maxfield
Comment 8 2021-12-10 15:16:49 PST
Comment on attachment 446494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446494&action=review Feels a bit weird that the patch title only is relevant to like 1/2 of this patch. > Source/WebCore/platform/graphics/coretext/FontCascadeCoreText.cpp:57 > +static constexpr AffineTransform rotateLeftTransform { 0, -1, 1, 0, 0, 0 }; 🆒 > Tools/ChangeLog:3 > + Remove unused AffineTransform variable from a test Are you sure you meant to include this in this patch?
Cameron McCormack (:heycam)
Comment 9 2021-12-10 15:29:23 PST
(In reply to Myles C. Maxfield from comment #8) > Feels a bit weird that the patch title only is relevant to like 1/2 of this > patch. I'll reword. > > Tools/ChangeLog:3 > > + Remove unused AffineTransform variable from a test > > Are you sure you meant to include this in this patch? Yes. After changing the constructor to be constexpr, the compiler complains that the variable is unused. (Before it didn't because I guess the constructor could be doing some useful work.)
Radar WebKit Bug Importer
Comment 10 2021-12-15 18:23:16 PST
<rdar://problem/86553047>
Note You need to log in before you can comment on or make changes to this bug.