NEW234052
Use thread safe initialization in rotateLeftTransform
https://bugs.webkit.org/show_bug.cgi?id=234052
Summary Use thread safe initialization in rotateLeftTransform
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
Patch (5.93 KB, patch)
2021-12-08 20:11 PST, Cameron McCormack (:heycam)
no flags
Patch (7.18 KB, patch)
2021-12-08 20:18 PST, Cameron McCormack (:heycam)
mmaxfield: review+
Cameron McCormack (:heycam)
Comment 1 2021-12-08 18:24:00 PST
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
Cameron McCormack (:heycam)
Comment 6 2021-12-08 20:18:35 PST
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
Note You need to log in before you can comment on or make changes to this bug.