Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2905013002:
[vr] Introduce ColorScheme (Closed)
|
Created:
3 years, 7 months ago by Ian Vollick Modified:
3 years, 7 months ago CC:
chromium-reviews, feature-vr-reviews_chromium.org, joshcarpenter1 Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[vr] Introduce ColorScheme
This refactors some colors into a color scheme struct, accessed by
mode. Currently, this only houses a few colors, but it would be a
convenient home for all colors that could vary between normal,
fullscreen and, eventually, incognito modes.
Note:
This incorporates some colors and changes from acondor@ and amp@.
BUG=715664
Review-Url: https://codereview.chromium.org/2905013002
Cr-Commit-Position: refs/heads/master@{#474766}
Committed: https://chromium.googlesource.com/chromium/src/+/5cee228225d5b929b96eac976994a8809cfbff20
Patch Set 1 #
Total comments: 1
Patch Set 2 : added comment about naming of colors in the scheme #
Total comments: 7
Patch Set 3 : address reviewer feedback #Patch Set 4 : . #Patch Set 5 : update grid color alpha #
Total comments: 10
Patch Set 6 : address reviewer feedback #Patch Set 7 : rebase #Patch Set 8 : rebase #
Dependent Patchsets: Messages
Total messages: 49 (29 generated)
Description was changed from ========== [vr] Introduce ColorScheme This refactors some colors into a color scheme struct, accessed by mode. Currently, this only houses a few colors, but it would be a convenient home for all colors that could vary between normal, fullscreen and, eventually, incognito modes. BUG=715664 ========== to ========== [vr] Introduce ColorScheme This refactors some colors into a color scheme struct, accessed by mode. Currently, this only houses a few colors, but it would be a convenient home for all colors that could vary between normal, fullscreen and, eventually, incognito modes. Note: This incorporates some colors and changes from acondor@ and amp@. BUG=715664 ==========
vollick@chromium.org changed reviewers: + cjgrant@chromium.org
vollick@chromium.org changed reviewers: + acondor@google.com, amp@chromium.org
https://codereview.chromium.org/2905013002/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/color_scheme.cc:26: fullscreen_scheme.floor = {0.02745098, 0.058823529, 0.109803922, 0.5}; These colors are the center color for the element, but the edge for some of the elements are supposed to blend to the edge using the same color but modifying only alpha and not all use the horizon color as the edge color. I'm not sure of the best way to represent that, and it may vary how much alpha is adjusted per element. Maybe this is something that should be contained within the element itself? Ie each element gets it's own color scheme internally and defines it's own center and horizon and the transition between each mode. Maybe that's more trouble than just tracking it all in the scene manager though.
On 2017/05/25 04:23:26, amp wrote: > https://codereview.chromium.org/2905013002/diff/1/chrome/browser/android/vr_s... > File chrome/browser/android/vr_shell/color_scheme.cc (right): > > https://codereview.chromium.org/2905013002/diff/1/chrome/browser/android/vr_s... > chrome/browser/android/vr_shell/color_scheme.cc:26: fullscreen_scheme.floor = > {0.02745098, 0.058823529, 0.109803922, 0.5}; > These colors are the center color for the element, but the edge for some of the > elements are supposed to blend to the edge using the same color but modifying > only alpha and not all use the horizon color as the edge color. > > I'm not sure of the best way to represent that, and it may vary how much alpha > is adjusted per element. > > Maybe this is something that should be contained within the element itself? Ie > each element gets it's own color scheme internally and defines it's own center > and horizon and the transition between each mode. Maybe that's more trouble > than just tracking it all in the scene manager though. We chatted offline. My understanding of the consensus (please correct me if I'm wrong acondor@, amp@) was that: 1. Alpha will indeed need to be varied across elements, but tweaking the alpha of a color from the standard scheme colors could be handled by the elements. 2. There is a risk of an explosion of colors that are very specific to each element. The goal of the scheme is to provide a standard set of common colors that can be reused by various elements (eg, text fg color might want to be lightly colored in dark schemes on a number of elements). I've added a comment to color_scheme.h about color naming to give reviewers a guide for what to look for in new color names. Does that sound reasonable?
Agree on the points. https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:25: fullscreen_scheme.horizon = {0.039215686, 0.0, 0.082352941, 0.149019608}; I didn't realize this before, but why this doesn't have alpha = 1.0, if it is the background color? amp@? https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/ui_scene_manager.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/ui_scene_manager.cc:248: element->set_edge_color(color_scheme().horizon); We could remove these calls and call UpdateBackgroundColor at the end of the function instead.
https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:25: fullscreen_scheme.horizon = {0.039215686, 0.0, 0.082352941, 0.149019608}; On 2017/05/25 15:34:40, acondor wrote: > I didn't realize this before, but why this doesn't have alpha = 1.0, if it is > the background color? amp@? I bet that this is baking in some of the alpha stuff amp@ was mentioning. I've switched these back to 1.0 for now, but I assume we can do the alpha tweaking in a follow on? https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/ui_scene_manager.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/ui_scene_manager.cc:248: element->set_edge_color(color_scheme().horizon); On 2017/05/25 15:34:41, acondor wrote: > We could remove these calls and call UpdateBackgroundColor at the end of the > function instead. Good call. That's much nicer. Done.
https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:25: fullscreen_scheme.horizon = {0.039215686, 0.0, 0.082352941, 0.149019608}; On 2017/05/25 15:34:40, acondor wrote: > I didn't realize this before, but why this doesn't have alpha = 1.0, if it is > the background color? amp@? From the chat thread, To quote. "Josh Carpenter Tue 5:03 PM @Adam Parker Background color: rba(10, 21, 38). That's a dark blue." I interpreted rba to mean red blue alpha with a green of zero, but perhaps rba was a typo and alpha was supposed to be 1. I agree it doesn't make sense to have background transparent so maybe just shift the colors over and make alpha 1 and we can revist the exact value with Josh later. However, he was very specific about the alpha for the floor being .5 (and then going to 0 on the edge), which I'm also not sure of the motivation for, so maybe we just need Josh to clarify. https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/ui_scene_manager.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/ui_scene_manager.cc:248: element->set_edge_color(color_scheme().horizon); On 2017/05/25 15:34:41, acondor wrote: > We could remove these calls and call UpdateBackgroundColor at the end of the > function instead. I had thought about doing that as well. It does have a nice consistency to always set the colors through one mechanism.
The CQ bit was checked by vollick@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...
lgtm https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:25: fullscreen_scheme.horizon = {0.039215686, 0.0, 0.082352941, 0.149019608}; On 2017/05/25 15:55:30, amp wrote: > On 2017/05/25 15:34:40, acondor wrote: > > I didn't realize this before, but why this doesn't have alpha = 1.0, if it is > > the background color? amp@? > > From the chat thread, To quote. "Josh Carpenter > Tue 5:03 PM > @Adam Parker Background color: rba(10, 21, 38). That's a dark blue." > > I interpreted rba to mean red blue alpha with a green of zero, but perhaps rba > was a typo and alpha was supposed to be 1. > > I agree it doesn't make sense to have background transparent so maybe just shift > the colors over and make alpha 1 and we can revist the exact value with Josh > later. > > However, he was very specific about the alpha for the floor being .5 (and then > going to 0 on the edge), which I'm also not sure of the motivation for, so maybe > we just need Josh to clarify. I guess it makes sense to have 0.5 for grid color, but not much for the floor. I'd prefer an absolute value there. But sure, let's solve this later to not hold this CL longer.
lgtm
On 2017/05/25 16:00:25, acondor wrote: > lgtm > > https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... > File chrome/browser/android/vr_shell/color_scheme.cc (right): > > https://codereview.chromium.org/2905013002/diff/20001/chrome/browser/android/... > chrome/browser/android/vr_shell/color_scheme.cc:25: fullscreen_scheme.horizon = > {0.039215686, 0.0, 0.082352941, 0.149019608}; > On 2017/05/25 15:55:30, amp wrote: > > On 2017/05/25 15:34:40, acondor wrote: > > > I didn't realize this before, but why this doesn't have alpha = 1.0, if it > is > > > the background color? amp@? > > > > From the chat thread, To quote. "Josh Carpenter > > Tue 5:03 PM > > @Adam Parker Background color: rba(10, 21, 38). That's a dark blue." > > > > I interpreted rba to mean red blue alpha with a green of zero, but perhaps rba > > was a typo and alpha was supposed to be 1. > > > > I agree it doesn't make sense to have background transparent so maybe just > shift > > the colors over and make alpha 1 and we can revist the exact value with Josh > > later. > > > > However, he was very specific about the alpha for the floor being .5 (and then > > going to 0 on the edge), which I'm also not sure of the motivation for, so > maybe > > we just need Josh to clarify. > > I guess it makes sense to have 0.5 for grid color, but not much for the floor. > I'd prefer an absolute value there. But sure, let's solve this later to not hold > this CL longer. Oh, that makes sense. I've made that change (putting the grid line alpha to 0.5) in this CL.
The CQ bit was checked by vollick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from acondor@google.com, amp@chromium.org Link to the patchset: https://codereview.chromium.org/2905013002/#ps80001 (title: "update grid color alpha")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was checked by vollick@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...
lgtm subject to nits (lets discuss offline) https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:27: fullscreen_scheme.ceiling = {0.015686275, 0.031372549, 0.058823529, 1.0}; Looking at this, the ARGB format from the UX spec feels like it would be much better here. https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.h (right): https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:14: enum Mode : int { kModeNormal = 0, kModeFullscreen, kNumModes }; I think this should be line-breaked. To make that happen, put a comma after the last item. https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:20: vr::Colorf horizon; I think we should be using SkColor personally, but we can start with this. I guess it gives us a layer of insulation from the implementation. https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:23: vr::Colorf grid; floor_grid as named elsewhere.
https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:27: fullscreen_scheme.ceiling = {0.015686275, 0.031372549, 0.058823529, 1.0}; On 2017/05/25 16:26:32, cjgrant wrote: > Looking at this, the ARGB format from the UX spec feels like it would be much > better here. I didn't realize there were different formats (I thought they all had to be floats). If needed, the values that I converted from which Josh gave in rgb format were: Background color: rba(10, 21, 38) Top plane: solid color rgb(4, 8, 15), with a circular gradient for alpha, runnning 1-0 over 50 meter radius ground plane: solid color rgb(7, 15, 28), with a circular gradient for alpha, runnning 0.5-0 over 50 meter radius. floor grid: rgb(163, 224, 255)
https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.cc (right): https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.cc:27: fullscreen_scheme.ceiling = {0.015686275, 0.031372549, 0.058823529, 1.0}; On 2017/05/25 16:26:32, cjgrant wrote: > Looking at this, the ARGB format from the UX spec feels like it would be much > better here. Agreed, though perhaps this could happen as part of our migration to SkColor (mentioned in subsequent comments). https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.h (right): https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:14: enum Mode : int { kModeNormal = 0, kModeFullscreen, kNumModes }; On 2017/05/25 16:26:32, cjgrant wrote: > I think this should be line-breaked. To make that happen, put a comma after the > last item. Done. https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:20: vr::Colorf horizon; On 2017/05/25 16:26:32, cjgrant wrote: > I think we should be using SkColor personally, but we can start with this. I > guess it gives us a layer of insulation from the implementation. Agreed. Created crbug.com/726363 for the vr::Colorf -> SkColor migration. https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:23: vr::Colorf grid; On 2017/05/25 16:26:32, cjgrant wrote: > floor_grid as named elsewhere. Done.
The CQ bit was checked by vollick@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...
https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... File chrome/browser/android/vr_shell/color_scheme.h (right): https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... chrome/browser/android/vr_shell/color_scheme.h:20: vr::Colorf horizon; On 2017/05/25 16:26:32, cjgrant wrote: > I think we should be using SkColor personally, but we can start with this. I > guess it gives us a layer of insulation from the implementation. I don't see the point on using SkColor, as we are not drawing to textures, but rather passing the colors as uniforms.
On 2017/05/25 16:37:29, acondor wrote: > https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... > File chrome/browser/android/vr_shell/color_scheme.h (right): > > https://codereview.chromium.org/2905013002/diff/80001/chrome/browser/android/... > chrome/browser/android/vr_shell/color_scheme.h:20: vr::Colorf horizon; > On 2017/05/25 16:26:32, cjgrant wrote: > > I think we should be using SkColor personally, but we can start with this. I > > guess it gives us a layer of insulation from the implementation. > > I don't see the point on using SkColor, as we are not drawing to textures, but > rather passing the colors as uniforms. It's true that we'll be breaking the SkColor out into the channels in vr_shell_renderer, but I actually think that'll be easier to read since we'll have to access the channels by name rather than index (which is less obvious at a glance). In general, I'd prefer to avoid creating new types for this sort of primitive (see also, crbug.com/718004).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by vollick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from acondor@google.com, cjgrant@chromium.org, amp@chromium.org Link to the patchset: https://codereview.chromium.org/2905013002/#ps100001 (title: "address reviewer feedback")
The CQ bit was unchecked by vollick@chromium.org
The CQ bit was checked by vollick@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for chrome/browser/android/vr_shell/ui_scene_manager.cc:
While running git apply --index -3 -p1;
error: patch failed: chrome/browser/android/vr_shell/ui_scene_manager.cc:60
Falling back to three-way merge...
Applied patch to 'chrome/browser/android/vr_shell/ui_scene_manager.cc' with
conflicts.
U chrome/browser/android/vr_shell/ui_scene_manager.cc
Patch: chrome/browser/android/vr_shell/ui_scene_manager.cc
Index: chrome/browser/android/vr_shell/ui_scene_manager.cc
diff --git a/chrome/browser/android/vr_shell/ui_scene_manager.cc
b/chrome/browser/android/vr_shell/ui_scene_manager.cc
index
e8a62ad89d2e8117d898443d11a75404e96663fe..10ba870b89a7face9e78fdf391bbc95c060a64d0
100644
--- a/chrome/browser/android/vr_shell/ui_scene_manager.cc
+++ b/chrome/browser/android/vr_shell/ui_scene_manager.cc
@@ -6,6 +6,7 @@
#include "base/callback.h"
#include "base/memory/ptr_util.h"
+#include "chrome/browser/android/vr_shell/color_scheme.h"
#include "chrome/browser/android/vr_shell/textures/close_button_texture.h"
#include "chrome/browser/android/vr_shell/textures/ui_texture.h"
#include
"chrome/browser/android/vr_shell/ui_elements/audio_capture_indicator.h"
@@ -60,8 +61,6 @@ static constexpr float kLoadingIndicatorOffset =
static constexpr float kSceneSize = 25.0;
static constexpr float kSceneHeight = 4.0;
static constexpr int kFloorGridlineCount = 40;
-static constexpr vr::Colorf kBackgroundHorizonColor = {0.57, 0.57, 0.57, 1.0};
-static constexpr vr::Colorf kBackgroundCenterColor = {0.48, 0.48, 0.48, 1.0};
static constexpr float kFullscreenWidthDms = 1.138;
static constexpr float kFullscreenHeightDms = 0.64;
@@ -80,8 +79,6 @@ static constexpr float kFullscreenWidth =
kFullscreenWidthDms * kFullscreenDistance;
static constexpr float kFullscreenVerticalOffset =
-kFullscreenVerticalOffsetDms * kFullscreenDistance;
-static constexpr vr::Colorf kFullscreenHorizonColor = {0.1, 0.1, 0.1, 1.0};
-static constexpr vr::Colorf kFullscreenCenterColor = {0.2, 0.2, 0.2, 1.0};
// Tiny distance to offset textures that should appear in the same plane.
static constexpr float kTextureOffset = 0.01;
@@ -248,8 +245,6 @@ void UiSceneManager::CreateBackground() {
element->set_translation({0.0, -kSceneHeight / 2, 0.0});
element->set_rotation({1.0, 0.0, 0.0, -M_PI / 2.0});
element->set_fill(vr_shell::Fill::OPAQUE_GRADIENT);
- element->set_edge_color(kBackgroundHorizonColor);
- element->set_center_color(kBackgroundCenterColor);
element->set_draw_phase(0);
floor_ = element.get();
content_elements_.push_back(element.get());
@@ -264,8 +259,6 @@ void UiSceneManager::CreateBackground() {
element->set_translation({0.0, kSceneHeight / 2, 0.0});
element->set_rotation({1.0, 0.0, 0.0, M_PI / 2});
element->set_fill(vr_shell::Fill::OPAQUE_GRADIENT);
- element->set_edge_color(kBackgroundHorizonColor);
- element->set_center_color(kBackgroundCenterColor);
element->set_draw_phase(0);
ceiling_ = element.get();
content_elements_.push_back(element.get());
@@ -280,17 +273,13 @@ void UiSceneManager::CreateBackground() {
element->set_translation({0.0, -kSceneHeight / 2 + kTextureOffset, 0.0});
element->set_rotation({1.0, 0.0, 0.0, -M_PI / 2});
element->set_fill(vr_shell::Fill::GRID_GRADIENT);
- element->set_center_color(kBackgroundHorizonColor);
- vr::Colorf edge_color = kBackgroundHorizonColor;
- edge_color.a = 0.0;
- element->set_edge_color(edge_color);
element->set_gridline_count(kFloorGridlineCount);
element->set_draw_phase(0);
floor_grid_ = element.get();
content_elements_.push_back(element.get());
scene_->AddUiElement(std::move(element));
- scene_->SetBackgroundColor(kBackgroundHorizonColor);
+ UpdateBackgroundColor();
}
void UiSceneManager::CreateUrlBar() {
@@ -366,32 +355,28 @@ void UiSceneManager::ConfigureScene() {
main_content_->set_translation(
{0, kFullscreenVerticalOffset, -kFullscreenDistance});
main_content_->set_size({kFullscreenWidth, kFullscreenHeight, 1});
-
- ConfigureBackgroundColor(kFullscreenCenterColor, kFullscreenHorizonColor);
} else {
// Note that main_content_ is already visible in this case.
main_content_->set_translation(
{0, kContentVerticalOffset, -kContentDistance});
main_content_->set_size({kContentWidth, kContentHeight, 1});
-
- ConfigureBackgroundColor(kBackgroundCenterColor, kBackgroundHorizonColor);
}
+ UpdateBackgroundColor();
scene_->SetBackgroundDistance(main_content_->translation().z() *
-kBackgroundDistanceMultiplier);
}
-void UiSceneManager::ConfigureBackgroundColor(vr::Colorf center_color,
- vr::Colorf horizon_color) {
- scene_->SetBackgroundColor(horizon_color);
- floor_->set_edge_color(horizon_color);
- floor_->set_center_color(center_color);
- ceiling_->set_edge_color(horizon_color);
- ceiling_->set_center_color(center_color);
- floor_grid_->set_center_color(horizon_color);
- vr::Colorf edge_color = horizon_color;
- edge_color.a = 0.0;
- floor_grid_->set_edge_color(edge_color);
+void UiSceneManager::UpdateBackgroundColor() {
+ scene_->SetBackgroundColor(color_scheme().horizon);
+ ceiling_->set_center_color(color_scheme().ceiling);
+ ceiling_->set_edge_color(color_scheme().horizon);
+ floor_->set_center_color(color_scheme().floor);
+ floor_->set_edge_color(color_scheme().horizon);
+ floor_grid_->set_center_color(color_scheme().floor_grid);
+ vr::Colorf floor_grid_edge_color = color_scheme().floor_grid;
+ floor_grid_edge_color.a = 0.0;
+ floor_grid_->set_edge_color(floor_grid_edge_color);
}
void UiSceneManager::SetAudioCapturingIndicator(bool enabled) {
@@ -482,4 +467,9 @@ int UiSceneManager::AllocateId() {
return next_available_id_++;
}
+const ColorScheme& UiSceneManager::color_scheme() const {
+ return ColorScheme::GetColorScheme(fullscreen_ ? ColorScheme::kModeFullscreen
+ : ColorScheme::kModeNormal);
+}
+
} // namespace vr_shell
The CQ bit was checked by vollick@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...
The CQ bit was checked by vollick@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by vollick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from acondor@google.com, cjgrant@chromium.org, amp@chromium.org Link to the patchset: https://codereview.chromium.org/2905013002/#ps140001 (title: "rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1495742019077840,
"parent_rev": "c3e0f34012c116cbfefa64683ac33f5ebe621618", "commit_rev":
"5cee228225d5b929b96eac976994a8809cfbff20"}
Message was sent while issue was closed.
Description was changed from ========== [vr] Introduce ColorScheme This refactors some colors into a color scheme struct, accessed by mode. Currently, this only houses a few colors, but it would be a convenient home for all colors that could vary between normal, fullscreen and, eventually, incognito modes. Note: This incorporates some colors and changes from acondor@ and amp@. BUG=715664 ========== to ========== [vr] Introduce ColorScheme This refactors some colors into a color scheme struct, accessed by mode. Currently, this only houses a few colors, but it would be a convenient home for all colors that could vary between normal, fullscreen and, eventually, incognito modes. Note: This incorporates some colors and changes from acondor@ and amp@. BUG=715664 Review-Url: https://codereview.chromium.org/2905013002 Cr-Commit-Position: refs/heads/master@{#474766} Committed: https://chromium.googlesource.com/chromium/src/+/5cee228225d5b929b96eac976994... ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://chromium.googlesource.com/chromium/src/+/5cee228225d5b929b96eac976994... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
