New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 905947 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature

Blocked on:
issue 885180
issue 923812



Sign in to add a comment

Check if rounded corners applied to the PIP window is worth it

Project Member Reported by edcourtney@chromium.org, Nov 16

Issue description

Applying rounded corners to the PIP window involves masking the window, which means it can't be displayed as a hardware overlay.

Continually compositing the window compared to a hardware overlay will have some performance and battery-life hit.

We need to measure/decide if the hit is worth having rounded corners.

First I will upload a CL making rounded corners for PIP windows controllable via a flag (default on). 
 
Cc: dcasta...@chromium.org malaykeshav@chromium.org
Components: Internals>Compositing UI>GFX
We're working on a better way to implement rounded corner (crbug.com/903486),
and we probably should include this as an additoinal goal.

Technically speaking, it should be possible to do underlay with rounded corner hole, but I guess it'd require different approach than normal rounded window.
+dcastagna@
Cc: huanr@chromium.org satorux@chromium.org
Cc: saintlou@chromium.org
Labels: -Type-Bug Type-Feature
Status: Available (was: Untriaged)
GPU Triage pass - this looks like feature work, changing to feature and marking available.
What are the engineering cost (implementation and upstreaming to Android) and run time performance cost for rounded corners?
If not mistaken this is already implemented. The reason for this is that rounded corners exclude the possibility of using an overlay (which is "free" (in regards of performance) and "nearly free" (in regards of battery use)). 

We had a meeting with UX and agreed that we should add a Chrome flag to see how the performance impact on lower end devices would be. (E.g. if the system would get sluggish or even janky when a PIP is being used with rounded corners vs. used without).

I chatted with Emmanuel about this and he immediately stated that we should not waste time on it and instead make them rectangular. (Note: YouTube has some experience in this area...).
Oshima-san found that currently, for video, we don't use hardware overlays regardless of whether rounded corners are used or not (on both Android and Chrome PIP), so we won't be able to measure the difference between hw overlay and composited for PIP until that's fixed.

Since it's already being composited the performance impact from rounded corners may be small (but we don't know until we measure).

Let me add the flag for now, but if it's permissible from a UI perspective, not having rounded corners is safer for performance.
Why exactly is that? Does this refer to the intel rotation problem?
Owner: ----
I talked with Oshima-san, and it seems to be because the controls are preventing the video from being displayed as an overlay (regardless of whether they are visible or not).
Wrt engineering cost: I noticed that the current implementation of rounded corners on the PIP window doesn't cast any shadows (they are masked off by the rounded corners), so we have that left to implement.
Ahh.. So we are not drawing the controls via Android as I was asking for but rather draw them via Chrome. Another reason for not mixing OS'es for no good reason. Why exactly do we do this?
[I do not accept comments that the buttons might look different, because that can easily be rectified + there will be buttons which will look different either way.]
Android draws the Android PIP controls, Chrome draws the Chrome PIP controls. But it looks like neither is using a hardware overlay for videos, whether it has rounded corners or not.
Blockedon: 885180
It is android that creates and draws UI layer, and that's the one picked up becaues it's above video layer. (crbug.com/885180)

Using chrome to draw UI will actualyl fix Android scenario becuse it won't use overlay, but I don't think that's what we want.
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e87a9c3911d928b02483d2756b6b23ec64279b10

commit e87a9c3911d928b02483d2756b6b23ec64279b10
Author: Eliot Courtney <edcourtney@google.com>
Date: Tue Nov 20 03:33:02 2018

Add a flag for rounded corners for PIP.

Applying rounded corners to the PIP window means it currently can not be
displayed using a hardware overlay. Add a flag for this so it's easy to
check how the UI looks with and without and for performance measurement.

  enabled, otherwise no rounded corners are applied.

Bug: 905947
Test: rounded corners is applied to the pip window with this flag
Change-Id: I59697c3fc77c3d253b7b70a1d39e7cc137b201ef
Reviewed-on: https://chromium-review.googlesource.com/c/1341447
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Commit-Queue: Eliot Courtney <edcourtney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609584}
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/ash/public/cpp/ash_features.cc
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/ash/public/cpp/ash_features.h
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/ash/wm/window_state.cc
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/chrome/browser/about_flags.cc
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/chrome/browser/flag-metadata.json
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/e87a9c3911d928b02483d2756b6b23ec64279b10/tools/metrics/histograms/enums.xml

Forgot to set this bug on it, but I submitted https://chromium-review.googlesource.com/c/chromium/src/+/1347969 which fixes the shadow issue for rounded corners. We should be able to do a flag flip if we decide to disable rounded corners.

Given that even if we disable it, currently neither uses a hardware overlay, and that, talking with Oshima-san, it /should/ be possible to implement rounded corners while keeping the video in an overlay, I recommend we leave it on.

Comment 17 by edcourtney@chromium.org, Yesterday (46 hours ago)

Blockedon: 923812

Sign in to add a comment