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

Issue 731894 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR



Sign in to add a comment

Rework VR texture font rendering to be testable on both Linux and Android

Project Member Reported by cjgrant@chromium.org, Jun 9 2017

Issue description

VR UI unit tests now run on Linux. One test works differently (and fails) because our font code falls flat on Linux:

TEST(UrlBarTextureTest, WillNotFailOnNonAsciiURLs)

We should either fix Linux font rendering, or make the font subsystem mockable, so that the test can run meaningfully on any platform.


 
Labels: -VR-BBB
Labels: -M-62 M-63
I filed a similar bug for Windows: crbug/770893.  I'm keeping them separate because there may need to be a Windows-specific implementation for font fallback.  However, if the fix for this also addresses 770893, feel free to resolve that as duplicate.
Labels: -M-63 Needs-Feedback
Owner: cjgrant@chromium.org
Chris, what would it take to get this running on linux?
Not sure other than it'll take an investigation.
Owner: ----
Status: Available (was: Assigned)
Components: -UI>Browser>VR Internals>VR
Labels: -Needs-Feedback -Proj-VR-Shell Proj-VR
Status: WontFix (was: Available)
Looking at this now - the tests in question work properly on Android, and text rendering works fine on Windows (even if the specific test in question doesn't work the same way on Windows).

I'm going to close this off as a WontFix.
Components: Internals>XR
Owner: cjgrant@chromium.org
Status: Untriaged (was: WontFix)
With newer versions of harfbuzz this test now fails on Android too. Can we please fix or disable the test as this is blocking several text rendering improvements and performance optimizations for Chrome and Blink.

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8938748478828460688/+/steps/vr_common_unittests_on_Android_device_Nexus_5X__with_patch_/0/stdout
Cc: vollick@chromium.org
Labels: -Pri-3 Pri-2
+vollick as cjgrant is OOO.
Looks like it's UrlText.WillNotFailOnNonAsciiURLs. I'll need to confirm that VR works with the harfbuzz changes, but if so, I'll disable this test on Android until we can get a proper fix.
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 10

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

commit a2ce72e4384ec85ce4730dad11aa59469272060e
Author: Ian Vollick <vollick@chromium.org>
Date: Fri Aug 10 19:54:17 2018

[vr] Disable WillNotFailOnNonAsciiURLs

This will currently not pass with new versions of HarfBuzz.

Bug: 731894
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ib5ae456bf44bfdedd6a0edf36b54dbbf12bae18a
Reviewed-on: https://chromium-review.googlesource.com/1169774
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582298}
[modify] https://crrev.com/a2ce72e4384ec85ce4730dad11aa59469272060e/chrome/browser/vr/elements/url_text_unittest.cc

We should be able to re-enable the test again now.
Oh great! I'll create a revert of the disabling patch.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 17

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

commit 86bdbfd2aff5ce74c6c22bfcf701ea81ede2c1a5
Author: Ian Vollick <vollick@chromium.org>
Date: Fri Aug 17 10:53:25 2018

Revert "[vr] Disable WillNotFailOnNonAsciiURLs"

This reverts commit a2ce72e4384ec85ce4730dad11aa59469272060e.

Reason for revert: this test should now pass.

Original change's description:
> [vr] Disable WillNotFailOnNonAsciiURLs
> 
> This will currently not pass with new versions of HarfBuzz.
> 
> Bug: 731894
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: Ib5ae456bf44bfdedd6a0edf36b54dbbf12bae18a
> Reviewed-on: https://chromium-review.googlesource.com/1169774
> Commit-Queue: Ian Vollick <vollick@chromium.org>
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582298}

TBR=vollick@chromium.org,mthiesse@chromium.org,eae@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 731894
Change-Id: Id02d8c770a763c5c37f993d701d03e518a043ee7
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1178164
Reviewed-by: Ian Vollick <vollick@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584018}
[modify] https://crrev.com/86bdbfd2aff5ce74c6c22bfcf701ea81ede2c1a5/chrome/browser/vr/elements/url_text_unittest.cc

Cc: -vollick@chromium.org cjgrant@chromium.org
Labels: -Proj-VR OS-Android OS-Linux
Owner: vollick@chromium.org
Status: Assigned (was: Untriaged)
Assigning to you, Ian, since you worked on this. Please add a milestone if appropriate or close this bug if no more work is necessary.

Sign in to add a comment