Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2219663003:
MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. (Closed)
|
Created:
4 years, 4 months ago by tapted Modified:
4 years, 4 months ago Reviewers:
karandeepb CC:
chromium-reviews, tfarina, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties.
Once we start scrolling with Layers, most of the CPU taken up when
scrolling is actually taken up by redrawing the window frame. This is
because BridgedContentView currently always claims that it is
transparent. So, when paint areas are invalidated, AppKit must first ask
the window frame to redraw that region in case BridgedContentView wants
to draw something transparent on top of that.
For opaque windows, none of the window background shows through, so
report YES from -[NSView isOpaque] in those cases.
BUG=594907, 605131
Committed: https://crrev.com/b7720c4c053a09050c9abab73b0af528150dd42c
Cr-Commit-Position: refs/heads/master@{#410012}
Patch Set 1 #
Total comments: 2
Patch Set 2 : _TRUE,_FALSE #
Messages
Total messages: 21 (14 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. Once we start scrolling with Layers, most of the CPU taken up when scrolling is actually taken up by redrawing the window frame. This is because BridgedContentView currently always claims that it is transparent. So, when paint areas are invalidated, AppKit must first ask the window frame to redraw that region in case BridgedContentView wants to draw something transparent on top of that. For opaque windows, none of the window background shows through, so report YES from -[NSView isOpaque] in those cases. BUG=594907, 605131 ========== to ========== MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. Once we start scrolling with Layers, most of the CPU taken up when scrolling is actually taken up by redrawing the window frame. This is because BridgedContentView currently always claims that it is transparent. So, when paint areas are invalidated, AppKit must first ask the window frame to redraw that region in case BridgedContentView wants to draw something transparent on top of that. For opaque windows, none of the window background shows through, so report YES from -[NSView isOpaque] in those cases. BUG=594907, 605131 ==========
tapted@chromium.org changed reviewers: + karandeepb@chromium.org
Hi Karan, please take a look
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/08/05 05:17:39, tapted wrote: > Hi Karan, please take a look LGTM. What about the scrolling performance for TRANSLUCENT window types, like the scrolling in Collected Cookies dialog, or is that somehow not affected by adding layers?
https://codereview.chromium.org/2219663003/diff/1/ui/views/widget/native_widg... File ui/views/widget/native_widget_mac_unittest.mm (right): https://codereview.chromium.org/2219663003/diff/1/ui/views/widget/native_widg... ui/views/widget/native_widget_mac_unittest.mm:1392: EXPECT_EQ(YES, [[window contentView] isOpaque]); nit: Maybe change to EXPECT_TRUE/FALSE (and more below), which seems to be the style for BOOL types elsewhere in the file.
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/08/05 06:10:35, karandeepb wrote: > On 2016/08/05 05:17:39, tapted wrote: > > Hi Karan, please take a look > > LGTM. What about the scrolling performance for TRANSLUCENT window types, like > the scrolling in Collected Cookies dialog, or is that somehow not affected by > adding layers? yeah I know :/. This only really helps the Task Manager right now. My plan for the others is to try and phase out `TRANSLUCENT` on Mac for things that just have rounded corners. Alternatively, we can simply paint the entire ScrollView to a layer -- that prevents the SchedulePaintInRect(..) calls propagating up to BridgedContentView. Currently I'm adding layers to the *Viewport* which helps a bit. But TableView headers, and scrollbars, still paint outside of the viewport. Giving ScrollView itself a layer would resolve that. https://codereview.chromium.org/2219663003/diff/1/ui/views/widget/native_widg... File ui/views/widget/native_widget_mac_unittest.mm (right): https://codereview.chromium.org/2219663003/diff/1/ui/views/widget/native_widg... ui/views/widget/native_widget_mac_unittest.mm:1392: EXPECT_EQ(YES, [[window contentView] isOpaque]); On 2016/08/05 06:10:47, karandeepb wrote: > nit: Maybe change to EXPECT_TRUE/FALSE (and more below), which seems to be the > style for BOOL types elsewhere in the file. Done.
The CQ bit was unchecked by tapted@chromium.org
The CQ bit was checked by tapted@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from karandeepb@chromium.org Link to the patchset: https://codereview.chromium.org/2219663003/#ps20001 (title: "_TRUE,_FALSE")
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. Once we start scrolling with Layers, most of the CPU taken up when scrolling is actually taken up by redrawing the window frame. This is because BridgedContentView currently always claims that it is transparent. So, when paint areas are invalidated, AppKit must first ask the window frame to redraw that region in case BridgedContentView wants to draw something transparent on top of that. For opaque windows, none of the window background shows through, so report YES from -[NSView isOpaque] in those cases. BUG=594907, 605131 ========== to ========== MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. Once we start scrolling with Layers, most of the CPU taken up when scrolling is actually taken up by redrawing the window frame. This is because BridgedContentView currently always claims that it is transparent. So, when paint areas are invalidated, AppKit must first ask the window frame to redraw that region in case BridgedContentView wants to draw something transparent on top of that. For opaque windows, none of the window background shows through, so report YES from -[NSView isOpaque] in those cases. BUG=594907, 605131 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. Once we start scrolling with Layers, most of the CPU taken up when scrolling is actually taken up by redrawing the window frame. This is because BridgedContentView currently always claims that it is transparent. So, when paint areas are invalidated, AppKit must first ask the window frame to redraw that region in case BridgedContentView wants to draw something transparent on top of that. For opaque windows, none of the window background shows through, so report YES from -[NSView isOpaque] in those cases. BUG=594907, 605131 ========== to ========== MacViews: Claim -[NSView isOpaque] for the content view according to the root layer properties. Once we start scrolling with Layers, most of the CPU taken up when scrolling is actually taken up by redrawing the window frame. This is because BridgedContentView currently always claims that it is transparent. So, when paint areas are invalidated, AppKit must first ask the window frame to redraw that region in case BridgedContentView wants to draw something transparent on top of that. For opaque windows, none of the window background shows through, so report YES from -[NSView isOpaque] in those cases. BUG=594907, 605131 Committed: https://crrev.com/b7720c4c053a09050c9abab73b0af528150dd42c Cr-Commit-Position: refs/heads/master@{#410012} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b7720c4c053a09050c9abab73b0af528150dd42c Cr-Commit-Position: refs/heads/master@{#410012} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
