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

Issue 806452 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

vr asset loader does incorrect version checking

Project Member Reported by vollick@chromium.org, Jan 27 2018

Issue description

I incorrectly compared major and minor versions which will lead to incorrect behavior in future (2.0 will be reported as preceding 1.1).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 27 2018

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

commit a04e5845a535c8e31c3182ed19e8065816b7e2c9
Author: Ian Vollick <vollick@chromium.org>
Date: Sat Jan 27 18:30:47 2018

[vr] Use alternate colors for environments with gradients

This CL uses base::Versions built-in comparison logic rather than
my incorrect, hand-rolled code. It also swaps out the color scheme
when the component has a gradient.

Also reduces the number of stars used with these environments.

Bug:  806452 , 806471 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I250692105e680e1f3d0fee372f6886f35f2ced95
Reviewed-on: https://chromium-review.googlesource.com/889842
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532240}
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/assets_loader.cc
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/assets_loader.h
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/elements/environment/stars.cc
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/model/color_scheme.cc
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/model/color_scheme.h
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/ui.cc
[modify] https://crrev.com/a04e5845a535c8e31c3182ed19e8065816b7e2c9/chrome/browser/vr/ui_scene_creator.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 30 2018

Labels: merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea

commit 57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea
Author: Ian Vollick <vollick@chromium.org>
Date: Tue Jan 30 22:48:43 2018

[vr] Use alternate colors for environments with gradients

This CL uses base::Versions built-in comparison logic rather than
my incorrect, hand-rolled code. It also swaps out the color scheme
when the component has a gradient.

Also reduces the number of stars used with these environments.

Bug:  806452 , 806471 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I250692105e680e1f3d0fee372f6886f35f2ced95
Reviewed-on: https://chromium-review.googlesource.com/889842
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#532240}(cherry picked from commit a04e5845a535c8e31c3182ed19e8065816b7e2c9)
Reviewed-on: https://chromium-review.googlesource.com/894246
Cr-Commit-Position: refs/branch-heads/3325@{#187}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/assets_loader.cc
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/assets_loader.h
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/elements/environment/stars.cc
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/model/color_scheme.cc
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/model/color_scheme.h
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/ui.cc
[modify] https://crrev.com/57f6deb5f185d0f42bce098d9c8b79a1eeaa34ea/chrome/browser/vr/ui_scene_creator.cc

Status: Fixed (was: Started)
Labels: Test-Complete

Sign in to add a comment