Windows.h and Angle

39 views
Skip to first unread message

Bruce Dawson

unread,
Aug 13, 2021, 7:04:02 PM8/13/21
to angleproject
I've been working on crbug.com/796644 which is an attempt to reduce how many translation units include windows.h when building Chrome. Doing so can slightly improve build times but mostly it reduces namespace pollution (#define DrawText DrawTextW, etc.).

In a build of the chrome target I see that 618 translation units in Angle include windows.h. Is there any interest in reducing this? If most of the files need windows.h then there is no point, but in the case of Chrome most of the #includes were unnecessary and were avoidable (with varying degrees of effort).

Let me know what you think.

Shahbaz Youssefi

unread,
Aug 14, 2021, 9:42:53 PM8/14/21
to angleproject
It's most likely because it's included in `src/common/platform.h` and that file is included everywhere (so yes, it's mostly unnecessary). I don't think there is any objection to including some system file less, but I don't believe anyone could prioritize this right now. I vaguely recall windows.h giving us some trouble with min/max before (which `#define NOMINMAX` took care of), but it's not really bothering us otherwise.

Jamie Madill

unread,
Aug 16, 2021, 7:23:41 AM8/16/21
to syou...@chromium.org, angleproject
What Shabi said. If someone wanted to volunteer time to clean this up, it would be appreciated. I'm not sure if we have someone available on the team at the moment to work on it.

--
You received this message because you are subscribed to the Google Groups "angleproject" group.
To unsubscribe from this group and stop receiving emails from it, send an email to angleproject...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/angleproject/b147afb5-ba3c-4f6c-9478-ef52c43124ean%40googlegroups.com.

Tim Van Patten

unread,
Aug 16, 2021, 11:11:48 AM8/16/21
to jma...@chromium.org, syou...@chromium.org, angleproject
I opened a bug to track this and marked it with "Hostlist-Polish".   Hopefully we can get to it during the next fix-it week.
anglebug.com/6283: windows.h should be included less



--

Tim Van Patten | Android Graphics | ti...@google.com | (720) 432-0997

Bruce Dawson

unread,
Aug 16, 2021, 1:17:50 PM8/16/21
to ti...@google.com, jma...@chromium.org, syou...@chromium.org, angleproject
Sounds good. I might poke at the task a bit to see if there are any easy wins, or I might leave it for the Angle team to do or ignore.

Angle's use of windows.h isn't bleeding over into Chromium so it doesn't affect me directly, it just shows up on the reports I generate of windows.h usage by directory so I thought I'd ask. The reports look something like this:

Found 6053 .obj files (15.9%) that used um/windows.h, 31954 that did not.
Windows.h usage by directory:
                 third_party/pdfium:  928
                  third_party/angle:  618
                 third_party/webrtc:  401
                     chrome/browser:  264
                  third_party/blink:  258
                          base/base:  160
                  native_client/src:  157
              third_party/boringssl:  152
                    content/browser:  147
                            net/net:  132
            third_party/swiftshader:  122
                    net/third_party:  113
               third_party/crashpad:  108

You received this message because you are subscribed to a topic in the Google Groups "angleproject" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/angleproject/UKrm6YpV_2E/unsubscribe.
To unsubscribe from this group and all its topics, send an email to angleproject...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/angleproject/CANkg5ez3x2j7ddeyfUDm0FPSCFzXQTE8uCMT0w7TAQb1S--m%3DQ%40mail.gmail.com.


--
Bruce Dawson, he/him

Geoff Lang

unread,
Aug 16, 2021, 2:46:02 PM8/16/21
to Bruce Dawson, Tim Van Patten, Jamie Madill, syou...@chromium.org, angleproject
Reply all
Reply to author
Forward
0 new messages