Summary: | Use thread safe initialization in rotateLeftTransform | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Cameron McCormack (:heycam) <heycam> | ||||||||
Component: | Layout and Rendering | Assignee: | 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
Cameron McCormack (:heycam)
2021-12-08 18:22:49 PST
Created attachment 446471 [details]
Patch
I'd be open to the argument we shouldn't bother caching this object at all. 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.
(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! Created attachment 446493 [details]
Patch
Created attachment 446494 [details]
Patch
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. 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? (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.) |