Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1852613003:
Remove flash of white borders on maximize/fullscreen (Closed)
DescriptionRemove flash of white borders on maximize/fullscreen
With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on
resize it looks ugly when it temporarily becomes visible. Chrome can
clear it itself on WM_NCPAINT.
BUG=599391
Committed: https://crrev.com/aa39868d81e25078a2a16dd7a214c5c31e1fb3f7
Cr-Commit-Position: refs/heads/master@{#385080}
Patch Set 1 #Patch Set 2 : #
Total comments: 6
Patch Set 3 : #Messages
Total messages: 17 (7 generated)
Description was changed from ========== Remove flash of white borders on maximize/fullscreen With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on resize it looks ugly when it temporarily becomes visible. Chrome can clear them itself on WM_NCPAINT. ========== to ========== Remove flash of white borders on maximize/fullscreen With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on resize it looks ugly when it temporarily becomes visible. Chrome can clear it itself on WM_NCPAINT. BUG=599391 ==========
jbauman@chromium.org changed reviewers: + sky@chromium.org
sky@chromium.org changed reviewers: + ananta@chromium.org
+ananta https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:1812: return; // Dirty region doesn't intersect window bounds, bale. If you early return do you need to SetMsgHandled?
On 2016/04/01 23:55:52, sky wrote: > +ananta > > https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... > File ui/views/win/hwnd_message_handler.cc (right): > > https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... > ui/views/win/hwnd_message_handler.cc:1812: return; // Dirty region doesn't > intersect window bounds, bale. > If you early return do you need to SetMsgHandled? Good question. I think it makes sense to have it keep the default of TRUE, because it's been handled (nothing had to be painted).
https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:1799: RECT window_rect; This code is only needed for Aero frames?. Perhaps move it to the if block. Additionally would be good to move this logic to its own function. https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:1833: ::OffsetRect(&client_rect, -window_rect.left, -window_rect.top); MapWindowPoints converts to screen above. Why is this offsetrect needed?
https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:1799: RECT window_rect; On 2016/04/02 01:35:06, ananta wrote: > This code is only needed for Aero frames?. Perhaps move it to the if block. > Additionally would be good to move this logic to its own function. No, this is also needed to get the dirty_region for HandlePaintAccelerated down below. Or at least that's how this code was working before; I'm not sure that's actually necessary. This code is very WM_NCPAINT-specific (with the special-case handling of "1"), so I don't think it makes sense to move it to a helper function. https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:1833: ::OffsetRect(&client_rect, -window_rect.left, -window_rect.top); On 2016/04/02 01:35:06, ananta wrote: > MapWindowPoints converts to screen above. Why is this offsetrect needed? We need client_rect to be relative to the top-left of the (non-client area of) the window, which is what this OffsetRect does.
lgtm % nit https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1852613003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:1812: return; // Dirty region doesn't intersect window bounds, bale. On 2016/04/01 23:55:52, sky wrote: > If you early return do you need to SetMsgHandled? +1. We should call SetMsgHandled(FALSE) here.
LGTM
The CQ bit was checked by jbauman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ananta@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/1852613003/#ps40001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1852613003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1852613003/40001
Message was sent while issue was closed.
Description was changed from ========== Remove flash of white borders on maximize/fullscreen With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on resize it looks ugly when it temporarily becomes visible. Chrome can clear it itself on WM_NCPAINT. BUG=599391 ========== to ========== Remove flash of white borders on maximize/fullscreen With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on resize it looks ugly when it temporarily becomes visible. Chrome can clear it itself on WM_NCPAINT. BUG=599391 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Remove flash of white borders on maximize/fullscreen With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on resize it looks ugly when it temporarily becomes visible. Chrome can clear it itself on WM_NCPAINT. BUG=599391 ========== to ========== Remove flash of white borders on maximize/fullscreen With Aero the default WM_NCPAINT doesn't clear the nonclient area, so on resize it looks ugly when it temporarily becomes visible. Chrome can clear it itself on WM_NCPAINT. BUG=599391 Committed: https://crrev.com/aa39868d81e25078a2a16dd7a214c5c31e1fb3f7 Cr-Commit-Position: refs/heads/master@{#385080} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/aa39868d81e25078a2a16dd7a214c5c31e1fb3f7 Cr-Commit-Position: refs/heads/master@{#385080} |
