Simplify NativeThemeWin::Paint() |
|||||
Issue descriptionNativeThemeWin::Paint() under some circumstances calls PaintIndirect(), which does a lot of additional work before and after calling PaintDirect(). However, it's not clear that we ever call PaintIndirect() in practice. According to comments, which seem to be consistent with the code, there are three conditions where we expect it to be invoked: 1. // This block will only get hit with --enable-accelerated-drawing flag. That flag was renamed in 2011 (commit db543d32157df951f29f7a731dd2f5995ab2e5de) to --enable-accelerated-painting; it seems to no longer be part of the codebase, and I haven't yet determined when or how it was removed. If it's defaulting to ON, we still don't seem to call PaintIndirect() today. 2. // Scrollbar components on Windows Classic theme (on all Windows versions) // have particularly problematic alpha values, so always draw them // indirectly. Turning on the Windows Classic theme on 64b Windows 7, I can't figure out how to cause PaintIndirect() to be called today. 3. // In addition, scrollbar thumbs and grippers for the Windows XP // theme (available only on Windows XP) also need their alpha values // fixed. We're no longer supporting XP and have been cleared to remove relevant code. If we really need PaintIndirect(), we should figure out when and how it's triggered, and update the comments appropriately. If we don't need it, https://codereview.chromium.org/2090003003/ passes the bots, but there are probably additional functions that can be removed elsewhere that were only invoked in this circumstance.
,
Jun 24 2016
,
Jul 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83a3c26417896b207efe22b116a03a9a162c0ca5 commit 83a3c26417896b207efe22b116a03a9a162c0ca5 Author: tomhudson <tomhudson@google.com> Date: Thu Jul 07 14:15:18 2016 Remove NativeThemeWin::PaintIndirect We don't seem to actually use this complicated codepath in practice. In particular: 1. // This block will only get hit with --enable-accelerated-drawing flag. That flag is obsolete, and its replacement also seems to no longer exist. 2. // Scrollbar components on Windows Classic theme (on all Windows versions) // have particularly problematic alpha values, so always draw them // indirectly. Turning on the Windows Classic theme on 64b Windows 7, I can't cause the block to be hit today. 3. // In addition, scrollbar thumbs and grippers for the Windows XP // theme (available only on Windows XP) also need their alpha values // fixed. We're no longer supporting XP and have been cleared to remove relevant code. Once it's gone, so are several copies of functions that paint the same UI element in different ways depending on whether they're drawing to HDC or SkCanvas: Gutter, MenuSeparator, MenuBackground, and MenuItemBackground. R=pkasting@chromium.org BUG= 543755 , 579196 , 622692 Review-Url: https://codereview.chromium.org/2090003003 Cr-Commit-Position: refs/heads/master@{#404155} [modify] https://crrev.com/83a3c26417896b207efe22b116a03a9a162c0ca5/ui/native_theme/native_theme_win.cc [modify] https://crrev.com/83a3c26417896b207efe22b116a03a9a162c0ca5/ui/native_theme/native_theme_win.h
,
Jul 7 2016
,
Jul 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d5fd19c90eb5e1715cd553b4161dc24d093eb23 commit 3d5fd19c90eb5e1715cd553b4161dc24d093eb23 Author: tomhudson <tomhudson@google.com> Date: Tue Jul 26 01:42:05 2016 Revert of Remove NativeThemeWin::PaintIndirect (patchset #3 id:40001 of https://codereview.chromium.org/2090003003/ ) Reason for revert: Directly causes crbug.com/628574 , which is ship-blocking; we'll have to find another way to clean up the mess. Original issue's description: > Remove NativeThemeWin::PaintIndirect > > We don't seem to actually use this complicated codepath in practice. > > In particular: > > 1. // This block will only get hit with --enable-accelerated-drawing flag. > That flag is obsolete, and its replacement also seems to no longer exist. > > 2. // Scrollbar components on Windows Classic theme (on all Windows versions) > // have particularly problematic alpha values, so always draw them > // indirectly. > Turning on the Windows Classic theme on 64b Windows 7, I can't cause the block > to be hit today. > > 3. // In addition, scrollbar thumbs and grippers for the Windows XP > // theme (available only on Windows XP) also need their alpha values > // fixed. > We're no longer supporting XP and have been cleared to remove relevant code. > > Once it's gone, so are several copies of functions that paint the > same UI element in different ways depending on whether they're drawing > to HDC or SkCanvas: Gutter, MenuSeparator, MenuBackground, and > MenuItemBackground. > > R=pkasting@chromium.org > BUG= 543755 , 579196 , 622692 > > Committed: https://crrev.com/83a3c26417896b207efe22b116a03a9a162c0ca5 > Cr-Commit-Position: refs/heads/master@{#404155} TBR=pkasting@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 543755 , 579196 , 622692 Review-Url: https://codereview.chromium.org/2181153002 Cr-Commit-Position: refs/heads/master@{#407679} [modify] https://crrev.com/3d5fd19c90eb5e1715cd553b4161dc24d093eb23/ui/native_theme/native_theme_win.cc [modify] https://crrev.com/3d5fd19c90eb5e1715cd553b4161dc24d093eb23/ui/native_theme/native_theme_win.h
,
Aug 4 2016
,
Aug 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10958c0a2ffe2828af635329b68c5522aa7cf75b commit 10958c0a2ffe2828af635329b68c5522aa7cf75b Author: tomhudson <tomhudson@google.com> Date: Tue Aug 16 18:39:32 2016 Simplify conditional call of PaintIndirect() In http://crbug.com/628574 we determined that PaintIndirect() was still used in modern Chrome. This CL corrects the source comments describing when it is used, and removes conditionals that weren't triggered in any of the reproductions of that bug. BUG= 622692 R=pkasting@chromium.org Review-Url: https://codereview.chromium.org/2210923003 Cr-Commit-Position: refs/heads/master@{#412298} [modify] https://crrev.com/10958c0a2ffe2828af635329b68c5522aa7cf75b/ui/native_theme/native_theme_win.cc
,
Sep 1 2016
Suspect we've done all we can for now. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by reve...@chromium.org
, Jun 23 2016